clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.54k stars 646 forks source link

shadow-cljs repl has incorrect cider-repl-type #2305

Closed cdorrat closed 6 years ago

cdorrat commented 6 years ago

Expected behavior

When a shadow-cljs clojurescript repl is launched with cider-jack-in-clojurescript (selecting "shadow" as the repl type) the value of the cider-repl-type variable should be "cljs" so cider-eval and friends work.

Actual behavior

The clojurescript repl starts correctly but the value of cider-repl-type is "clj" and attempts to evaluate clojure script code give an error like

user-error: ‘cider-eval-defun-at-point’ needs a ClojureScript REPL.

Running (cider-repl-set-type "cljs") with emacs in the clojurescript buffer fixes the problem

Steps to reproduce the problem

Environment & Version information

CIDER version information

;; CIDER 0.18.0snapshot (package: 20180531.823), nREPL 0.2.13
;; Clojure 1.9.0, Java 1.8.0_111
;

shadow-cljs version

cli version: 2.3.32 node: v6.9.2

Emacs version

GNU Emacs 25.1.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version 10.9.5 (Build 13F1911)) of 2016-09-21

Operating system

OSX 10.13.4 "High SIerra"

dpsutton commented 6 years ago

I'm seeing the same behavior on a figwheel project.

(-->
  id         "9"
  op         "eval"
  session    "18a726d7-5cbf-404b-ab90-69290504e04c"
  time-stamp "2018-06-01 12:23:17.960307755"
  code       "(do (require 'figwheel-sidecar.repl-api) (figwheel-sidecar.r..."
  ns         "breeze.jib.server.core"
)
....

(<--
  id         "9"
  session    "18a726d7-5cbf-404b-ab90-69290504e04c"
  time-stamp "2018-06-01 12:23:42.417416365"
  ns         "cljs.user"
  value      "nil"
)
(<--
  id         "9"
  session    "18a726d7-5cbf-404b-ab90-69290504e04c"
  time-stamp "2018-06-01 12:23:42.422806302"
  status     ("done")
)

(<--
  id                 "9"
  session            "18a726d7-5cbf-404b-ab90-69290504e04c"
  time-stamp         "2018-06-01 12:23:42.741229845"
  changed-namespaces .....
  repl-type          "clj"
  status             ("state")
)

notice that the cljs jack in returns repl-type "clj". That poisons everything.

dpsutton commented 6 years ago
Starting nREPL server via lein with-profile dev update-in :dependencies conj \[org.clojure/tools.nrepl\ \"0.2.13\"\ \:exclusions\ \[org.clojure/clojure\]\] -- update-in :dependencies conj \[cider/piggieback\ \"0.3.5\"\] -- update-in :plugins conj \[cider/cider-nrepl\ \"0.18.0-SNAPSHOT\"\] -- repl :headless :host ::...

results in

(<--
  id                 "9"
  session            "e9b08605-e889-47b1-b5e6-c9d1db14ead9"
  time-stamp         "2018-06-01 14:24:55.973673364"
  changed-namespaces (dict ...)
  repl-type          "clj"
  status             ("state")
)

I can prevent this behavior by using cemerick/piggieback 0.2.2. When i inhibit CIDER from injecting cider/pigggieback this problem goes away.

bbatsov commented 6 years ago

@dpsutton Are you using the latest version of Figwheel, btw?

dpsutton commented 6 years ago

ah good point. no this was 0.5.14 and current version is 0.5.16. I'm still trying to chase down why we can't get a cljs repl up and running with our project with cider/piggieback 0.3.5 and 0.5.16 figwheel but it does work with cemerick/piggieback 0.2.2 and figwheel 0.5.14

dpsutton commented 6 years ago

its also possible that this clj status is because both cider/piggieback and cemerick/piggieback are both present now that cider unconditionally injects cider/piggieback 0.3.5

bbatsov commented 6 years ago

0.5.16 is the first Figwheel that works with cider/piggieback, so one mystery was solved.

its also possible that this clj status is because both cider/piggieback and cemerick/piggieback are both present now that cider unconditionally injects cider/piggieback 0.3.5

Maybe. Having both deps around would make the decision which one to use somewhat arbitrary, as it would depend on the order in which cider-nrepl and fidwheel are checking for their presence.

cdorrat commented 6 years ago

With nrepl message logging enabled I can see that we're getting back repl-type "clj" in one of the messages, perhaps I should be looking for the issue on the shadow-cljs side.

The set of relevant messages was:

(-->
  id         "9"
  op         "eval"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:02:58.887857000"
  code       "(do (require '[shadow.cljs.devtools.api :as shadow]) (shadow/watch :browser) (shadow/nrepl-select :browser))"
  ns         "shadow.user"
)
(<--
  id            "7"
  session       "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp    "2018-06-02 15:02:59.014164000"
  out-subscribe "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  status        ("done")
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:02:59.253124000"
  err        "[:browser] Configuring build.
"
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:02:59.401340000"
  err        "[:browser] Compiling ...
"
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:03:09.440671000"
  err        "[:browser] Build completed. (1272 files, 1 compiled, 0 warni..."
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:03:09.459384000"
  out        "To quit, type: :cljs/quit
"
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:03:09.460475000"
  ns         "cljs.user"
  value      "[:selected :browser]"
)
(<--
  id         "9"
  session    "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp "2018-06-02 15:03:09.462321000"
  status     ("done")
)
(<--
  id                 "9"
  session            "82e13b10-7fed-4c78-be52-1fa237ea2ae1"
  time-stamp         "2018-06-02 15:03:10.879661000"
  changed-namespaces (dict ...)
  repl-type          "clj"
  status             ("state")
)
achikin commented 6 years ago

I'm seeing this with figwheel repl too

bbatsov commented 6 years ago

For the latest version?

On Sun, 3 Jun 2018 at 5:14, Anton Chikin notifications@github.com wrote:

I'm seeing this with figwheel repl too

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/clojure-emacs/cider/issues/2305#issuecomment-394114143, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGVyogURa0gp0uY-uSm0bceo8F5fMYTks5t4vIvgaJpZM4UWZHf .

-- Best Regards, Bozhidar Batsov

http://www.batsov.com

dpsutton commented 6 years ago

I'm using figwheel 0.5.16 with cemerick/piggieback 0.2.2 (cider/piggieback does not work in our application). I am seeing the same behavior

(-->
  id           "41"
  op           "eval"
  session      "b58001e9-0a82-40be-a74f-bd01993c8947"
  time-stamp   "2018-06-12 11:37:24.710780917"
  code         "(def modifier-ep \"CPT/EP\")"
  column       12
  content-type "true"
  file         "*cider-repl jib(cljs)*"
  line         89
  ns           "cljs.user"
)
(<--
  id         "41"
  session    "b58001e9-0a82-40be-a74f-bd01993c8947"
  time-stamp "2018-06-12 11:37:25.147042596"
  ns         "cljs.user"
  value      "#'cljs.user/modifier-ep"
)

(<--
  id         "41"
  session    "b58001e9-0a82-40be-a74f-bd01993c8947"
  time-stamp "2018-06-12 11:37:25.183620636"
  status     ("done")
)
(<--
  id                 "41"
  session            "b58001e9-0a82-40be-a74f-bd01993c8947"
  time-stamp         "2018-06-12 11:37:25.186160074"
  changed-namespaces (dict)
  repl-type          "clj"
  status             ("state")
)
bbatsov commented 6 years ago

@dpsutton see the conversation here.

dpsutton commented 6 years ago

@bbatsov did you forget a link?

bbatsov commented 6 years ago

@dpsutton I sure did. 😆 https://github.com/clojure-emacs/cider/pull/2316

Basically you should just debug the middleware check and see what's going wrong there. Likely the fix is pretty simple, but I don't have time for this until next week. And we'll likely add this workaround to be able to control the REPL-type only manually just to be on the safe side.

vemv commented 6 years ago

now that cider unconditionally injects cider/piggieback 0.3.5

Would it be possible to prevent this behavior soon?

I'm getting issues under both CIDER 0.1.7 (stable) and 0.1.8-SNAPSHOT. It would help if I could use a known-good setup (like com.cemerick/piggieback 0.2.2) or something very close to that.

vemv commented 6 years ago

(message above edited, you might have received an empty-looking email)

bbatsov commented 6 years ago

I'm getting issues under both CIDER 0.1.7 (stable) and 0.1.8-SNAPSHOT. It would help if I could use a known-good setup (like com.cemerick/piggieback 0.2.2) or something very close to that.

I'm reasonably sure the issue is something to do with the middleware check, not with piggieback itself (after all shadow doesn't even use it).

