Fork me on GitHub
#helix
<
2021-06-24
>
Alex J Henderson14:06:20

Hi, I asked this first in the shadow-cljs channel but I was told I might have more luck here. I'm noticing that a tagged js literal such as `#js []` when placed anywhere within the form of a call to react/useState, breaks the hooks state preservation on hot reload. For example `test-state` is preserved if I make a code change with this component:

(defnc test-component []
  (let [[test-state set-test-state] (react/useState (do [] 1))]
    ($ "button" {:on-click #(set-test-state inc)} (str "test1: " test-state))))
but if I tag the array inside the do form with a #JS like the following, the state of test-state is not preserved if I make a code change:
(defnc test-component []
  (let [[test-state set-test-state] (react/useState (do #js [] 1))]
    ($ "button" {:on-click #(set-test-state inc)} (str "test1: " test-state))))
but any help would be much appreciated, thanks! (the reason for the do form is just to demonstrate that it's not the actual value that's passed to useState) Could this be something to do with the way the defnc macro looks through the body for any use* hooks?

dominicm19:06:52

I wonder if it's because (identical? #js [] #js []) is false

dominicm19:06:22

Oh, no, because it's not using the value as the do detected, yeah

Derek19:06:47

Does it work properly if you use helix.hooks/use-state?

dominicm19:06:35

https://github.com/Lokeh/helix/blob/99406e80c6f10231f88db5be12a969237d712306/src/helix/core.clj#L213-L215 looks like the code does a string join on all your hooks, so it makes sense to me that the (do) is irrelevant somehow. It clearly impacts the key of some kind. Reading through https://github.com/facebook/react/issues/16604#issuecomment-528663101 to see how this is sposed to work.

dominicm20:06:52

https://github.com/facebook/react/blob/a1558600183b4321d4cdc117925980f1865641fa/packages/react-refresh/src/ReactFreshRuntime.js#L644-L647 so the string/join is definitely a key of some kind to the cache. So presumably this is changing unintentionally. Not sure why though. I can't believe a human being parsed all these 700 loc in the babel plugin and understood them enough to translate them into ~20 lines of clojure. Massive hat tip to you! https://github.com/facebook/react/blob/a1558600183b4321d4cdc117925980f1865641fa/packages/react-refresh/src/ReactFreshBabelPlugin.js#L239-L250 is where the work happens. What is done by helix does differ slightly in that: • Hooks are joined with a newline • React uses a name+key system to make up it's key (rather than all the args) • React actually sets customHooks - not relevant in this case but surprised me 🙂 Here's how React determines the key in name+key: https://github.com/facebook/react/blob/a1558600183b4321d4cdc117925980f1865641fa/packages/react-refresh/src/ReactFreshBabelPlugin.js#L404-L418 It doesn't use the args usually, but some kind of id (I think it's the binding variable name? Clever, if so). This id is used even in the special case for useState, but as in this case the key should be less rather than more specific, I think it should be OK. Specifically though, it sets the key to the specific argument of useState, which seems like it should be OK. [Also, this hot refresh thing seems a lot worse than I realized, yikes!] I'm not feeling any wiser for having done this code dive, as by all indications this specific use case should work fine. Nothing about #js seems to alter the key according to:

(require 'clojure.string)
(clojure.string/join
  [
    '(react/useState #js [])
    '(react/useState #js [])
    ])

dominicm20:06:21

Ah, string/join runs in clojure-lang. Derp! Right, I bet it's getting something that looks very different then!

dominicm20:06:54

"hooks-key" "(hooks/use-state (do #object[cljs.tagged_literals.JSValue 0x73ca9fc4 \"cljs.tagged_literals.JSValue@73ca9fc4\"] \"Lisa\"))" That's why! It's printing out the object using the JSValue, which is an object so has a memory reference associated @jahenderson777 @lilactown here's the bug 😄

🎉 4
Alex J Henderson20:06:35

Awesome work, well done @U09LZR36F!

😄 2
dominicm20:06:02

(string/join (map cljs.compiler/emit-constant hooks)) does the trick. But it's pretty verbose. Based on a cursory look around the source, you might only need to solve for regex/JSValue (based on the fact those are both things that broke Cljs' own caching in the past, and are still the only 2 custom ones: https://github.com/clojure/clojurescript/blob/9d3dfc369a01b31244eb925fef4c9fafa3824e24/src/main/clojure/cljs/analyzer.cljc#L94-L103). As far as I can tell, JSValue is the only deftype in a cljc file in ClojureScript, so special casing JSValue probably makes sense.

(string/join
                                    (clojure.walk/postwalk
                                      (fn [x]
                                        (if (instance? JSValue x)
                                          (str "#js" (.val x) "")
                                          x))
                                      hooks))
Also works if you just special case JSValue types.

🙌 2