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

[Fast Refresh] Don’t Scan the Tree #21139

Open
gaearon opened this issue Mar 30, 2021 · 2 comments
Open

[Fast Refresh] Don’t Scan the Tree #21139

gaearon opened this issue Mar 30, 2021 · 2 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2021

We had a conversation with @sebmarkbage about #20417. He had a different implementation idea that should resolve such issues. I probably won't do this now but I want to write it down for future reference.

Currently, after we gather a list of types that need to be updated in the tree, we scan the tree and tag Fibers to be updated. This happens here. But like #20417 shows, this doesn't work if the type is not in the tree. E.g. if the function we changed is only being called directly, and its "type" is not the actual wrapper that exists in the tree.

An alternative implementation could instead inject a special Hook call into each registered component. For example by generating a wrapper. That Hook would return the latest actual implementation, and schedule updates on itself when it changes. Which means there would be no need to scan the tree at all. Instead, each component would register itself for updates. For false positives outside of React rendering, the Hook would just be callthrough so it wouldn't break anything.

This seems like a pretty significant change to the implementation so I'm probably not going to work on this now. Maybe we could consider it next time we need to change the implementation details. Or maybe somebody sufficiently motivated wants to hack on this. I can provide some code pointers but it’s not a beginner-friendly issue.

@gaearon gaearon added Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Component: Fast Refresh Difficulty: challenging React Core Team Opened by a member of the React Core Team Resolution: Backlog and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Mar 30, 2021
@backbencher00
Copy link

Hello @gaearon hope you are doing well, I have a good hands on in react and wants to work on this issue, It would be great if you assign this to me.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 21, 2021

Hi, this is a really challenging issue. I'm happy to answer questions you might have here, but it requires a pretty deep understanding of how React works internally. You're welcome to try though!

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

No branches or pull requests

2 participants