dpsutton commented 6 years ago

I think i've figured out why but the obvious fix breaks things.

So shadow-cljs was nice enough to mimic piggieback's location of the cljs compiler environment for us:

          (when-let [pvar (find-var 'cemerick.piggieback/*cljs-compiler-env*)]
            (.set pvar
              (reify
                clojure.lang.IDeref
                (deref [_]
                  (when-let [worker (get-worker id)]
                    (-> worker :state-ref deref :build-state :compiler-env))))))

Because we check for this comiler-env in cider-nrepl to see if we are in cljs land:

(defn- cljs-env-path
  "Returns the path in the session map for the ClojureScript compiler
  environment used by piggieback."
  []
  [(if cider-piggieback?
     (resolve 'cider.piggieback/*cljs-compiler-env*)
     (resolve 'cemerick.piggieback/*cljs-compiler-env*))])

but the problem is that CIDER automatically injects cider.piggieback 0.3.5 in every clojurescript project now. So ciderpiggieback is resolved and that var is nil since piggieback isn't involved at all (only the fake version provided by shadow-cljs).

I am trying the simple fix of replacing cider-piggieback? with cemerick-piggieback? which should work but when I lein install CIDER it works for clj projects, figwheel project for work, but not for the shadow-cljs project.

But since we have injected cider/piggieback that test will always return the cider/path.

dpsutton commented 6 years ago

also note that if i lein install cider-nrepl with 0 changes i get the same behavior:

error in process filter: ‘cider-connect-sibling-cljs’ requires the nREPL op "classpath". Please, install (or update) cider-nrepl 0.18.0-SNAPSHOT and restart CIDER

(-->
  id         "3"
  op         "describe"
  session    "d51070db-b94a-44a6-aa52-4e3972949ecf"
  time-stamp "2018-06-30 23:25:49.857913501"
)
(<--
  id         "3"
  session    "d51070db-b94a-44a6-aa52-4e3972949ecf"
  time-stamp "2018-06-30 23:25:49.866874251"
  aux        (dict ...)
  ops        (dict
               cljs/select (dict)
               clone       (dict)
               close       (dict)
               describe    (dict)
               eval        (dict)
               interrupt   (dict)
               load-file   (dict)
               ls-sessions (dict)
               stdin       (dict))
  status     ("done")
  versions   (dict ...)
)

and further down in the messages error messages not recognizing the ops:


(<--
  id         "7"
  op         "out-subscribe"
  session    "d51070db-b94a-44a6-aa52-4e3972949ecf"
  time-stamp "2018-06-30 23:25:50.103084993"
  status     ("done" "unknown-op" "error")
)
(<--
  id         "8"
  op         "init-debugger"
  session    "d51070db-b94a-44a6-aa52-4e3972949ecf"
  time-stamp "2018-06-30 23:25:50.104838033"
  status     ("done" "unknown-op" "error")
)
bbatsov commented 6 years ago

Fixed by https://github.com/clojure-emacs/cider-nrepl/pull/535