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

Consider dispatch from useActionState stable #29665

Merged
merged 3 commits into from
Jul 6, 2024

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented May 30, 2024

Summary

The dispatch function from useState and useReducer is stable so exhaustive-deps doesn't require them to be in dependencies. This applies the same rule to useActionState and useFormState

How did you test this change?

  • yarn test packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js fails with on the first commit but passes on the second.

Copy link

vercel bot commented May 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 24, 2024 7:39am

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels May 30, 2024
@react-sizebot
Copy link

react-sizebot commented May 30, 2024

Comparing: ea6e059...510f053

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.11% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 496.04 kB 496.04 kB = 88.77 kB 88.77 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 500.84 kB 500.84 kB = 89.45 kB 89.46 kB
facebook-www/ReactDOM-prod.classic.js = 593.48 kB 593.48 kB = 104.38 kB 104.38 kB
facebook-www/ReactDOM-prod.modern.js = 569.87 kB 569.87 kB = 100.77 kB 100.77 kB
test_utils/ReactAllWarnings.js Deleted 63.82 kB 0.00 kB Deleted 15.95 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.82 kB 0.00 kB Deleted 15.95 kB 0.00 kB

Generated by 🚫 dangerJS against 510f053

@eps1lon eps1lon marked this pull request as ready for review May 30, 2024 13:17
@sophiebits
Copy link
Contributor

Should we just do this logic for the new name and not the old one?

@eps1lon
Copy link
Collaborator Author

eps1lon commented Jun 4, 2024

It's already out there with the lightest warning that it's just been renamed i.e. you can still use ReactDOM.useFormState. I don't see a harm treating useFormState just like useActionState because that's what it ultimately is. If it would require more implementation effort to add support for it, I'd probably use your argument but since it's pretty trivial to implement, I don't see an issue.

josephsavona pushed a commit that referenced this pull request Jun 5, 2024
… is non-reactive (#29705)

Summary
The dispatch function from useReducer is stable, so it is also non-reactive.

the related PR: #29665
the related comment: #29674 (comment)

I am not sure if the location of the new test file is appropriate😅.

How did you test this change?
Added the specific test compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md.
github-actions bot pushed a commit that referenced this pull request Jun 5, 2024
… is non-reactive (#29705)

Summary
The dispatch function from useReducer is stable, so it is also non-reactive.

the related PR: #29665
the related comment: #29674 (comment)

I am not sure if the location of the new test file is appropriate😅.

How did you test this change?
Added the specific test compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md.

DiffTrain build for commit 704aeed.
@eps1lon
Copy link
Collaborator Author

eps1lon commented Jun 18, 2024

Going with just useActionState to align with the React Compiler.

@eps1lon eps1lon requested a review from sophiebits June 24, 2024 07:33
@eps1lon eps1lon changed the title Consider dispatch from useActionState and useFormState stable Consider dispatch from useActionState stable Jun 24, 2024
@eps1lon
Copy link
Collaborator Author

eps1lon commented Jun 24, 2024

Aligned with React Compiler to only consider React.useActionState. Code using ReactDOM.useFormState should just switch to React.useActionState

@eps1lon eps1lon merged commit 1b0132c into facebook:main Jul 6, 2024
156 checks passed
@eps1lon eps1lon deleted the useActionState-stable-dispatch branch July 6, 2024 06:57
github-actions bot pushed a commit that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants