Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React 18 #81

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

React 18 #81

wants to merge 11 commits into from

Conversation

greglittlefield-wf
Copy link
Contributor

No description provided.

Comment on lines -26 to +25
"@babel/cli": "^7.12.13",
"@babel/core": "^7.12.13",
"@babel/plugin-transform-runtime": "^7.12.15",
"@babel/preset-env": "^7.12.13",
"@babel/preset-typescript": "^7.12.13",
"babel-plugin-transform-inline-environment-variables": "^0.4.3",
"@rollup/plugin-babel": "^5.2.3",
"@rollup/plugin-commonjs": "^17.1.0",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^11.1.1",
"rollup": "^2.38.5",
"rollup-plugin-filesize": "^9.1.0",
"rollup-plugin-node-builtins": "^2.1.2",
"rollup-plugin-node-globals": "^1.4.0"
"vite": "^5.4.9"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow that's cool that vite replaced all those!

@@ -58,7 +58,7 @@ void main() {
var numRuns = 0;
await rtl.waitFor(() async {
numRuns++;
expect(numRuns, 5);
expect(numRuns, 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just to make the test faster or did waitFor change somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, we should look into this; will probably just have to try changing it locally and see what happens

Comment on lines +111 to +112
external bool? Function(Node?)? get filterNode;
external set filterNode(bool? Function(Node?)? value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the arg and return value aren't typed as non-nullable like in the TS type?

Suggested change
external bool? Function(Node?)? get filterNode;
external set filterNode(bool? Function(Node?)? value);
external bool Function(Node)? get filterNode;
external set filterNode(bool Function(Node)? value);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; not that I'm aware of, and we can probably update this

@greglittlefield-wf
Copy link
Contributor Author

@sydneyjodon-wk Thanks for the comments! You cool if we address those in the follow-up to add dual-17/18 support?

@sydneyjodon-wk
Copy link
Contributor

@greglittlefield-wf yep that works for me!

@kealjones-wk
Copy link
Contributor

i love this

@greglittlefield-wf
Copy link
Contributor Author

@kealjones-wk you mean the parts that you did? 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants