search

tolitius / mount

managing Clojure and ClojureScript app state since (reset)
Eclipse Public License 1.0
1.22k stars 88 forks source link

mount.core reload-prevention for clojure.tools.namespace isn't working #106

Closed krajj7 closed 5 years ago

krajj7 commented 5 years ago

Mount contains code to prevent clojure.tools.namespace from reloading its vars:

https://github.com/tolitius/mount/blob/c5f3e4cdf81d3b632a463a1db19ae3f73c0ad2c9/src/mount/core.cljc#L20-L22

But this is not effective, because ::load in this context expands to :mount.core/load, while clojure.tools.namespace is looking for keywords in its own clojure.tools.namespace.repl namespace (here ::load expands to :clojure.tools.namespace.repl/load)

So the code should be changed to:

(alter-meta! *ns* assoc :clojure.tools.namespace.repl/load false)

This probably isn't a problem for most projects using c.t.n.r/refresh. I accidentally set up a project in a way that it refreshed the libraries including mount, but I'm not even sure how since it usually doesn't do that. For testing it can be done by calling c.t.n.repl/set-refresh-dirs directly on a local path to mount/core.cljc file and then calling c.t.n.repl/refresh-all, which shows whether mount.core is reload-protected or not.

tolitius commented 5 years ago

interesting, yea, definitely makes sense. not sure how it slipped through. want to PR it?

krajj7 commented 5 years ago

OK I will make a PR.

krajj7 commented 5 years ago

I made the PR fixing one occurrence of this problem, but only just noticed it is also present in almost all test namespaces in mount and in other repos as well (eg. mount-up, yurt).

It seems to me that in mount mount.core is the only namespace where reload prevention makes sense, although I'm not sure why it was used in the tests so maybe I'm missing something.

tolitius commented 5 years ago

thanks for the pull request!

yea, I'd need to clean it up from mount-up / yurt and rethink test applications. one reason it is needed in some of the tests was to make sure the fixtures are not wiped when testing for refresh / reload.