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

Experimental :fast-refresh feature is broken #29

Closed
khmelevskii opened this issue Feb 24, 2020 · 9 comments
Closed

Experimental :fast-refresh feature is broken #29

khmelevskii opened this issue Feb 24, 2020 · 9 comments
Labels
fast-refresh Having to do with React Fast Refresh

Comments

@khmelevskii
Copy link

This feature was broken in this commit 0ac5b18

@khmelevskii khmelevskii changed the title Experimental :fast-refresh is broken Experimental :fast-refresh feature is broken Feb 24, 2020
@lilactown
Copy link
Owner

In the future, please report what you mean by "broken." e.g. if it doesn't reload at all, or throws an error, etc.

That was fixed in 7edfe88. I'll prepare a new release soon, but in the meantime you can depend directly on that commit using deps.edn.

FWIW, the fast refresh support is still very experimental and does not work in all cases: edits to namespaces which components transitively dependent on will not trigger a proper refresh.

@khmelevskii
Copy link
Author

@Lokeh sure.
but unfortunately fast refresh doesn't work for me. I tried to use last commit and it didn't help.
About "broken" - I mean that I can't see any exceptions but refresh doesn't work.
So, this feature works correctly in this commit e43b5ed and doesn't work after it (tried to use last commit too).

@lilactown lilactown reopened this Feb 24, 2020
@lilactown
Copy link
Owner

Ah, I think another change I made is biting you. Apologies for the out of date documentation.

In order to enable fast refresh, you must also create a function to inject the global React devtools hooks, and an after-load hook that will call helix.experimental.refresh/refresh!.

An example would be:

(ns my-app.dev
  (:require [helix.experimental.refresh :as refresh])

(refresh/inject-hook!)

(defn ^:dev/after-load after-load []
  (refresh/refresh!))

And include this in your app at dev time. With shadow-cljs, adding it to your preloads should work.

Let me know if this does or does not fix it for you. Thanks!

@aiba
Copy link
Contributor

aiba commented Feb 25, 2020

Looks like refresh/inject-hook! was introduced after the latest release (0.0.8). Can we cut a new release? (Alternatively, is there is a way for shadow-cljs to depend on a git repo/commit, the way you can with deps.edn?)

@lilactown
Copy link
Owner

You can use dep.edn with shadow-CLJS as laid out in their user guide.

I will cut a new release tomorrow.

@khmelevskii
Copy link
Author

@Lokeh thank you! inject-hook! works correct. Only one thing, as I can see helix.experimental.refresh includes to production build but it would be great to remove this code on prod.

@aiba
Copy link
Contributor

aiba commented Feb 25, 2020

You can use dep.edn with shadow-CLJS as laid out in their user guide.

Oh wow, thanks for pointing this out!

Using that, I got fast-refresh working with react-native. I realized you actually want to tell react-native to "Disable Fast Refresh" in the simulator. Otherwise, it tries to do its own fast-refresh hooking which does not jive with shadow-cljs recompile/reload cycles.

Anyway, here's a working example project for anyone interested: https://github.com/aiba/helix-react-native.

I'm really excited to port all my hx code to helix and to be able to use fast-refresh!

@lilactown
Copy link
Owner

lilactown commented Feb 25, 2020

@Lokeh thank you! inject-hook! works correct. Only one thing, as I can see helix.experimental.refresh includes to production build but it would be great to remove this code on prod.

Yes, you should configure your project to only include this code in development - either by adding it to user.cljs or by adding it to a namespace that is loaded via :preloads.

@lilactown lilactown added the fast-refresh Having to do with React Fast Refresh label Feb 25, 2020
@khmelevskii
Copy link
Author

@Lokeh thank you. Everything works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-refresh Having to do with React Fast Refresh
Projects
None yet
Development

No branches or pull requests

3 participants