Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cdorrat opened this issue Jun 1, 2018 · 19 comments
Closed

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

cdorrat opened this issue Jun 1, 2018 · 19 comments

Comments

@cdorrat
Copy link

cdorrat commented Jun 1, 2018

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

(custom-set-variables
 '(clojure-build-tool-files
   (quote
    ("project.clj" "build.boot" "build.gradle" "shadow-cljs.edn"))))
  • open a clojurescript file in a shadow-cljs project (eg. https://github.com/shadow-cljs/examples)
  • run cider-jack-in-clojurescript
  • select 'shadow' for the repl type and and a build from the 'shadow-cljs.edn' file in the projects root ('browser' is common)
  • once the clojurescript repl is up try evaluating some clojurescript

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

dpsutton commented Jun 1, 2018

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

dpsutton commented Jun 1, 2018

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

bbatsov commented Jun 2, 2018

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

@dpsutton
Copy link
Contributor

dpsutton commented Jun 2, 2018

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

dpsutton commented Jun 2, 2018

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

bbatsov commented Jun 2, 2018

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

cdorrat commented Jun 2, 2018

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

achikin commented Jun 2, 2018

I'm seeing this with figwheel repl too

@bbatsov
Copy link
Member

bbatsov commented Jun 3, 2018 via email

@dpsutton
Copy link
Contributor

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

bbatsov commented Jun 12, 2018

@dpsutton see the conversation here.

@dpsutton
Copy link
Contributor

@bbatsov did you forget a link?

@bbatsov
Copy link
Member

bbatsov commented Jun 12, 2018

@dpsutton I sure did. 😆 #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
Copy link
Member

vemv commented Jun 15, 2018

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

vemv commented Jun 15, 2018

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

@bbatsov
Copy link
Member

bbatsov commented Jun 17, 2018

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).

bbatsov added a commit that referenced this issue Jun 21, 2018
That mostly a workaround for cases where the track-state REPL type
detection is not functioning properly (seems right now it's not
working properly for shadow-cljs).
@dpsutton
Copy link
Contributor

dpsutton commented Jul 1, 2018

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

dpsutton commented Jul 1, 2018

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

bbatsov commented Jul 15, 2018

Fixed by clojure-emacs/cider-nrepl#535

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

No branches or pull requests

5 participants