Skip to content

Bump to cider/piggieback#730

Open
viesti wants to merge 1 commit into
bhauman:masterfrom
viesti:master
Open

Bump to cider/piggieback#730
viesti wants to merge 1 commit into
bhauman:masterfrom
viesti:master

Conversation

@viesti
Copy link
Copy Markdown

@viesti viesti commented Mar 23, 2019

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.

Allows use with Leiningen 2.9.1
@bhauman
Copy link
Copy Markdown
Owner

bhauman commented Mar 23, 2019

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
Copy link
Copy Markdown
Author

viesti commented Mar 23, 2019

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
Copy link
Copy Markdown

bbatsov commented Apr 20, 2019

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 2b67fa4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants