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

*cljs-compiler-env* does not get set on setup error #62

Open
johnbendi opened this issue May 8, 2015 · 4 comments
Open

*cljs-compiler-env* does not get set on setup error #62

johnbendi opened this issue May 8, 2015 · 4 comments

Comments

@johnbendi
Copy link

When starting the cljs-repl of piggieback the *cljs-compiler-env* does not get set causing the repl to bootstrap cljs.core on every evaluation. There should be a provision for setup errors so that the env gets reset properly.

The actual error causing it in my case is reported here:
https://groups.google.com/forum/#!topic/clojurescript/ewRpTfSOYq8

Thanks,
John

@cemerick
Copy link
Collaborator

cemerick commented May 9, 2015

Looking at the groups thread, I only see mention of RangeErrors coming out of your JavaScript environment. Anything related to *cljs-compiler-env* would be a Clojure exception; do you have a stack trace of that somewhere?

@johnbendi
Copy link
Author

Yes the error is related to my Javascript environment. But what I wanted you to take note of was that any such error which is happening on first call to cljs-repl will not abort after run-cljs-repl executes in the code below but goes ahead to set! *cljs-repl-env* and *ns* despite *cljs-compiler-env* not been set. Which will still make the cljs-repl prompt appear to users and cause the reloading of cljs.core on every evaluation adding 2-3 seconds because *cls-compiler-env* was not set on the first call. Unless the user is deeply knowledgeadble to know that this is not the intended behavior. So what I am saying is there should be a way to catch the first -evaluation error and prevent cls-repl from executing the rest of its body.

(defn cljs-repl
  "Starts a ClojureScript REPL over top an nREPL session.  Accepts
   all options usually accepted by e.g. cljs.repl/repl."
  [repl-env & {:as options}]
  ;; TODO I think we need a var to set! the compiler environment from the REPL
  ;; environment after each eval
  (try
    (let [repl-env (delegating-repl-env repl-env nil)]
      (set! ana/*cljs-ns* 'cljs.user)
      ;; this will implicitly set! *cljs-compiler-env*
      (run-cljs-repl (assoc ieval/*msg* ::first-cljs-repl true)
        (nrepl/code
          (+ 1)
          ;; (ns cljs.user
          ;;   (:require
          ;;    [clojure.browser.repl]
          ;;    [cljs.repl :refer-macros (source doc find-doc
          ;;                               apropos dir pst)]))
          )
        repl-env nil options)
      ;; (clojure.pprint/pprint (:options @*cljs-compiler-env*))
      (set! *cljs-repl-env* repl-env)
      (set! *cljs-repl-options* options)
      ;; interruptible-eval is in charge of emitting the final :ns response in this context
      (set! *original-clj-ns* *ns*)
      (set! *ns* (find-ns ana/*cljs-ns*))
      (println "To quit, type::" :cljs/quit))
    (catch Exception e
      (set! *cljs-repl-env* nil)
      (throw e))))

I hope the issue is clear enough. This took me days to observe as I was not familiar with the new changes made from 1.6.

@cemerick
Copy link
Collaborator

Okay, I see what you mean now. The fix for this would be to have cljs-repl check that *cljs-compiler-env* is non-nil before continuing on to set the rest of the dynamic environment that piggieback manages, and throw an error instead. Happy to take a PR for that if someone wants to add a check before I get around to it.

Of course, this won't fix the underlying problem (which does not require piggieback to reproduce, IIRC?).

@bbatsov
Copy link
Contributor

bbatsov commented May 15, 2019

Is this still an issue?

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

3 participants