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 Fast Refresh #2184

Closed
garrett-hopper opened this issue Dec 15, 2019 · 12 comments
Closed

React Fast Refresh #2184

garrett-hopper opened this issue Dec 15, 2019 · 12 comments

Comments

@garrett-hopper
Copy link

Since react-hot-loader is being replaced with react-refresh (https://github.com/gaearon/react-hot-loader#deprecation-note), are there any plan to integrate Preact with React Fast Refresh? (facebook/react#16604 (comment))

@theKashey
Copy link

Or, at least, react-hot-loader - #1120

@marvinhagemeister
Copy link
Member

@theKashey Do I get this right, that a hook to check equality for vnodes in our reconciler is all that'd be needed?

@theKashey
Copy link

That was all needed before PreactX, not sure how it works nowadays, but both React-Hot-Loader and FastRefresh are relying on the "controllable comparison" of the old tree and the new. However, there are many nuances around proper reset or preserve of internal states, what should be updated and how, and I am afraid but both RHL and FastRefresh are not doing that correctly.

Speaking of required code changes - while RHL injects code via webpack-loader(the patch) and potentially could do the same with Preact, with FastRefresh you are expected to:

  1. work with the new dev tools, features are tightly bound together (as far as I know)
  2. support hot-reloading as a core feature
  3. provide methods for bundler-level code changes (aka function "signatures" and families)

@Rafi993
Copy link
Contributor

Rafi993 commented Dec 16, 2019

I would really like to help !!! If this is still not taken.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Dec 16, 2019

I see two potential tasks here:

  • React hot loader support
  • Fast refresh support

Fast refresh might be quite hard if there's a binding to the dev-tools but this can be resolved by us making a custom babel-plugin and runtime for it. I have not yet looked into this but it might be hard if our current system does not support it.

I think the easiest win would be to ensure RHL support, we had that in the past (will test if it's still working)

Looking at the code it might be possible to integrate this to be fair, we would just need to find a way to avoid the React symbols and support our way of rendering.

When reading through the babel-plugin code I get a bit scared about Functional components due to us doing the class-conversion for them. This might turn out to not be an issue if the diffing reexecutes which if I understand the fast-refresh well should not happen.

Our hooks are covered here: https://github.com/facebook/react/blob/master/packages/react-refresh/src/ReactFreshBabelPlugin.js#L213

React useState initialState seems to be enriched (?) with an additional getSource function https://github.com/facebook/react/blob/master/packages/react-refresh/src/ReactFreshBabelPlugin.js#L384 this will be a hard one to achieve since this would be an options call across different packages in /debug possibly (in diffed I would assume).

Problem being that if we would bet on implementing refresh that we would probably bump into problems integrating extra packages like composition.

This is a little high-level writeup

@marvinhagemeister
Copy link
Member

@Rafi993 Feel free, I doubt that this task is for a single person alone. It likely requires coordination between multiple devs. Easiest way is to share all findings approaches in this thread here 👍

@Rafi993
Copy link
Contributor

Rafi993 commented Dec 16, 2019

Awesome thanks @marvinhagemeister

@theKashey
Copy link

theKashey commented Dec 16, 2019

@JoviDeCroock - please ignore everything around babel plugin - it just provides data to the underlying comparison function as well as function bodies (getSource) to perform the comparison.

Preparation:

  • babel plugin helps identify families - components which might replace each other without destruction of the underlying tree.
  • babel plugin helps identify hooks composition to break the "rule of hooks". Like you can add or remove hooks

Preact:

  • preact should compare two nodes, and if they are not qual - check are they from the same family. If yes - do nothing, continue rendering using the new one, and if they are different - purge the old branch.
  • if family branch is used - preact should compare provided signatures, and if they are not equal - destroy hook state/drop local state.

That's all. Only amended comparison and the communication channel between babel plugin and preact are needed.

Keep in mind, ReactRefresh does not support classes by design, however just drops the local state, keeping the branches.

@debugpai
Copy link

debugpai commented Apr 1, 2020

Hello guys,
I wanted to understand what is the level of support now for @hot-loader/react-dom with preact.

I'm able to use the latest version of preact but get the error
TypeError: ReactSharedInternals is undefined
I'm assuming that's happening because @hot-loader/react-dom expects a react of same version to be installed.

@theKashey
Copy link

@hot-loader/react-dom should be 100% preact incompatible. It is actually a normal react, and expects a React of corresponding version to be used.

@JoviDeCroock
Copy link
Member

Work for a hot-reloader for Preact is being done here it's experimental for now.

@JoviDeCroock
Copy link
Member

Closing this issue since I feel like prefresh is stable enough to be used.

Integrations are available for webpack, gatsby, next, snowpack, vite and nollup

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

7 participants