js->clj breaks on Objects with the key "v"

Question

With advanced compilation enabled, a JS Object with the key "v" causes the (seq? x) clause of js->clj to return true, but since js/Object does not satisfy ISeqable the map on the following line causes an error. I've identified this issue in the circleci frontend codebase. Working on creating a minimal project test case now.

Answer 1

Comment made by: iandavis

Was not able to generate a minimal case. It definitely fails in our repository, but cannot make it fail on a simpler case. Leaving this issue open in case anyone else encounters it, but don't expect any follow up unless there are other reports.

Answer 2

_Comment made by: thheller_

The {{(seq? x)}} does a protocol check which checks if the given object has a marker property for the protocol. In your case that property was most likely renamed to {{x.v}} which then causes a hit.

This was fixed for normal protocols a while back: https://dev.clojure.org/jira/browse/CLJS-1658

But fast-path protocols (ie. ISeq) use a bit check {{(x.cljs$lang$protocol_mask$partition0$ & (64))}} and I assume your value in {{v}} satisfies that check?

Answer 3

Comment made by: dnolen

I believe Thomas's analysis is correct here. I think having the seq? and coll? checks in js->clj was probably ill-considered but changing that will probably introduce a different kind of breakage for a different group of users. You can either provide an externs file for your JS objects and their properties (difficult to do I know if dynamic) or write a simpler custom js->clj which only expects JS values (no protocol checks) and use that instead.

Answer 4

Comment made by: iandavis

Ok, I figured it was something like that. I considered removing the first three cases, or just reversing the order of the js and protocol checks, but I wasn't entirely sure what kind of breakage that might introduce. If we are just using the modified version in our json parser, it should be fine, right? It seems like those protocol checks are only useful if you have clojure intermixed with the json.

Answer 5

Reference: https://clojure.atlassian.net/browse/CLJS-2050 (reported by alex+import)