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

Add js global classes to be injected in the vm context #14

Merged
merged 1 commit into from Feb 9, 2014

Conversation

joakin
Copy link
Contributor

@joakin joakin commented Feb 6, 2014

Hi!

I had a problem with some code on the repl calling the node core api
https://groups.google.com/d/msg/clojurescript/znsyTrAyplw/5J_NY0uo8oUJ

It would appear that the JS classes like Error (and I guess it happens the same with Array, Object, etc.) are different in the vm and in the node core context, so when getting results from core apis like fs the constructors aren't the same leading to some problems in clojurescript where those are checked/used (instance? type and the js->clj functions for example).

Example:

(let [fs (js/require "fs")]
  (.readFile fs "asdf asdf"
             (fn [err data]
               (println "err: " err)
               (println "type: "(type err))
               (println "instance?: "(instance? js/Error err)))))
;> err:  #<Error: ENOENT, open 'asdf asdf'>nil
;> type:  #<function Error() { [native code] }>
;> instance?:  false

(The last false should be true?)

A solution proposed by David would be to inject also those classes into the vm's context in here:
https://github.com/bodil/cljs-noderepl/blob/master/cljs-noderepl/src/cljs/repl/node_repl.js#L7

So I've added them to contextProperties:

I've taken into account the line that checks hasOwnProperty, I've tried it on the repl and it seems that even those properties are not listed when printing the object they are actually there (so all good):

ClojureScript:cljs.user> (def ps [  "Array", "ArrayBuffer", "Boolean", "Buffer", "DataView", "Date", "Error",
  "EvalError", "Float32Array", "Float64Array", "Function", "Infinity",
  "Int16Array", "Int32Array", "Int8Array", "NaN" "Number", "Object",
  "RangeError", "ReferenceError", "RegExp", "String", "SyntaxError",
  "TypeError", "URIError", "Uint16Array", "Uint32Array", "Uint8Array",
  "Uint8ClampedArray", "clearInterval", "clearTimeout", "console", "process",
  "require", "setInterval", "setTimeout"])
ClojureScript:cljs.user> (every? #(.hasOwnProperty js/global %) ps)
true

I've tried using checkouts in a local example repo after doing lein install in my fork, but I'm not sure if I'm using the modified version and in the repl I get the same (incorrect) behavior...

I'm a super noob so If you could point me to how to check it myself or check if the behavior is corrected it would be great.

Cheers

@bodil
Copy link
Owner

bodil commented Feb 9, 2014

It works for me, so I'll merge and release.

bodil added a commit that referenced this pull request Feb 9, 2014
Add js global classes to be injected in the vm context
@bodil bodil merged commit 147d566 into bodil:master Feb 9, 2014
@joakin joakin deleted the node_global_classes branch February 9, 2014 16:46
@joakin joakin restored the node_global_classes branch February 9, 2014 16:46
@joakin joakin deleted the node_global_classes branch February 9, 2014 16:46
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

Successfully merging this pull request may close these issues.

None yet

2 participants