Fork me on GitHub
#helix
<
2020-05-26
>
dominicm08:05:45

Just learned another cause of refresh not working - not installing react devtools :)

🤯 4
Space Guy14:05:15

I'm making another 'REBL inspector' thing with Smalltalk-ey floating windows: https://github.com/jc776/bubble

8
lilactown15:05:47

Very cool!!!

lilactown15:05:23

I tried something similar myself but didn’t get nearly that far 😅

lilactown15:05:41

@spacemods I see you created a way of handling reloading components with hooks. Did the fast-refresh / react-refresh integration not work for you?

Space Guy19:05:06

The regular 'react-refresh' integration does work OK - no problems. I find it easier to follow "what's going on" on reload with this one given the rule "it's the same function, so state+hooks are preserved". react-refresh does the same with internal fibers/roots. Since it always re-renders "from the top", it might do a little better at 'transitive dependencies' & inputs that aren't components. I also haven't needed any node/webpack yet, although a 'cljsjs.react-refresh' would also do the same.

lilactown19:05:58

and I guess you don’t mind losing local state much since you’re storing most of it in a global atom?

Space Guy21:05:24

I'm not noticing any local state being lost - the selected tab in 'TabView' is local; seems to be preserved in small changes to the window, tab view, the 'register-inspector' and all. It does reset on breaking changes to nesting or order without 'key', and deliberately when hooks change - does react-refresh do something more clever than 'reset' for changes to hooks?

lilactown21:05:22

hmm yeah, I guess as long as you keep the function reference the same it won’t re-mount the component :thinking_face:

lilactown21:05:34

react-hooks technically supports the ability to differentiate between different “kinds” of hooks changes - e.g. if you change the body of an effect, it won’t re-mount. but if you change the order of hooks or change the initial state, it will re-mount

lilactown21:05:52

but it might be just as easy to support that in what you’re doing which is just keeping the reference stable

Space Guy22:05:12

cool, thanks!

Derek Passen15:05:49

How do we deal with peer dependencies on ‘react’ with the deps.cljs in 0.11?

lilactown15:05:20

:thinking_face: can you explain what you mean?

lilactown15:05:32

this is the second time someone has brought up issues with deps.cljs

Derek Passen15:05:54

I suppose it’s not a big deal. A lot of libraries declare a peer dependency on react directly

Derek Passen15:05:09

Is the recommended way of using helix to omit react from package.json?

lilactown15:05:00

AFAICT deps.cljs allows me to do a few things: • improve the “quick start” experience because I don’t even need to talk about installing via npm, if you don’t care what React version you use • ensure that all npm libs are installed, e.g. react-refresh, before enabling feature flags by default in the future • improve dev experience when used with the new :bundle target

Derek Passen15:05:11

I see an error (not sure if shadow or helix) when I upgrade:

IllegalArgumentException: No matching field found: exist for class .File
        clojure.lang.Reflector.getInstanceField (Reflector.java:397)
        clojure.lang.Reflector.invokeNoArgInstanceMember (Reflector.java:440)
        shadow.cljs.devtools.server.npm-deps/install-deps (npm_deps.clj:160)
        shadow.cljs.devtools.server.npm-deps/install-deps (npm_deps.clj:131)
        shadow.cljs.devtools.server.npm-deps/main (npm_deps.clj:242)
        shadow.cljs.devtools.server.npm-deps/main (npm_deps.clj:228)
        shadow.cljs.devtools.cli-actual/main (cli_actual.clj:141)
        shadow.cljs.devtools.cli-actual/main (cli_actual.clj:132)
        clojure.core/apply (core.clj:669)
        clojure.core/apply (core.clj:660)
        shadow.cljs.devtools.cli-actual/-main (cli_actual.clj:219)
        shadow.cljs.devtools.cli-actual/-main (cli_actual.clj:217)
        clojure.lang.Var.applyTo (Var.java:705)
        clojure.core/apply (core.clj:665)
        clojure.core/apply (core.clj:660)
        shadow.cljs.devtools.cli/-main (cli.clj:75)
        shadow.cljs.devtools.cli/-main (cli.clj:67)
        clojure.lang.Var.applyTo (Var.java:705)
        clojure.core/apply (core.clj:665)
        clojure.main/main-opt (main.clj:514)
        clojure.main/main-opt (main.clj:510)
        clojure.main/main (main.clj:664)
        clojure.main/main (main.clj:616)
        clojure.lang.Var.applyTo (Var.java:705)
        clojure.main.main (main.java:40)

Derek Passen15:05:39

I can try to carve out a minimal repro if this is new/unknown

lilactown15:05:47

I haven’t seen that

lilactown15:05:52

do you have a package.json file?

lilactown15:05:32

yeah, it might be a bug in shadow?

lilactown15:05:37

I would try and repro it

lilactown15:05:34

i’m going to bump our project at work just to be sure

Derek Passen15:05:22

Looks like it broke from shadow 2.9.8 to 2.9.9

Derek Passen16:05:34

So deps.cljs get written to my package.json, nice!

lilactown16:05:57

but yes, feel free to override react however you like

Derek Passen16:05:07

That makes way more sense than my mental model earlier

Space Guy19:05:06

The regular 'react-refresh' integration does work OK - no problems. I find it easier to follow "what's going on" on reload with this one given the rule "it's the same function, so state+hooks are preserved". react-refresh does the same with internal fibers/roots. Since it always re-renders "from the top", it might do a little better at 'transitive dependencies' & inputs that aren't components. I also haven't needed any node/webpack yet, although a 'cljsjs.react-refresh' would also do the same.