Bump to cider/piggieback

viesti commented 5 years ago

Allows use with Leiningen 2.9.1.

This would allow upgrading projects that still use lein-figwheel to work with Leiningen 2.9.1, which brings nrepl/nrepl (mentioning the organization here for clarity).

On a related note, upgrading org.clojure/tools.nrepl to nrepl/nrepl as discussed in #718 might also be in order, although I guess the newer nrepl get's bundled in when used with Leiningen 2.9.1.

bhauman commented 5 years ago

The code should currently work with Lein 2.9.1. Does it not? The :dev dependency should not influence use of the plugin.

I should change the :dev dependency so that folks could can work on it. But this string replace patch doesn't take into account the context of the changes. Which already account for using cider/piggieback and also allow the use of earlier versions of piggieback and lein.

But I will check against lein 2.9.1 and verify this.

viesti commented 5 years ago

Thanks for the fast reply! And apologies for the lack of context :/

What set me on this path was encountering the following on an older project that uses cemerick/piggieback "0.2.1" and duct/figwheel-component "0.3.3" while using Leiningen 2.9.1:

[WARNING] No nREPL middleware descriptor in metadata of #'cemerick.piggieback/wrap-cljs-repl, see nrepl.middleware/set-descriptor!
nREPL server started on port 62473 on host 127.0.0.1 - nrepl://127.0.0.1:62473
ERROR: Unhandled REPL handler exception processing message {:id 4f377039-b544-4d1d-87c9-fa62756895f3, :op clone}
java.lang.NullPointerException
    at clojure.core$deref_future.invokeStatic(core.clj:2290)
    at clojure.core$deref.invokeStatic(core.clj:2310)
    at clojure.core$deref.invoke(core.clj:2296)
    at cemerick.piggieback$wrap_cljs_repl$fn__38772.invoke(piggieback.clj:288)
    at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__352.invoke(middleware.clj:22)
    at nrepl.server$handle_STAR_.invokeStatic(server.clj:18)
    at nrepl.server$handle_STAR_.invoke(server.clj:15)
    at nrepl.server$handle$fn__31739.invoke(server.clj:27)
    at clojure.core$binding_conveyor_fn$fn__6772.invoke(core.clj:2020)
    at clojure.lang.AFn.call(AFn.java:18)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

Bumping piggieback to cider/piggieback and nrepl to nrepl/nrepl helped to remove the error (this is where I ran transitively into sidecar). It might be that the com.cemerick/piggieback lineage doesn't work with current nrepl, but I didn't yet find a definite root cause. I guess eventually the most neat thing would be to move this project in question to figwheel-main, but that might require some effort :)

Anyways, thank you! I'll try narrowing the cause, and if I'm off the track, you'r welcome to reject this PR :)

bbatsov commented 5 years ago

Yeah, the old piggieback doesn't work with modern releases of nREPL, but I think @bhauman was making the point that the code as currently structured allows the use of older version of Lein.

I think that probably most people have already migrated off Lein 2.8.1, so it's probably a good idea to drop the conditional checks and simplify the code. That would be a nice follow-up to https://github.com/bhauman/lein-figwheel/commit/2b67fa45bfec4a264b814d43c4cffa00a57d74b6

bhauman / lein-figwheel

Bump to cider/piggieback #730