nbb nrepl-server: can't send expressions from .cljs file

Question

nbb nrepl-server: can't send expressions from .cljs file

borkdude opened this issue 4 years ago · 44 comments

borkdude commented 4 years ago

Expected behavior

I expect to be able to send expressions from a .cljs file in clojurescript-mode to an nbb nrepl server.

Actual behavior

Nothing happens until I activate clojure-mode in a .cljs file.

Steps to reproduce the problem

npm install -g nbb
nbb nrepl-server :port 1338

Then cider-connect from emacs to that server and try to evaluate some expressions from a .cljs file:

foo.cljs:

(defn version []
  js/process.version)

(version)

(require '["fs" :as fs])

(fs/existsSync "README.md")
(fs/existsSync "README2.md")

(require '[clojure.string :as str])

(first (str/split js/process.env.PATH ":"))

Environment & Version information

CIDER version information

Include here the version string displayed when
CIDER's REPL is launched. Here's an example:

;; Connected to nREPL server - nrepl://localhost:1337
;; CIDER 1.2.0snapshot (package: 20210914.1315)

Emacs version

27.2

Operating system

macOS

bbatsov commented 4 years ago

Got it.

vemv commented 2 years ago

Nice!

Answer 1 · 2021-10-02T06:51:26.000Z

Do you have the same problem with cider-connect-cljs? I'm guess "yes" as this still assumes the Clojure nREPL server, but I'm asking just in case.

Answer 2 · 2021-10-02T07:51:19.000Z

With cider-connect-cljs it's the same + the clojure-mode workaround doesn't work there.

Answer 3 · 2021-10-29T06:26:58.000Z

I finally looked at this and the problem is that when you use cider-connect this is actually cider-connect-clj, which sets the connection type automatically to Clojure. Then if you try to evaluate something from a cljs file CIDER is looking for a cljs REPL, which it can't find. That's why switching to clojure-mode fixed this. I'll have to think a bit how to best approach this, but it shouldn't be hard to fix.

I guess we'll have to add some simple detection that we're connected to nbb and just skip the regular cljs init sequence (as for all other REPLs first we started a Clojure connection that gets upgraded to ClojureScript by evaluating a bit of "magic" code). Clearly, that's not need for a native ClojureScript REPL like nbb.

Answer 4 · 2021-10-30T09:59:06.000Z

Thanks for looking into this.

Answer 5 · 2022-01-15T09:17:58.000Z

with cider 318fe6878d8bedf5db9dfa649dedb45d72b2e7ee
I try

run nbb nrepl-server :port 1338
cider-connect-clj
then cider tries to call some classpath functions or something that are not there
falls back to reading from minibuffer

output from nbb server:


Connection accepted
request {:op "clone", :id "1"}
response {"new-session" "a70ec1ab-299e-494f-a6d9-c83c54e36511", "status" ["done"], 
"id" "1"}
request {:op "clone", :id "2"}
response {"new-session" "e4b9cc25-91a2-48b5-bd24-b13dbccc6035", "status" ["done"], 
"id" "2"}
request {:op "describe", :session "a70ec1ab-299e-494f-a6d9-c83c54e36511", :id "3"}
response {"versions" {"nbb-nrepl" {"major" "TODO", "version-string" "TODO"}, "node"
 {"major" "v17", "minor" "3", "incremental" "0", "version-string" "v17.3.0"}}, "aux
" {}, "ops" {"eval" {}, "describe" {}, "clone" {}, "close" {}, "load-file" {}}, "st
atus" ["done"], "id" "3", "session" "a70ec1ab-299e-494f-a6d9-c83c54e36511"}
request {:nrepl.middleware.print/buffer-size 4096, :file "*cider-repl clojure/nbb-d
ropdown:localhost:1338(clj)*", :nrepl.middleware.print/quota 1048576, :nrepl.middle
ware.print/print "cider.nrepl.pprint/pprint", :op "eval", :column 1, :line 3, :id "
4", :code "(clojure.core/apply clojure.core/require clojure.main/repl-requires)", :
inhibit-cider-middleware "true", :nrepl.middleware.print/stream? "1", :nrepl.middle
ware.print/options {:right-margin 70}, :session "a70ec1ab-299e-494f-a6d9-c83c54e365
11"}
response {"err" "Could not resolve symbol: clojure.main/repl-requires\n", "id" "4",
 "session" "a70ec1ab-299e-494f-a6d9-c83c54e36511"}
response {"ex" "#error {:message \"Could not resolve symbol: clojure.main/repl-requ
ires\", :data {:type :sci/error, :line 1, :column 42, :file nil, :phase \"analysis\
"}}", "ns" "user", "status" ["done"], "id" "4", "session" "a70ec1ab-299e-494f-a6d9-
c83c54e36511"}
request {:op "eval", :code "(seq (.split (System/getProperty \"java.class.path\") \
":\"))", :session "e4b9cc25-91a2-48b5-bd24-b13dbccc6035", :id "5"}
response {"err" "Could not resolve symbol: System/getProperty\n", "id" "5", "sessio
n" "e4b9cc25-91a2-48b5-bd24-b13dbccc6035"}
response {"ex" "#error {:message \"Could not resolve symbol: System/getProperty\", 
:data {:type :sci/error, :line 1, :column 15, :file nil, :phase \"analysis\"}}", "n
s" "user", "status" ["done"], "id" "5", "session" "e4b9cc25-91a2-48b5-bd24-b13dbccc
6035"}
request {:op "eval", :code "(seq (.split (System/getProperty \"java.class.path\") \
":\"))", :session "e4b9cc25-91a2-48b5-bd24-b13dbccc6035", :id "6"}
response {"err" "Could not resolve symbol: System/getProperty\n", "id" "6", "sessio
n" "e4b9cc25-91a2-48b5-bd24-b13dbccc6035"}
response {"ex" "#error {:message \"Could not resolve symbol: System/getProperty\", 
:data {:type :sci/error, :line 1, :column 15, :file nil, :phase \"analysis\"}}", "n
s" "user", "status" ["done"], "id" "6", "session" "e4b9cc25-91a2-48b5-bd24-b13dbccc
6035"}

cider-repl contents:

;; Connected to nREPL server - nrepl://localhost:1338
;; CIDER 1.3.0-snapshot (package: 1.3.0-snapshot)
Could not resolve symbol: clojure.main/repl-requires
user>

And here is the stacktrace for the "List expression" read from minibuffer call (which feels like a bug in that situation)

Debugger entered--Lisp error: (minibuffer-quit)
  #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_57>()
  read-minibuffer("Lisp expression: ")
  cider-fallback-eval:classpath()
  cider-classpath-entries()
  #f(compiled-function (system session) "Check if SESSION is a friendly session." #<bytecode -0x649c99c46adde4>)(CIDER ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>))
  apply(#f(compiled-function (system session) "Check if SESSION is a friendly session." #<bytecode -0x649c99c46adde4>) CIDER ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>))
  sesman-friendly-session-p(CIDER ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>))
  #f(compiled-function (ses) #<bytecode 0xa02ea2209b5b91>)(("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>))
  #f(compiled-function (elt) #<bytecode -0x14816b7dbc015a5f>)(("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>))
  mapcar(#f(compiled-function (elt) #<bytecode -0x14816b7dbc015a5f>) (("clojure/support-bot:localhost:38341" #<buffer *cider-repl clojure/support-bot:localhost:38341(clj)*>) ("clojure/support-bot:localhost:7000" #<buffer *cider-repl clojure/support-bot:localhost:7000(clj)*>) ("clojure/aws-play:localhost:46141" #<buffer *cider-repl clojure/aws-play:localhost:46141(clj)*>) ("clojure/support-bot:localhost:44177" #<buffer *cider-repl clojure/support-bot:localhost:44177(clj)*>) ("clojure/support-bot:localhost:45165" #<buffer *cider-repl clojure/support-bot:localhost:45165(clj)*>) ("clojure/reply-bot:localhost:42139" #<buffer *cider-repl clojure/reply-bot:localhost:42139(clj)*>) ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>)))
  #f(compiled-function #'sequence #<bytecode 0x1842db430419e5f4>)(#f(compiled-function (elt) #<bytecode -0x14816b7dbc015a5f>) (("clojure/support-bot:localhost:38341" #<buffer *cider-repl clojure/support-bot:localhost:38341(clj)*>) ("clojure/support-bot:localhost:7000" #<buffer *cider-repl clojure/support-bot:localhost:7000(clj)*>) ("clojure/aws-play:localhost:46141" #<buffer *cider-repl clojure/aws-play:localhost:46141(clj)*>) ("clojure/support-bot:localhost:44177" #<buffer *cider-repl clojure/support-bot:localhost:44177(clj)*>) ("clojure/support-bot:localhost:45165" #<buffer *cider-repl clojure/support-bot:localhost:45165(clj)*>) ("clojure/reply-bot:localhost:42139" #<buffer *cider-repl clojure/reply-bot:localhost:42139(clj)*>) ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>)))
  apply(#f(compiled-function #'sequence #<bytecode 0x1842db430419e5f4>) #f(compiled-function (elt) #<bytecode -0x14816b7dbc015a5f>) (("clojure/support-bot:localhost:38341" #<buffer *cider-repl clojure/support-bot:localhost:38341(clj)*>) ("clojure/support-bot:localhost:7000" #<buffer *cider-repl clojure/support-bot:localhost:7000(clj)*>) ("clojure/aws-play:localhost:46141" #<buffer *cider-repl clojure/aws-play:localhost:46141(clj)*>) ("clojure/support-bot:localhost:44177" #<buffer *cider-repl clojure/support-bot:localhost:44177(clj)*>) ("clojure/support-bot:localhost:45165" #<buffer *cider-repl clojure/support-bot:localhost:45165(clj)*>) ("clojure/reply-bot:localhost:42139" #<buffer *cider-repl clojure/reply-bot:localhost:42139(clj)*>) ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>)) nil)
  seq-map(#f(compiled-function (elt) #<bytecode -0x14816b7dbc015a5f>) (("clojure/support-bot:localhost:38341" #<buffer *cider-repl clojure/support-bot:localhost:38341(clj)*>) ("clojure/support-bot:localhost:7000" #<buffer *cider-repl clojure/support-bot:localhost:7000(clj)*>) ("clojure/aws-play:localhost:46141" #<buffer *cider-repl clojure/aws-play:localhost:46141(clj)*>) ("clojure/support-bot:localhost:44177" #<buffer *cider-repl clojure/support-bot:localhost:44177(clj)*>) ("clojure/support-bot:localhost:45165" #<buffer *cider-repl clojure/support-bot:localhost:45165(clj)*>) ("clojure/reply-bot:localhost:42139" #<buffer *cider-repl clojure/reply-bot:localhost:42139(clj)*>) ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>)))
  seq-filter(#f(compiled-function (ses) #<bytecode 0xa02ea2209b5b91>) (("clojure/support-bot:localhost:38341" #<buffer *cider-repl clojure/support-bot:localhost:38341(clj)*>) ("clojure/support-bot:localhost:7000" #<buffer *cider-repl clojure/support-bot:localhost:7000(clj)*>) ("clojure/aws-play:localhost:46141" #<buffer *cider-repl clojure/aws-play:localhost:46141(clj)*>) ("clojure/support-bot:localhost:44177" #<buffer *cider-repl clojure/support-bot:localhost:44177(clj)*>) ("clojure/support-bot:localhost:45165" #<buffer *cider-repl clojure/support-bot:localhost:45165(clj)*>) ("clojure/reply-bot:localhost:42139" #<buffer *cider-repl clojure/reply-bot:localhost:42139(clj)*>) ("clojure/nbb-dropdown:localhost:1338" #<buffer *cider-repl clojure/nbb-dropdown:localhost:1338(clj)*>)))
  sesman--friendly-sessions(CIDER sort)
  sesman-current-sessions(CIDER (project))
  cider--check-existing-session((:project-dir "~/repos/clojure/nbb-dropdown/" :host "localhost" :port 1338))
  cider-connect-clj(nil)
  funcall-interactively(cider-connect-clj nil)
  command-execute(cider-connect-clj)

Answer 6 · 2022-03-16T14:23:54.000Z

any updates?

Answer 7 · 2022-03-16T16:13:23.000Z

Had no time for this so far. The way things are going it's unlikely that I'll be able to tackle it before April/May. If someone wants to beat me to it - be my guest.

Answer 8 · 2022-04-30T13:07:43.000Z

(It's not just nbb, btw. I also have some "connect directly to a clojurescript REPL" stuff happening that would benefit greatly from being able to evaluate forms while in CLJS mode.)

Answer 9 · 2022-04-30T13:09:13.000Z

@bbatsov I'm probably going to look into this next Monday/Tuesday. Any pointers that will set me on the right track will be appreciated. As cider-connect is an alias for cider-connect-clj, where should we start? I assume we should support cider-connect-cljs instead? Or should we have a cider-connect-generic alternative that doesn't assume anything but just plays by the rules of the nREPL "spec"?

Answer 10 · 2022-04-30T16:40:53.000Z

Looking at the spec and a trace of an nREPL session between cider and a jack'd in JVM, it appears that cider wants a few non-standard ops and falls back to trying to execute JVM-specific code when they're absent. (For example, cider seems to want a stacktrace op, and falls back to calling (clojure.stacktrace/print-cause-trace *e) directly if it isn't available.

Should we discuss improving the protocol a bit?

Answer 11 · 2022-04-30T16:48:30.000Z

I think this isn't a protocol problem, it's a problem of a client that expects the nREPL server to be of a certain implementation (JVM Clojure) rather than expecting it to just talk nREPL and implement the specification. Or maybe it is this way because the protocol isn't rich or specified/documented enough?

Answer 12 · 2022-04-30T16:55:06.000Z

The protocol may not be perfectly suited to purpose. The official documentation lacks a way to explicitly request a stacktrace, for example. It looks like cider is using describe to find out if the server supports cider's extensions, then falls back to the hack of trying to execute some Java code if those extensions aren't there. Presumably this is done to provide the user with a better experience in situations where those extensions/that middleware are absent. 🤷🏻‍♂️

Answer 13 · 2022-04-30T18:42:32.000Z

@jackrusher Yeah, that's more or less the case. In general I've always aimed to have a minimal API for nREPL itself, as it had to be something fairly portable across programming languages.

The protocol may not be perfectly suited to purpose. The official documentation lacks a way to explicitly request a stacktrace, for example.

You receive it upon an evaluation error automatically. There's no way to request it manually.

I assume we should support cider-connect-cljs instead? Or should we have a cider-connect-generic alternative that doesn't assume anything but just plays by the rules of the nREPL "spec"?

I like the generic idea. I guess such a command can prompt for the backend to use and spin the appropriate nREPL server.

The protocol may not be perfectly suited to purpose.

Even now CIDER works decently without any additional middleware. Obviously some features don't work this way, but the core functionality is there. The problem is mostly that in this case CIDER falls back to evaluating Clojure code to provide some features and this obviously is implementation specific. In the early days of CIDER it didn't seem like a problem as back then I never thought they'd be multiple Clojure implementations/dialects.

Answer 14 · 2022-04-30T18:43:39.000Z

What we see in practice is that every editor implementing gets some kind of dropdown menu: connect to clj, cljs, nbb (Calva), babashka (Calva) so the editor knows what to probe. Ideally any editor / nREPL client could talk to any nREPL server in a language agnostic way (like LSP is a language agnostic protocol), but that's not the world we live in. So the information I would like to have from @bbatsov: how should we proceed enabling CIDER to connect to an nREPL server that is implemented purely in CLJS and can eval CLJS (no JVM in between). Should we add bespoke cider-connect-nbb / cider-connect-... targets or cider-connect-generic that is a very general type of connection that only supports minimal stuff? Or should we try to make either cider-connect-clj or cider-connect-cljs more robust, like e.g.:

(when-let [r (requiring-resolve 'clojure.stacktrace/print-cause-trace)] (r *e))

or so, so it will work with the new targets?

Beyond nbb there will be at least two or three new ones running in pure JS:

SCI environment in the browser (scittle, clerk)
VSCode Joyride extension

Answer 15 · 2022-04-30T18:44:59.000Z

@bbatsov Our messages crossed each other, I'm going to read yours now.

Answer 16 · 2022-04-30T18:48:57.000Z

If you grep for "babashka" in CIDER you'll see the kind of changes I had to do to make cider-connect-clj and cider-jack-in-clj work with bb. I guess we can make similar changes to cider-connect-cljs, although as mentioned above I do like the universal value of having some truly "generic" REPL type that makes no assumptions about the runtime. cider-connect-nbb is okay as well by me, as I assume that'd be the easiest to implement.

Answer 17 · 2022-04-30T18:51:18.000Z

@bbatsov I'll experiment with both next week then, thanks.

Answer 18 · 2022-04-30T19:00:10.000Z

@borkdude funny, I was just digging into it and up with this:

(cider-register-cljs-repl-type 'nbb "(+ 42)")

(defun mm/cider-connected-hook ()
  (when (eq 'nbb cider-cljs-repl-type)
    (cider-set-repl-type 'cljs)))
(add-hook 'cider-connected-hook #'mm/cider-connected-hook)

Answer 19 · 2022-04-30T19:01:09.000Z

@benjamin-asdf Interesting, keep going please :)

Answer 20 · 2022-04-30T19:05:20.000Z

both of these pieces are of course a bit hacky.

I would like to support nil for the cljs-repl-type init form (nbb doesn't need to eval anything).
the cljs nrepl buffer does not move from pending to cljs, hence the hook
something about the nrepl err does not register correctly. ~~Maybe you already discussed this with the stacktrace above~~

for 1)

  (unless (or (stringp init-form) (symbolp init-form))
    (user-error "The init form must be a string or a symbol referring to a function"))

it is currently enforced to define a init form

Answer 21 · 2022-04-30T19:08:19.000Z

I, for one, would very much prefer that the protocol be both generic and capable enough that (as the nREPL spec claims) it doesn't make any assumptions about what language/implementation is on the other side of the conversation. My main reason for this is that I would very much not like to implement and maintain a connection function for every nREPL-enable editor/execution context pair.

Answer 22 · 2022-04-30T19:28:13.000Z

Well, as eval gives you a lot you can argue that the core protocol is really quite generic and capable even in it's relatively simple form. 😉 And the more you extend the protocol the higher the burden on those who have to implement it, that's why nREPL went with a core protocol + the ability to extend it when needed.

The problems that people typically experience with CIDER when trying to use it with something other than Clojure have little to do with nREPL and a lot to do with assumptions that CIDER made early on (that'd it'd be used for Clojure only). 10 years ago I didn't think big and I aimed to focus only my immediate problem - build a Clojure IDE. I got interested in nREPL only later and I've started to maintain it a lot later. Making CIDER more generic would be nice, but as the primary focus remain Clojure and friends putting in the time to make it a "well-behaved" nREPL client never seemed worth it. It's on the wishlist, but I doubt it will happen any time soon.

Answer 23 · 2022-04-30T19:30:50.000Z

now I also have a workaround for the errors

(cider-register-cljs-repl-type 'nbb "(+ 1 2 3)")

(defun mm/cider-connected-hook ()
  (when (eq 'nbb cider-cljs-repl-type)
    (setq-local cider-show-error-buffer nil)
    (cider-set-repl-type 'cljs)))

(add-hook 'cider-connected-hook #'mm/cider-connected-hook)

I don't know yet why the default stacktrace buffer does not pop up. But this oldschool overlay is actually ok for me. Because the error message doesn't have a stacktrace anyway.
Maybe it is worth trying to emit eval-error instead of err on the nbb nrepl side. I do not know the semantics.

Answer 24 · 2022-04-30T19:35:44.000Z

A very generic protocol isn't a very useful protocol imo. A protocol should be an agreement between to parties so they can talk in an agnostic way. Relying on eval doesn't make the protocol language agnostic.

Answer 25 · 2022-04-30T19:38:31.000Z

Meanwhile @benjamin-asdf is solving our problems, 👏 👏

Answer 26 · 2022-04-30T19:55:34.000Z

Seems to work well! Thank you @benjamin-asdf !

Answer 27 · 2022-05-01T07:40:08.000Z

Relying on eval not only doesn't make it language agnostic, it fails even to make it implementation agnostic. cider is currently sending my browser-hosted nREPL server extremely JVM-specific evaluation requests, like (seq (.split (System/getProperty "java.class.path") ":")).

Answer 28 · 2022-05-01T08:01:19.000Z

I'm not arguing that's good, I'm just trying to explain how we ended up where we are. Everything's solvable, it's just a matter of some effort and prioritization.

Also - evolving a protocol is a bit trickier than it sounds, mostly because there's always some lag between updates to the protocol, implementations catching up and the versions used by end users catching up. And mistakes are really hard to undo, so all changes have to be considered carefully. :-)

Answer 29 · 2022-05-01T08:07:20.000Z

Btw, feel free to open a separate discussion/ticket about addressing the usage of runtime-specific code. I think it'd be better to focus here on solving the actual issue with nbb.

Answer 30 · 2022-05-03T07:48:04.000Z

New issue for that conversation...
#3193

Answer 31 · 2022-05-03T09:20:31.000Z

In Spacemacs I had to do the following or cider-register-cljs-repl-type would be void and take out my config. 🔫

  (with-eval-after-load 'cider
    (cider-register-cljs-repl-type 'nbb "(+ 1 2 3)")
    )

I don't know why some fns work and others don't , and I don't care but hopefully this helps someone else. 😅

Answer 32 · 2022-05-03T09:26:37.000Z

I don't know why some fns work and others don't

Could you explain what "fns" you mean? nbb fns or cider fns?

Answer 33 · 2022-05-03T09:38:08.000Z

@borkdude Apologies. I meant emacs/spacemacs function calls in the context of the different stages of bootstrapping.

Answer 34 · 2022-05-03T17:12:04.000Z

@dmg46664 probably because spacemacs lazy loads everything so even if you call the function in your user config cider is not loaded yet. Guess your options are the with-eval-after-load form you already have, or loading cider eagerly. Maybe spacemacs has some :config thingy in the layer config for this purpose.

Answer 35 · 2022-05-03T19:48:19.000Z

With @benjamin-asdf's snippet, all works pretty well I must say. So perhaps I should just document that snippet in nbb's repo and we could close this issue? I documented a similar snippet here:

https://github.com/nextjournal/clerk/blob/sci-nrepl/doc/browser-repl.md

It's basically the same for all SCI JS based nREPLs I assume.

Answer 36 · 2022-05-16T18:06:01.000Z

@borkdude you could change the cljs-repl type to "plain-cljs" or something. In the snippet I mean, because it is not special to nbb.

Answer 37 · 2022-05-16T18:08:26.000Z

@benjamin-asdf Exactly! Meanwhile @bbatsov is improving CIDER in such a way that a snippet should maybe not be necessary at all. Not sure how far he is with that.

Answer 38 · 2022-05-17T08:29:02.000Z

I'm on a business trip for the next 10 days, but I'll focus more on the necessary CIDER changes when I'm back.

Answer 39 · 2023-09-28T21:10:44.000Z

Is this issue still valid? I see commits at the bottom of the timeline so I'd guess not

cc/ @benjamin-asdf

Answer 40 · 2023-09-29T07:59:18.000Z

@vemv No I wouild say this is fixed. Cider nbb support is quite complete as far as I am aware.

Answer 41 · 2023-10-01T03:48:13.000Z

nice work all, big achievement here

Answer 42 · 2023-10-02T11:36:04.000Z

Confirmed I needed to remove the snippet. Nice one.