ClojureScript

IEncodeClojure only works on same-context Objects

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

The default impl uses (identical? (type x) js/Object), but objects created in another JS context (e.g. another frame) will fail this test, since their constructor is a different js/Object. Thus js->clj is the identity on such objects.

I wonder if there are any related problems anywhere else, e.g. with protocols? This seems to be the only occurrence of js/Object.

Maybe this can be fixed by extending IEncodeClojure to object? I don't immediately see how to do that without incurring the option destructuring overhead recursively.

Activity

Hide
David Nolen added a comment -

I don't think this is something that ClojureScript should try to address at all. To me it seems similar to various classloader issues in JVM land.

Show
David Nolen added a comment - I don't think this is something that ClojureScript should try to address at all. To me it seems similar to various classloader issues in JVM land.
Hide
Tom Jack added a comment -

OK, I don't disagree.

Show
Tom Jack added a comment - OK, I don't disagree.
Hide
Sean Grove added a comment -

This is causing some unpleasantness, and I'm not aware of the classloader issues. Why check (identical? js/object x) instead of (goog.isObject x) on https://github.com/clojure/clojurescript/blob/master/src/cljs/cljs/core.cljs#L6948 ?

Show
Sean Grove added a comment - This is causing some unpleasantness, and I'm not aware of the classloader issues. Why check (identical? js/object x) instead of (goog.isObject x) on https://github.com/clojure/clojurescript/blob/master/src/cljs/cljs/core.cljs#L6948 ?
Hide
Chris Granger added a comment - - edited

this bit me as well and I can't see a downside to using goog.isObject. I agree with David about not going down a rabbit hole here, but I think we can do the "right thing" for free.

EDIT: I spoke too soon. Looks like native objects, like dom nodes would blow up in this scenario. Also, goog.isObject returns true on functions, but that would be easy enough to deal with.

Show
Chris Granger added a comment - - edited this bit me as well and I can't see a downside to using goog.isObject. I agree with David about not going down a rabbit hole here, but I think we can do the "right thing" for free. EDIT: I spoke too soon. Looks like native objects, like dom nodes would blow up in this scenario. Also, goog.isObject returns true on functions, but that would be easy enough to deal with.
Hide
Tom Jack added a comment -

I suppose extending to object would similarly cause trouble for dom nodes etc?

If you can get ahold of the js/Object from another frame, is it possible to extend IEncodeClojure to it?

Show
Tom Jack added a comment - I suppose extending to object would similarly cause trouble for dom nodes etc? If you can get ahold of the js/Object from another frame, is it possible to extend IEncodeClojure to it?
Hide
David Nolen added a comment - - edited

I have some experience with this stuff - it's a rabbit hole. There are many objects that can cross contexts for which we can provide no sensible equality guarantees. If you need to move data between JS contexts then use GClosure and stick to the basic JS data types. I'm inclined to close this one unless someone has a brilliant comprehensive solution that I'm not seeing right now.

Show
David Nolen added a comment - - edited I have some experience with this stuff - it's a rabbit hole. There are many objects that can cross contexts for which we can provide no sensible equality guarantees. If you need to move data between JS contexts then use GClosure and stick to the basic JS data types. I'm inclined to close this one unless someone has a brilliant comprehensive solution that I'm not seeing right now.
Hide
David Nolen added a comment -

This one is tricky. Closing for now.

Show
David Nolen added a comment - This one is tricky. Closing for now.
Hide
Richard Newman added a comment -

I just ran into this.

js->clj works perfectly in a Node environment.

It doesn't work in a Firefox add-on, where an object is created in the add-on code and processed by cljs code loaded through `require`.

This cost me quite a bit of debugging time, because of course it works fine when you test it inside the same source file, and works fine in Node, which is a much simpler JS environment.

JS engine documentation makes it very clear that you should not do comparisons against prototypes, because it's nonsensical:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_context_(e.g._frames_or_windows)

The naïve workaround with the current implementation — to convert a JS object to a cljs object in the calling context – is difficult, because of course the calling context is pure JS (so it doesn't even have a js->clj function!), and if it did, it would be a different cljs, and the same problem would occur elsewhere when handling that cljs object.

Using `(js->clj (.parse js/JSON (.stringify js/JSON o)))` is an expensive alternative, and doesn't work for rich objects (including JS's own Date type).

So much as it's tempting to say "too hard, wontfix", this makes building a non-trivial library in ClojureScript very inconvenient indeed.

Proposed solutions:

  • Provide a js->clj function that assumes that the input is a JS type, and converts it through sane JS-appropriate logic. E.g., it calls Array.isArray(o) to determine whether the input is an array. Object is the final fallback.
  • Adjust the existing js->clj function that inverts its test: checking whether the input is a ClojureScript type first (because of course they're all objects), and otherwise falling through correct JS type determination code.

For the moment I am going to write the former myself, because I don't have any more time to waste.

Show
Richard Newman added a comment - I just ran into this. js->clj works perfectly in a Node environment. It doesn't work in a Firefox add-on, where an object is created in the add-on code and processed by cljs code loaded through `require`. This cost me quite a bit of debugging time, because of course it works fine when you test it inside the same source file, and works fine in Node, which is a much simpler JS environment. JS engine documentation makes it very clear that you should not do comparisons against prototypes, because it's nonsensical: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_context_(e.g._frames_or_windows) The naïve workaround with the current implementation — to convert a JS object to a cljs object in the calling context – is difficult, because of course the calling context is pure JS (so it doesn't even have a js->clj function!), and if it did, it would be a different cljs, and the same problem would occur elsewhere when handling that cljs object. Using `(js->clj (.parse js/JSON (.stringify js/JSON o)))` is an expensive alternative, and doesn't work for rich objects (including JS's own Date type). So much as it's tempting to say "too hard, wontfix", this makes building a non-trivial library in ClojureScript very inconvenient indeed. Proposed solutions:
  • Provide a js->clj function that assumes that the input is a JS type, and converts it through sane JS-appropriate logic. E.g., it calls Array.isArray(o) to determine whether the input is an array. Object is the final fallback.
  • Adjust the existing js->clj function that inverts its test: checking whether the input is a ClojureScript type first (because of course they're all objects), and otherwise falling through correct JS type determination code.
For the moment I am going to write the former myself, because I don't have any more time to waste.
Hide
Richard Newman added a comment -

Here's an implementation that works for my purposes. Feel free to extend or incorporate this into ClojureScript under the EPL, MPL-2.0, or otherwise.

https://gist.github.com/rnewman/0de1869fdd1667f6b4960cd49fa6d813

Show
Richard Newman added a comment - Here's an implementation that works for my purposes. Feel free to extend or incorporate this into ClojureScript under the EPL, MPL-2.0, or otherwise. https://gist.github.com/rnewman/0de1869fdd1667f6b4960cd49fa6d813

People

  • Assignee:
    Unassigned
    Reporter:
    Tom Jack
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: