clojure-emacs / clj-suitable

ClojureScript "IntelliSense" support for JS objects and their properties/methods. Via figwheel and Emacs CIDER.
MIT License
114 stars 7 forks source link

Shadow compat no longer necessary? #34

Open vemv opened 1 year ago

vemv commented 1 year ago

Given

https://github.com/thheller/shadow-cljs/blob/51b15dd52c74f1c504010f00cb84372bc2696a4d/src/main/shadow/cljs/devtools/server/nrepl_impl.clj#L46-L59

, I believe it's no longer necessary to have shadow-cljs specific code.

bbatsov commented 1 year ago

@vemv Can you elaborate on this? This bit of piggieback interface has existed in Shadow forever. (since before suitable was created)

vemv commented 1 year ago

Hmm, that was unexpected :)

If that bit has always existed, then why is shadow-specific code needed at all?

Maybe that bit doesn't work as intended?

I'll be looking at this, mainly because in cider-nrepl we only use piggieback-based cljs stuff. And some features doesn't appear to work.

So cider-nrepl would have to be fixed some way or another (fix the piggieback compat or introduce suitable-like compat to cider-nrepl code).

bbatsov commented 1 year ago

We don't use anything else just because I didn't want to bother to learn shadow's API and Tomas (the author of Shadow) was kind enough to offer this basic compatibility. :-) In an ideal world we use shadow's own nREPL middleware, as it provides full access to shadow's functionality. I recall not everything was working properly with the piggieback adapter, but I haven't touched the ClojureScript code in cider-nrepl in ages and I've started to forget the details.

bbatsov commented 1 year ago

See https://github.com/thheller/shadow-cljs/issues/62 and https://github.com/thheller/shadow-cljs/issues/561 Those are most useful discussions related to the nREPL support in shadow I could find now.

vemv commented 1 year ago

Thanks much!

I'll also check out https://github.com/thheller/shadow-cljs/blob/51b15dd52c74f1c504010f00cb84372bc2696a4d/src/main/shadow/cljs/devtools/server/nrepl.clj to see what it exactly does.

vemv commented 1 year ago

One thing I've just realised is that cider-nrepl and refactor-nrepl use cider.nrepl.middleware.util.cljs/grab-cljs-env, suitable doesn't

Perhaps using it here would simplify some code.