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

A fast refresh example, that doesn't work #38

Closed
wants to merge 1 commit into from

Conversation

danieroux
Copy link
Contributor

Hi Will,

I've tried to put together a fast-refresh example that works, so that I can replicate it in my code.

I cannot get it to work. What am I missing?

My expectation:

  • If I change the style color from green to red, I'll see the change.
  • And when that happens, for the local name-state to not change

@lilactown
Copy link
Owner

Hey @danieroux. I've updated the refresh example to be in line with the latest helix API.

Your PR is relatively correct. The biggest problem I ran into was a mismatch between react-refresh and react.

I'm not sure which react-refresh version maps to which react version, but I had to forcefully install react-refresh@latest in helix in order to pick up the correct version, as well as clear out node_modules and my shadow-cljs build cache.

- Changing the code in `greet` will render the difference
- Changing the color from green to red, will change the component, thanks to react-refresh
- Although: The `(hooks/use-effect :once ,,,)` will run on every react-refresh

The last one is a bit of a bummer.
@danieroux
Copy link
Contributor Author

Thank you, this now works for me too.

This may very well be out of the realm of helix, and a react-refresh issue:

(hooks/use-effect :once ,,,) runs on every (refresh/refresh!). In the demo the js/setInterval gets called on every refresh.

Which is different from running :once :-)

@lilactown
Copy link
Owner

According to react-refresh's tests, this is expected behavior:

https://github.com/facebook/react/blob/master/packages/react-refresh/src/__tests__/ReactFresh-test.js#L2282

@danieroux
Copy link
Contributor Author

That's quite surprising behavior. Thank you for finding it.

This is where they lose me: https://github.com/facebook/react/blob/235a6c4af67e3e1fbfab7088c857265e0c95b81f/packages/react-refresh/src/__tests__/ReactFresh-test.js#L2344

@danieroux danieroux closed this Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants