REPL buffer rejects change to "cljs", sticks out tongue in derision

[jackrusher]

Behavior

I have a project that uses shadow-cljs to which I'd to connect with a single command. I've written a small helper that connects to the REPL, sends the right Clojure code, and then sets the REPL to type "cljs", but the last of those steps fails in a strange way.

When I eval:

(with-current-buffer (cider-default-connection)
    (setq cider-repl-type "cljs"))

... all works as expected. However, when I try to do the same thing within a function that also sends the REPL upgrade code, the buffer switches momentarily to CLJS then back to CLJ. I have tried a series of ever more ridiculous things to force this change, the final one of which was:

(defun maria-connect ()
  (interactive)
  (cider-connect "localhost" "9000") ; doesn't return the buffer in 0.16.0 😢
  (with-current-buffer (cider-default-connection)
    (setq cider-repl-type "cljs")
    (cider-nrepl-send-request
     `("op" "eval"
       "ns" ,(cider-current-ns)
       "code" "(do (in-ns 'shadow.cljs.devtools.api) (repl :live))")
     (nrepl-make-response-handler (current-buffer) ; also tried (cider-default-connection) to be sure
                                  nil
                                  nil
                                  nil
                                  (lambda (buf)
                                    (with-current-buffer buf
                                      (setq cider-repl-type "cljs")
                                      (message cider-repl-type)))))
    (setq cider-repl-type "cljs")))

... which contains three separate calls to set the buffer to "cljs", including one in a callback. 😹 Nonetheless, the buffer switches back to CLJ until I exec a form like the first one, after which it works as expected. I find this quite perplexing

CIDER version information

CIDER 0.16.0 (Riga)

Emacs version

25.1.1

Operating system

OSX 10.13.3

[bbatsov]

Can add here the output from the *nrepl-log-messages* buffer? I'm curious to see what's exactly is going on, as I don't think CIDER should be trying to auto-detect the type of the REPL of eval of something like this.

[bbatsov]

I'm asking because the only way the REPL type can change on eval is via https://github.com/clojure-emacs/cider-nrepl/blob/1168991da4b1c9221246196653372fce5013aeb6/src/cider/nrepl/middleware/track_state.clj#L146

Frankly, I don't even remember why we track it this way, but presumably it's to be able to auto-detect the REPL type, so people don't have to set it manually.

[jackrusher]

Thanks for looking at this so quickly. Indeed, it does appear that the last message is a state update with the repl type set to "clj":

(-->
  id         "8"
  op         "eval"
  session    "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp "2018-03-18 09:11:10.776199000"
  code       "(do (in-ns 'shadow.cljs.devtools.api) (repl :live))"
  ns         "user"
)
(<--
  id            "6"
  session       "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp    "2018-03-18 09:11:10.832556000"
  out-subscribe "51b69748-6d27-48c9-8195-d958b954aa48"
  status        ("done")
)
(<--
  id         "8"
  session    "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp "2018-03-18 09:11:10.833380000"
  out        "To quit, type: :cljs/quit
"
)
(<--
  id         "8"
  session    "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp "2018-03-18 09:11:10.833730000"
  ns         "shadow.cljs.devtools.api"
  value      "[:selected :live]"
)
(<--
  id         "8"
  session    "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp "2018-03-18 09:11:10.833943000"
  status     ("done")
)
(<--
  id                 "8"
  session            "51b69748-6d27-48c9-8195-d958b954aa48"
  time-stamp         "2018-03-18 09:11:11.292888000"
  changed-namespaces (dict ...)
  repl-type          "clj"
  status             ("state")
)

[bbatsov]

Seems this needs to be debugged - https://github.com/clojure-emacs/cider-nrepl/blob/1168991da4b1c9221246196653372fce5013aeb6/src/cider/nrepl/middleware/track_state.clj#L172

Normally it should return the cljs compiler env and indicate the REPL is a cljs REPL but for some reason that's not happening for you.

[jackrusher]

I'll look into it on the shadow-cljs side, thanks. (Also, when do you sleep?!?)

[bbatsov]

If I had to guess I'd say that probably shadow-cljs doesn't interact properly with piggieback, but I'm not familiar with shadow-cljs.

[bbatsov]

I found this ticket https://github.com/thheller/shadow-cljs/issues/62 and I guess we can just ping @thheller for input here. He certainly has the right idea and added exactly what's needed for CIDER to work, but for some reason this doesn't work.

[bbatsov]

I think I have an idea what might be going on. Reading through the ticket I noticed:

as of shadow-cljs@2.0.82 the :middleware for cider is added automatically when cider.nrepl is found on the classpath. It will use the cider.nrepl/cider-middleware var so it should always be up-to-date.

Unfortunately in 0.16 there was a change to that var to make the middlewares loaded on demand https://github.com/clojure-emacs/cider-nrepl/issues/447

My best guess is that this broke the auto-addition of shadow-cljs's middleware, but only @thheller can confirm/deny this.

[bbatsov]

Also, when do you sleep?!?

Only when absolutely necessary. 😉

[thheller]

Hi!

So if I understand this correctly the problem is that the piggieback binding is only available on the next message and this breaks some assumptions by the cider middleware. I can fix that easily.

Can someone tell me however how I can test this in cider to verify that it actually works?

Ideally via some commands I can eval directly in emacs without having to go through tons of setup and stuff?

[thheller]

I just pushed https://github.com/thheller/shadow-cljs/commit/6fecb5b2f47a6b204394f8e387c38d9fbdde7b58 and 2.2.13 which should fix this I think.

[jackrusher]

@thheller sadly, it still reverts on every evaluation

[bbatsov]

So if I understand this correctly the problem is that the piggieback binding is only available on the next message and this breaks some assumptions by the cider middleware. I can fix that easily.

One of the middlewares checks on each evaluation whether the piggieback cljs compiler env is present and based on it sets the REPL type in the client to cljs or clj. I recall that originally we did it like this so when you connect to a remote repl you'd be able to infer its type automatically.

Ideally via some commands I can eval directly in emacs without having to go through tons of setup and stuff?

You just have to use M-x cider-jack-in in Emacs, evaluate in the REPL the code to make it a shadow-cljs repl, invoke M-x cider-set-repl-type and afterwards try to evaluate some cljs code in the REPL.

[bbatsov]

As I said in one of the previous comments I'm wondering if the cljs middleware gets properly loaded in 0.16+. That should be the first thing you need to check.

[bbatsov]

Btw, shadowcljs is supposed to work without piggieback present at all, right?

[thheller]

I only emulate piggieback by "faking" the var binding that cider-nrepl expects.

https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/cljs/devtools/fake_piggieback.clj

Ideally I wish that tools would send different nrepl messages based on which language is active. Currently every middleware has to figure out of you are talking to CLJS or CLJ and then switch accordingly instead of having separate middleware to begin with.

I think it would be much simpler if tools either directly send :lang "cljs" or :lang "clj" as part of the message itself. Or send different ops altogether cljs/eval + eval, cljs/complete + complete, ... This would make writing middleware much simpler I think.

But this probably wouldn't solve the problem we have here. I'll try to figure it out, need to wrap my head around what cider is doing first.

[thheller]

@bbatsov I just add this var to the nREPL middleware stack unmodified. Does CLJS require any addition setup besides me injecting mine + piggieback?

[bbatsov]

Ideally I wish that tools would send different nrepl messages based on which language is active. Currently every middleware has to figure out of you are talking to CLJS or CLJ and then switch accordingly instead of having separate middleware to begin with.

That's a good point and we might very well get to doing it this way down the road. Even now there are different implementations for certain middleware and internally they dispatch on whether the cljs env is present or not, which can certainly be improved.

@bbatsov I just add this var to the nREPL middleware stack unmodified. Does CLJS require any addition setup besides me injecting mine + piggieback?

But how is evaluation going to work this way? Piggieback is just a very simple middleware which overwrites the eval and load-file middleware ops with cljs versions of those ops. If shadowcljs doesn't work via piggieback how are evaluations supposed to work?

[bbatsov]

Ah, just noticed here - https://shadow-cljs.github.io/docs/UsersGuide.html#_nrepl_usage I assume you just opted to override those opts in two separate middlewares looking at their names.

It seems to me that it might be easier (and better) to modify cider-nrepl instead of trying to emulate piggieback's presence, though. I guess the code here can be made more generic https://github.com/clojure-emacs/cider-nrepl/blob/6a3872564d20c54adbf0396f86a67975f019f2fe/src/cider/nrepl/middleware/util/cljs.clj#L52 - e.g. check for piggieback or shadow-cljs.

[thheller]

I'm sorry but emacs makes me feel like I have never used a computer before. I don't understand anything cider is doing or how to get any info about it at all. Maybe its just my setup but I can't even figure out where I would see if cider is in clj or cljs mode.

Until I can figure this out I can only guess at what the issue might be but that is not very productive.

[dpsutton]

@thheller i think @bbatsov is right. We can change cider-nrepl to be aware that the nrepl message is a cljs payload in the way that shadow-cljs currently does. Is there a place in an nrepl message that we can look for the cljs environment the way we currently do? I would hate for you to put random namespaces into your project just for tooling. The tooling should just be aware of how your project works.

[dpsutton]

@jackrusher the reason your repl-type is being reset is because there's a function called on ever "done" message:

(defun cider-repl--state-handler (response)
  "Handle server state contained in RESPONSE."
  (with-demoted-errors "Error in `cider-repl--state-handler': %s"
    (when (member "state" (nrepl-dict-get response "status"))
      (nrepl-dbind-response response (repl-type changed-namespaces)
        (when repl-type
          (setq cider-repl-type repl-type))
        (unless (nrepl-dict-empty-p changed-namespaces)
          (setq cider-repl-ns-cache (nrepl-dict-merge cider-repl-ns-cache changed-namespaces))
          (dolist (b (buffer-list))
            (with-current-buffer b
              ;; Metadata changed, so signatures may have changed too.
              (setq cider-eldoc-last-symbol nil)
              (when (or cider-mode (derived-mode-p 'cider-repl-mode))
                (when-let* ((ns-dict (or (nrepl-dict-get changed-namespaces (cider-current-ns))
                                         (let ((ns-dict (cider-resolve--get-in (cider-current-ns))))
                                           (when (seq-find (lambda (ns) (nrepl-dict-get changed-namespaces ns))
                                                           (nrepl-dict-get ns-dict "aliases"))
                                             ns-dict)))))
                  (cider-refresh-dynamic-font-lock ns-dict))))))))))

If you hijakc that just to debug you can see if everything else will work out

[bbatsov]

@thheller i think @bbatsov is right. We can change cider-nrepl to be aware that the nrepl message is a cljs payload in the way that shadow-cljs currently does. Is there a place in an nrepl message that we can look for the cljs environment the way we currently do? I would hate for you to put random namespaces into your project just for tooling. The tooling should just be aware of how your project works.

There's no such way right now. Because so far we had to deal just with piggieback we're assuming the we have to evaluate something with cljs if some dynamic vars it sets are present (more or less). Technically speaking the only thing we need is the cljs compiler environment as we use it as the basis for a lot of the cljs-related functionality. That's pretty much all there is to it https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/util/cljs.clj#L60

(and a couple of dependencies to the piggieback middleware being present, but those can be changed to something more generic easily as well - https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/util/cljs.clj#L28 and https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/util/cljs.clj#L34)

I'm sorry but emacs makes me feel like I have never used a computer before. I don't understand anything cider is doing or how to get any info about it at all. Maybe its just my setup but I can't even figure out where I would see if cider is in clj or cljs mode.

@thheller I think the cider-nrepl part is more important and there's nothing Emacs specific about it. If the evaluation works with any nREPL client when cider-nrepl is active it should work with CIDER as well. The trick, of course, to trigger a few middleware ops to see that they are returning something meaningful for cljs (e.g. code completion).

I'd love to help you setup some CIDER environment on your end, though, as I think it's important to make sure shadow-cljs works well with CIDER. Seems like a very cool project and I'd love to play a bit with it as well!

[dpsutton]

it looks like we can find shadow.cljs.devtools.api/*nrepl-cljs* to find out if we are evaling in a cljs context. This is unfortunately just the build-id I'm still looking how to get the compiler environment. However, the specific issue in this thread is to prevent the state handler from resetting the repl type. so even if we can't find the compiler environment we can still be aware if we are in a cljs context. with (some? shadow.cljs.devtools.api/*nrepl-cljs*)

@bbatsov I was reading the cider-nrepl code and it doesn't seem really tied to piggieback. as far as i can tell, we just use piggieback here only in the sense that it tells us where to look in the nrepl message to find the compiler environment. that's all i meant to thheller was don't imitate piggieback, just let us know where to get the compiler environment.

(defn- grab-cljs-env*
  [msg]
  (let [path (cljs-env-path)]
    (some-> msg
            :session
            maybe-deref
            (get-in path))))

[bbatsov]

Yeah, that's completely true. That needs to be changed together with the middleware expectations - there it will has to become piggieback or shadow-cljs.

[thheller]

Please don't look at any shadow-cljs specific stuff in cider-nrepl. I would much prefer to hook into a generic API and have tools not worry about "who" they are talking to. We should be able to abstract this properly.

I'm fine with piggieback being this API, it shouldn't be too hard to figure out what piggieback is doing that I'm not.

Maybe we could simplify and just add additional fields to the nrepl msg set by the middleware.

(defn- grab-cljs-env*
  [msg]
  (let [path (cljs-env-path)]
    (or (:cemerick.piggieback/compiler-env msg)
        (some-> msg
          :session
          maybe-deref
          (get-in path)))))

The piggieback middleware already is configured to run before any of the other middlewares so modifying the message before passing it along seems like a reasonable thing to do. As I understand piggieback is being rewritten anyways.

My strategy of fake-piggieback certainly is not ideal and I could alternatively just implement something so (cemerick.piggieback/cljs-repl <something>) just works. As I understand it <something> is already configurable somewhere in cider (and vim-fireplace for that matter). I just didn't do this because piggieback does a bunch of hacks which simply aren't necessary in shadow-cljs.

@bbatsov I recently migrated to Windows so my emacs setup consists of downloading the Windows package plus a spacemacs checkout. It asked to install cider when I opened a .cljs file so I did. No other config beyond that.

[thheller]

I think I figured it out. Just needed to tweak the bindings a bit. I tried only via cider-connect because I can't seem to get cider-jack-in to work but I believe that doesn't make a difference when it comes to the REPL itself.

@jackrusher please try 2.2.15

[bbatsov]

The piggieback middleware already is configured to run before any of the other middlewares so modifying the message before passing it along seems like a reasonable thing to do. As I understand piggieback is being rewritten anyways.

With @bhauman we took it over, but there are no plans for major updates. At least not from my side, as my knowledge of ClojureScript is pretty basic and I can't contribute much meaningful input there.

My strategy of fake-piggieback certainly is not ideal and I could alternatively just implement something so (cemerick.piggieback/cljs-repl ) just works. As I understand it is already configurable somewhere in cider (and vim-fireplace for that matter). I just didn't do this because piggieback does a bunch of hacks which simply aren't necessary in shadow-cljs.

What's the problem with you implementing the real API and we just trimming what's not needed for shadow-cljs? We have complete control over piggieback now, so obviously we can change it however we want to. I'm not just certain what's problematic for you there right now.

[bbatsov]

I think I figured it out. Just needed to tweak the bindings a bit. I tried only via cider-connect because I can't seem to get cider-jack-in to work but I believe that doesn't make a difference when it comes to the REPL itself.

Yeah, there's no real difference. jack-in just starts some nREPL server and connects to it. Btw, what was the problem in the end?

[jackrusher]

@thheller 2.2.15 works :)

I leave it up to you two as to when you'd prefer to close this ticket, given that you're still discussing architectural issues on this thread.

[bhauman]

I concur that piggieback is not slated for a rewrite, just some updating and some cleaning.

[thheller]

@bbatsov I did not use the standard piggieback since the implementation was pretty ridiculous. It used to append :cljs/quit to all code strings it got to eval. So it would basically launch the REPL and immediately exit after for every string it got. I understand why it worked this way and its better now but I just wanted to avoid this hacky stuff when I could do it clean instead.

Besides that there are a few architectural differences in how shadow-cljs operates which just make it tedious to hook into the cljs.repl stuff.

I might reconsider when piggieback is done cleaning.

[dpsutton]

well if there are thoughts between @bhauman @thheller and @cursive-ide have thoughts on how to make piggieback better for editors now would be a good time. just important that all parties are aware of any proposals of change before they happen

[bbatsov]

@thheller Btw, didn't @bhauman solve this already https://github.com/clojure-emacs/piggieback/commit/52941231086cccab037069360bbb1c6af673a711 ?

I guess we just need to cut a new release. Anyways, I agree that some cleanup/improvements are needed, and as I said right now it's very easy to propose and implement those.

[thheller]

As a middleware implementor the one thing I would suggest is to stop piggybacking on Clojure middleware ops and instead use dedicated CLJS ops. So every op basically gets a cljs/<op> equivalent where it makes sense (e.g. eval + cljs/eval).

BUT I have never implemented the Editor-side of things so I have no idea whether this makes sense from that side at all.

[bhauman]

Piggieback was designed to work with existing nREPL clients with the existing nrepl messages. Doing something different than that would simply be different middleware.

On Wed, Mar 21, 2018 at 4:39 PM, Thomas Heller notifications@github.com wrote:

As a middleware implementor the one thing I would suggest is to stop piggybacking on Clojure middleware ops and instead use dedicated CLJS ops. So every op basically gets a cljs/ equivalent where it makes sense (e.g. eval + cljs/eval).

BUT I have never implemented the Editor-side of things so I have no idea whether this makes sense from that side at all.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clojure-emacs/cider/issues/2239#issuecomment-375087761, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQOY5pLOtF8Gi1AeFImd-XCv7b-siks5tgrqBgaJpZM4Su2vL .

[bbatsov]

I guess we can close this ticket and continue the other part of the conversation under tickets for specific improvements of piggieback or the shadow-cljs integration in CIDER (which is currently being brewed).