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

feat(compiler): Consider dispatch fn from useActionState and useFormState to be non-reactive #29758

Conversation

hieudo-dev
Copy link
Contributor

@hieudo-dev hieudo-dev commented Jun 4, 2024

Summary

Currently, the dispatch functions returned by useActionState and useFormState are considered as reactive. This will cause the compiler to miss some optimizations opportunities.

This PR updates babel-plugin-compiler-react to tag those dispatch functions as non-reactive so they can be pruned and optimized accordingly.

Addresses #29674

How did you test this change?

Manually, by running the compiler playground locally. My assumption is that non-reactive returnValues will be checked against Symbol.for("react.memo_cache_sentinel") in the compiled code, so this was used as the success criteria of this PR.

Fixtures were added to test this as well.

Copy link

vercel bot commented Jun 4, 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 12, 2024 6:43am

@hieudo-dev hieudo-dev force-pushed the feat/consider-dispatchs-non-reactive branch from 941c01a to 9f817ae Compare June 5, 2024 14:36
@hieudo-dev hieudo-dev requested a review from josephsavona June 5, 2024 14:37
@hieudo-dev
Copy link
Contributor Author

@josephsavona Thanks for the review! I have updated the import.
Let me know if you see any remaining issues.

@eps1lon
Copy link
Collaborator

eps1lon commented Jun 10, 2024

@hieudo-dev Looks like this needs a rebase and formatting of the code via Prettier.

@hieudo-dev
Copy link
Contributor Author

hieudo-dev commented Jun 12, 2024

@hieudo-dev Looks like this needs a rebase and formatting of the code via Prettier.

Thanks for the heads up! I've updated the PR.
I also run prettier again (using configs inside of ./compiler) but found no apparent difference. If there's something in particular that indicates some code is unformatted pls let me know.

@hieudo-dev
Copy link
Contributor Author

Hi @josephsavona, just wanted to give you a friendly reminder that this PR should be ready for further review/merging. Feel free to let me know if you see any issues left

@josephsavona
Copy link
Contributor

I couldn't push here, so i added an updated version at #29917 which includes your commit. Thanks for the contribution!

josephsavona added a commit that referenced this pull request Jun 17, 2024
Updated version of #29758 removing `useFormState` since that was the
previous name for `useActionState`.

---------

Co-authored-by: Hieu Do <hieudn.uh@gmail.com>
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.

4 participants