search

nrepl / piggieback

nREPL support for ClojureScript REPLs
480 stars 48 forks source link

Establish a binding to `cljs.analyzer/*unchecked-if*`. #97

Closed sinistral closed 6 years ago

sinistral commented 6 years ago

Any attempt to define a protocol (via defprotocol) in a CLJS repl fails with the error:

Can't change/establish root binding of: unchecked-if with set

The same error is not seen when using the "raw" CLJS REPL described in the ClojureScript quickstart. Online references suggest that CIDER/Piggieback is responsible for setting up this binding; see https://groups.google.com/forum/#!msg/clojurescript/__Qf6L40cwU/MP7pTpTFDAAJ

codecov-io commented 6 years ago

Codecov Report

Merging #97 into master will increase coverage by 0.08%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   83.15%   83.24%   +0.08%     
==========================================
  Files           2        2              
  Lines         190      191       +1     
  Branches        9        9              
==========================================
+ Hits          158      159       +1     
  Misses         23       23              
  Partials        9        9
Impacted Files Coverage Δ
src/cider/piggieback.clj 84.15% <100%> (+0.08%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c1a0beb...487c690. Read the comment docs.

bbatsov commented 6 years ago

I'm fine with the change, but @bhauman is the resident cljs expert here.

@bhauman Where do these new bindings keep coming from? Can we just automatically check somehow if new bindings needs to be added?

sinistral commented 6 years ago

I presume that I should at least be defaulting to the current value of the root binding, rather than defaulting to false; or is the @session pattern preferred (I'm unsure of those semantics)?

sinistral commented 6 years ago

Updated to use established pattern, and added test.

bbatsov commented 6 years ago

We copy the root bindings to the session, so you took the right approach IMO. The PR looks good. Thanks!

bbatsov commented 5 years ago

I've just released 0.3.10 which includes your fix. Enjoy!