This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2019-11-07
Channels
- # announcements (37)
- # babashka (28)
- # beginners (104)
- # calva (28)
- # cider (32)
- # clj-kondo (35)
- # cljs-dev (4)
- # cljsrn (3)
- # clojure (35)
- # clojure-conj (4)
- # clojure-dev (57)
- # clojure-europe (4)
- # clojure-france (6)
- # clojure-gamedev (1)
- # clojure-germany (1)
- # clojure-hamburg (2)
- # clojure-italy (7)
- # clojure-nl (4)
- # clojure-spec (9)
- # clojure-uk (11)
- # clojuredesign-podcast (2)
- # clojurescript (36)
- # clojurex (48)
- # core-async (6)
- # cursive (12)
- # data-science (1)
- # datomic (21)
- # defnpodcast (7)
- # duct (1)
- # events (1)
- # fulcro (56)
- # graalvm (30)
- # graphql (5)
- # jobs (1)
- # joker (21)
- # keechma (1)
- # leiningen (4)
- # off-topic (109)
- # parinfer (20)
- # pathom (27)
- # re-frame (4)
- # shadow-cljs (80)
- # spacemacs (18)
- # sql (32)
- # testing (2)
- # tools-deps (32)
- # vim (20)
After thinking about the react refresh problem some more, I think the best thing to try right now is to add the ability to reload all dependent ns
thheller, would you be willing to help me if I tried to implement that in shadow-cljs to try it out?
@lilactown you can "test" this by copying this file https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/cljs/devtools/client/browser.cljs#L148 to your classpath
it did reload the namespaces, but in my simple test it also reloaded React et. al. which broke the app
quick question: what's the :always-load
bit there? is that something I can annotate an ns with?
@lilactown the other case in the and
must remain
so you can add another filter so it filters out all (str/starts-with? (str ns) "module$node_modules")
that will work for my test, but might that break hot loading of npm deps after (re)installation?
but if you checkout the msg
data the handle-build-complete
fn receives you should find everything you could possibly need
don't really know why re-declaring a file plays a role in your setup though. it is going to be too slow for any practical uses anyways
the way react refresh is modeled, your macro outputs a call to a register
function with a stable ID and a component. when you call register
with a new component (not ===
to the last one), it marks it as dirty in the react refresh runtime and tells the react reconciler to re-render it or re-mount it
in the case where you update a namespace with a function or piece of data that components depend on, but isn’t a component itself, the dependent components won’t be refreshed
yes, it is. the current strategy they suggest to people implementing this in webpack, rollup, etc. is: - if a module exports only components use react refresh - if a module exports anything other than a component, reload the page
how does refresh do its thing to re-render c? assuming that hte problem you are trying to solve is that starting from a short circuits since it wasn't changed
because if c
isn’t a component, then the components that use it to implement some behavior aren’t reloaded
it means that if your module contains anything other than components, can’t use hot reloading; reload the whole app!
the problem as I understand it is that doing the regular CLJS reload we have today is losing useState
hook data (and others)
I have the simple case working where when you edit a component, it refreshes correctly using the react-refresh stuff. the “magic” works
the case I’m working on now is how to handle the case where in webpack, rollup, etc. they decide to bail and reload the page
where in the React component tree, you have:
<A>
<B /> <---- uses `moduleC.foo`
</A>
when moduleC gets reloaded, but the module that contains components A and B are not reloaded, then B will not be refreshedso what you are trying to solve is "shouldComponentUpdate" deciding to skip updates?
hmm? but if you render from the root (as regular CLJS does) then it'll eventually reach the new moduleC.foo
?
ah yes, another strategy that doesn’t use react-refresh might be to force a re-render from the root
react-refresh does some other magic that makes it so if you’ve changed a component in a certain way, it will re-mount that component, and it handles hooks in slightly different ways like restarting effects but keeps state
the example I created displays the problem I’m trying to solve:
https://github.com/Lokeh/react-hmr-cljs/
I was actually able to get the behavior I want in that case by annotating the core
ns as :dev/always
I’m not sure it will refresh child components as well, so just marking the top-level ns as :dev/always
might not be enough. that’s why I’m experimenting if I can reload all dependent namespaces
the other option I think is we can do something similar to webpack, which is to apply some heuristic to see if a namespace is “safe” to refresh and if not, remount everything via React.render
that’s more problematic for Clojure because everything is public by default… so just checking to see if everything is a component would probably not work
reloading all namespace is going to be too slow. therefore experimenting with it is a dead end
if the problem is just that the ===
on the component type fails then that is an easy fix
I can’t tell if that is the problem yet. The first problem is you need to call register
with a type and ID for each component you want to refresh, each time you perform a refresh
well I’ve been trying to think of how to refresh only the dependent components. if I refresh every component… it might be easier
if I could detect what components are in the dependency tree of a reloaded ns, without having to actually reload the ns they’re in, then that could improve the speed a lot
seems pretty inconsequential in terms of affecting functionality, but is this warning something im causing? only happens for browser
target, cant figure it out
------ WARNING #4 - -----------------------------------------------------------
Resource: node_modules/buffer/index.js:2
constant module$node_modules$buffer$index assigned a value more than once.
Original definition at externs.shadow.js:3
--------------------------------------------------------------------------------