nrepl / weasel

ClojureScript browser REPL using WebSockets
The Unlicense
324 stars 35 forks source link

C-c C-k trigger reload of *all* namespaces #47

Closed the-kenny closed 9 years ago

the-kenny commented 9 years ago

Please note that I write this in a rather bad mood. I'll excuse in advance for unclarities etc.

The recent weasel SNAPSHOT reloads the whole world when I reload any single namespace in my project. This is really really problematic as it wreaks havoc with react for some reason, leading to all kinds of horrible untraceable errors.

Figwheel seem to handle the namespace-reloading much better, but fails spectacularly with other errors.

I'd really like to invest some time into this to finally settle this "api upgrade" we're suffering for weeks now. Please contact me in IRC or here if you have any pointers for me.

swannodette commented 9 years ago

I'm curious as to whether Weasel respects the conditions outlined here?

tomjakubowski commented 9 years ago

@swannodette It does not at the moment - I've left #37 open for that.

I'm very sorry to hear that Weasel's slow upgrade to the new REPL APIs causes so much frustration. Recently I haven't had as much time to put into maintaining Weasel as I'd like. On the other hand, contributors (including you) have done a great job doing work in maintaining Weasel's compatibility with the impressive pace of changes in ClojureScript's compiler and APIs.

I suspect that Weasel is duplicating some analysis work of the compiler + new REPL support, which might cause these kind of horrific bugs. I'll see what I can figure out and I'll ping you when I have more information.

tomjakubowski commented 9 years ago

I added another namespace to the "weasel-example" project in tree to try to debug this. Here are the files I used:

;; example.cljs
(ns weasel-example.example
  (:require [weasel.repl :as repl]
            [weasel-example.foo :as foo]))

(when-not (repl/alive?)
  (repl/connect "ws://localhost:9001"))

(println "Loading example")
;; foo.cljs
(ns weasel-example.foo)

(println "Loading foo")

(def foo 1234)

Here's a log from *nrepl-messages*: https://gist.github.com/tomjakubowski/e4c890b68544bd53fe39

The steps I performed were:

  1. lein cljsbuild auto in the project directory.
  2. M-x cider-jack-in
  3. Piggieback Weasel onto the nREPL session.
  4. Open up a browser with the compiled JS output loaded, let the Weasel REPL client connect.
  5. From the example.cljs buffer, run C-c C-k. That is at this point in the logs. Note that, as you reported, the weasel-example.foo namespace was also reloaded.
  6. From the same buffer, run C-c C-k a few times. This time, however, only the weasel-example.example namespace was reloaded.
  7. Switch to the foo.cljs buffer, and run C-c C-k. Only weasel-example.foo was reloaded.

Not logged, but I did steps 1-6 again, this time trying the initial reload from the foo namespace. Only foo was reloaded the first time, then when I switched to the example namespace, C-c C-k reloaded foo and then example the first time, but only example on subsequent reloads.

So to my untrained eye, it looks like the first time the load-file op is performed on a file, all of its (transitively, presumably) dependent files are reloaded as well.

swannodette commented 9 years ago

Note that prior to the addition of this logic https://github.com/tomjakubowski/weasel/blob/master/src/cljs/weasel/repl.cljs#L85 copied over from the standard Browser REPL, REPLs were not aware of what libraries were loaded on the client. I can see that Weasel is also still using the old way of tracking loaded libs server side, which again, never worked since it did not account for the libraries actually involved in the application, only a preloaded set. Now that we have a standard loaded libs tracking mechanism based on what the client actually has, it's important to confirm that the *loaded-libs* set on the client matches expectations, and if it doesn't determine why.

23min commented 9 years ago

As a newcomer to the clojurescript ecosystem I find the ripple effect of lib versions in the tooling worrisome. I've been pulling my hairs out for days now trying to get browser repl working with boot. Is this the achilles heal of the "small libraries" approach?

tomjakubowski commented 9 years ago

@the-kenny can you try again using either the latest SNAPSHOT I just deployed or a checkout dependency of master?

I've removed quite a bit from Weasel (thanks to the new cljs.repl support), so please let me know as well if I've accidentally introduced new problems to your setup as well. I've confirmed that I can't replicate the duplicate namespace reloading when using C-c C-k from example.cljs.

I should be on IRC the next several hours if you run into issues. If both tjakubo_ and tjakubow are online, ping tjakubo_.

tomjakubowski commented 9 years ago

After IRC discussion + a few days use, @the-kenny reports all is well with this issue.