mfikes / ambly

ClojureScript REPL into embedded JavaScriptCore
http://ambly.fikesfarm.com
Eclipse Public License 1.0
541 stars 21 forks source link

Warn on undeclared Var after :reload #17

Closed mfikes closed 9 years ago

mfikes commented 9 years ago

If I set up a project making use of Ambly, and have lein cljsbuild auto dev running, and define a new symbol in a namespace in my project, say

(defn my-square [x] (* x x))

and then (require 'my.namespace :reload) then my.namespace/my-square works (the square is calculated and printed), but I get an undefined Var warn:

ClojureScript:cljs.user> (my.namespace/my-square 3)
WARNING: Use of undeclared Var my.namespace/my-square at line 1 <cljs repl>
9
mfikes commented 9 years ago

The described misbehavior doesn't occur in the Node REPL.

mfikes commented 9 years ago

In the Node REPL, (ns-interns 'my.namespace) returns a map

{my-square #'my.namespace/my-square}

while the Ambly REPL returns the empty map.

Additionally, the Node REPL spends several seconds processing (require 'my.namespace :reload) while the Ambly REPL returns immediately.

mfikes commented 9 years ago

Breakthrough: I found that if I set the compiler option :target :nodejs then the issue goes away.

What led me to this theory is that the (require ...) special form is implemented in terms of (ns ... (:require ..)) and emit* ns involves a call to load-libs which has a conditional branch for :nodejs.

I haven't tracked down why this makes it work. In fact, this is perplexing because it should lead to a call to nodeGlobalRequire.

mfikes commented 9 years ago

The Ambly REPL Clojure code mimics a lot of the Node.js REPL code and patches goog.require simply because the Node.js REPL did so.

Additionally you can see that the ClojureScript compiler invokes cljs.core/load-file because good.require has been patched.

So that logically explains why we need :target :node: The Ambly REPL is mimicking the Node.js REPL to such high fidelity that it appears to require the same treatment.

mfikes commented 9 years ago

If I comment out the lines in the -setup impl that much with goog.require, then starting up Ambly results in memory being consumed rapidly until the simulator crashes. (Perhaps without the *loaded-libs* tracking, loading occurs in infinite recursion?)

swannodette commented 9 years ago

@mfikes this is definitely a flaw. I think there needs to be another flag beyond :target :nodejs that can take this path.

swannodette commented 9 years ago

@mfikes I took a closer look at this. I do not think that you need to use :nodejs target. That branch of cljs.compiler/load-libs is truly for a Node.js exceptional case for foreign dependencies. I suspect the :reload issue is something simpler.

mfikes commented 9 years ago

Cool. Since I now know what makes the Ambly REPL work (to a certain level), I need to further isolate it and learn why. :)

swannodette commented 9 years ago

@mfikes what happens if you try without the autobuild running in the background?

mfikes commented 9 years ago

@swannodette Bingo.

Furthermore, not only does it work if you turn of the "competing compiler", it is fast compared to the equivalent in the Node.js REPL.

Corroborating your theory: If I introduce a compilation bug (I threw in unbalanced parens in the source), you can see that :reload causes a compilation:

ClojureScript:cljs.user> (require 'hello-ambly.core :reload)
clojure.lang.ExceptionInfo: failed compiling file:/Users/mfikes/Documents/Projects/hello-ambly/src/hello_ambly/core.cljs {:file #<File /Users/mfikes/Documents/Projects/hello-ambly/src/hello_ambly/core.cljs>}
    at clojure.core$ex_info.invoke(core.clj:4403)
... (long stack elided from here)

The question in my mind now is: Was this just a lack of experience on my part, not really understanding the way things are supposed to work? (If so, cool.) Or, is it supposed to also work if you have auto build running in the background? (In which case perhaps I can dig into file timestamps or other potential root causes.)

swannodette commented 9 years ago

@mfikes so the problem is that you have two uncoordinated processes racing to analyze/compile things on disk. I haven't thought about this issue long enough to know whether it's surmountable or not. It definitely seems desirable to be able to drive compilation via REPL and autobuild process at the same time. Alternatively, it also makes sense to me that REPLs can drive autobuild since running two JVM processes for this is overkill. So something else to consider is that REPLs could use the the new cljs.closure/watch feature in some form to deliver this.

mfikes commented 9 years ago

This issue is not Ambly-specific and I just happened to provoke some behavior that is currently not yet defined.

David's written a ticket to define and clarify the semantics of auto-building and how REPLs would interact with that capability: CLJS-1035

mfikes commented 9 years ago

If anyone encounters this, the solution, given CLJS-1035 is to simply add :watch <src-dir> to the REPL options.

(repl/repl* (jsc/repl-env) 
   { ... 
     :watch "src"})