A real-life example where implements? can potentially fail in minified code on particular JS objects: https://clojurians.slack.com/archives/C03S1L9DN/p1756723252000659
Tried it in my end. The minimized version of thisfn in js->clj produces a bunch of conditions like g.j & 64, and sure enough if you feed {j: ...} that matches one of these conditions, the function throws.
Perhaps msym in implements? should not be minified?
it's too late to change anything here, looking at the history I think people found themselves converting JS values w/ CLJS values in them.
my suggestion would be don't bother w/ js->clj at all - it's really hard for me to come w/ examples that I've seen in projects that I worked on that it was worth the trouble.
it might be worth adding a warning to js->clj about advanced compilation
> it's too late to change anything here
But why? Surely it's not a breaking change to stop renaming an already existing field?
> my suggestion would be don't bother w/ js->clj at all
But the impact goes well beyond that, to any instance?, satisfies? and even, as you mentioned, some goog stuff.
yes I would not consider this a bug, but a bad idea wrt. to js->clj
but for a lot of cases people use js->clj w/o this problem
I have been burned by this before myself, but I think I learned my lesson and stop using js->clj for JSON values I don't control
there's more than just one property we can't rename - three it seems
in general, I like the idea of discouraging people from using it (which has other repercussions) than preventing renaming of protocols which is a bad case of special casing
> wrt. to js->clj
What about all other usages of implements? and satisfies? though?
this is a DCE issue for sure depending on what you're doing to not collapse these, not interested
sorry not DCE, rather but code size
anyways changing stuff because there's one bad API decision is undesirable
Sorry, I still don't quite get it.
Why would the change be undesirable if it has no significant repercussions to anyone but can potentially save hours if not days of work for multiple engineers that have randomly stumbled upon it?
I've seen some weird issues that took me days and ended up seemingly going away on their own, so I'm personally just predisposed against leaving rare but pernicious footguns around. :)
It's not too hard to imagine some code out there that uses walk on some data that can have JS object with the branch fn using implements? or something like that.
Regarding code size - it's a factor of two things:
• How the end result looks like - x.long_field = 1 vs. x[get_field_name()] = 1. If it's the latter, there will be no significant difference. Although I can see from defmethod emit* :deftype that it's the former, and changing this to the latter would probably have an undesirable performance impact
• How well it compresses. AFAICT you yourself mostly care about compressed bundle size. And surely strings like cljs$lang$protocol_mask$partition would be compressed quite well? Could it be something worth experimenting with?
code, maybe users are generating types and protocols, you have no idea what people might be doing
if you can imagine it I guarantee it is being done
js->clj is not good
changing code size is extremely undesirable
those are the 2 principles at play here
> if you can imagine it I guarantee it is being done
That's for sure, but I might be lacking imagination in this department. :) I'm a "boring" kinda guy with my code.
What do you mean by "generating"? Using macros to spit out deftype so that there are thousands of types/protocols/whatever? If so, it would just mean that there are thousands of -cljs$lang$protocol_mask$partition17$ being around, and it is my assumption that the part before the number is very amenable to compression.
> js->clj is not good
Yeah, forget about js->clj, I'm not talking about it at all. Only about the impl details of implements? et al.
> changing code size is extremely undesirable
If code size post-compression remains the same, why would increasing the code size pre-compression be a negative? Are pre-compression sizes relevant at all?
we cannot fix all the cases - this is just papering over a hard fact of advanced compilation.
again js->clj is not good, it was extremely reluctantly added years ago
and the bad idea was "improved" over time
investing in bad ideas is just not a good use of time, thus the preference for warning to stop using this thing
Were implements? and satsifies? bad ideas by themselves?
I can come up with a case where they result in errors in advanced compilation just as easily, without js->clj whatsoever.
Here a proper fix is possible. As long as people don't actually use -cljs$lang$protocol_mask$partition in their own objects.
a proper fix is not possible - I only agree that you could make an assessment that's is all.
but this is just not a good use of time if you think about it some more
there are many, many other generated properties, i.e. for fn dispatch
Is bundle size the only thing that makes not renaming generated property names an improper fix? Or are there other concerns?
if you go from first principles on how CLJS is intended to be used (advanced compilation) it becomes obvious that in general you can not expect to put foreign obj / JSON values into a CLJS program and expect it to work w/o using the right apis (goog.object)
solving the bundle size problem is ClojureScript
there is absolutely nothing wrong w/ us generating properties or whatever
the only thing thing that is wrong is that js->clj is being used for foreign object values which can't possible be advanced compilation aware
fixing things downstream of a misconception does not lead to good place.
> you can not expect to put foreign obj / JSON values into a CLJS program and expect it to work w/o using the right apis (goog.object)
That's an interesting perspective. Definitely not something I'd call obvious or even intuitive.
Especially given that CLJS has some affordances for consuming JS objects of all kinds from CLJS directly, without goog or JS APIs. Like being able to (seq js-iterable), which can also potentially explode if the iterable object has some j field or whatever.
I'm not claiming it is obvious - because now you're talking about something that Closure knows about
Closure knows about programmatic patterns w/ JS object values, it cannot possibly know about JSON data values from some random API
Suppose I include a random JS library from NPM in my build. Does it get passed through GCC so it learns about each and every field in each and every object and avoids using those names for itself?
no it is foreign, all the same issues as js->clj apply - but in this case you have a different tool to help you, externs
(which also means you could probably extern this API that is breaking w/ js->clj to avoid a property collapse collision)
But it's not a part of a public API of the library, it's an internal detail that's supposed to stay minified - why would there be externs for it?
Even if I notice that some of the object prototypes in some library have a field j that conflicts with whatever GCC produces and I add extern for that field so that GCC doesn't use the field name anymore for itself (assuming that's how it works in this case - I don't actually know), that won't solve anything whatsoever. Library 0.0.1 works today, library 0.0.2 pushes a new mifinied dist and my externs become immediately obsolete, even though from the perspective of the library it wasn't a breaking change at all.
this is not argument, this is just a fact
any possible collision w/ advanced compilation you could have imagine is guaranteed to have occurred to someone
what I mean is you are correct - those kinda of problems are a direct result of Google Closure not seeing something
and they occur in practice
this isn't new, and again in the general case, nothing you can really do about other than, care less about bundle size 😉
But we can reduce the collisions - that's my whole point. Currently, we exchange a degree of correctness for some ephemeral bytes that will be compressed away anyway. That could be improved. Even if it doesn't make everything perfect. Seatbelts don't prevent incidents or even eliminate deaths. And they do cost money. But they do reduce the overall harm being done to people.
I don't know, these kinda of reports were more prevalent in the early days
you can go back through the history and hear me complain about js->clj for years
I think externs inference, Transit etc helped so people don't talk about it as much
but if you want to provide evidence that time is well spent here, I'm also not against it.
a typical case I would be concerned about is code gen against a database.
i.e. hundreds of generated types
probably anything greater than 10% code size increase would be considered unnacceptable
compressed against uncompressed would be weighed
I'm just not interested in the effort myself but would seriously consider a good report
We're all in a bubble, we never get full reports on anything.
A random engineer would likely just slap ;; NOTE: Don't use this, use the other parameter instead. This one breaks for some reason onto some commented out code and be done with it. And that's closer to the best end of the best-worst case scenario.
I would not be surprised if the vast majority of Clojure engineers never even touch any Clojure-related social outlets. It's my own bubble of course, but that was the picture in every single place I worked at while I was still an office worker. Maybe like 1 in 20 of people would be discussing things online or reporting issues.
> probably anything greater than 10% code size increase would be considered unnacceptable
> I'm just not interested in the effort myself but would seriously consider a good report
Ah, OK. You've answered a question I've been typing out.
> compressed against uncompressed would be weighed
Wait, why?
Shouldn't it be "vanilla+compressed vs. changed+compressed"?
That’s what I meant.
So let me see if I understand correctly: Arbitrary JS objects fundamentally cannot exist bug-free in a Closure universe because Closure objects have magical properties that may happen to collide with arbitrary JS objects. (Cljs objects are Google Cosure objects) The solution that p-himlk suggests is to reduce these conflicts, which it seems you'd be open to if it doesn't regress file size, but overall not optimistic about. Personally I'm the opposite, I don't care at all about any standard library functions working on arbitrary JS objects, but for a language that prides itself in good interop, I think it's essential to have a safe and reliable way to safely consume JS objects into the cljs universe. I'm sure that people are using js->clj for mixed JS/Cljs objects that are aiui fundamentally a bad idea, but what I would want is essentially the json->clj that p-himlk provided. It seems to be adding a :safe or :pure kwarg to that with a warning would be the right approach to let people work with interop return values in a safe way.
that's not a accurate summary
ClojureScript generates regular old JavaScript, Google Closure advanced compilation is whole program, it assumes it knows everything
Anything it does not know about you must declare an extern
the design of js->clj is flawed since it was designed to consume external JS stuff, but it does not take into account the extern problem
all you need to do is make an extern for the CouchDB SPICE parameters
They are pretty much user generated content, so for now I've just switched to the json->clj implementation that converts them to clojure maps. Maybe this is a more obscure problem than I imagine since most structured APIs could indeed return externed data, and most actual json would better be parsed in clojurescript so the problem I'm having pretty much only occurs when you use a js library that parses arbitrary data
Could you explain what is going on here for someone not intimately familiar with cljs compilation?
I guess for now the workaround is "don's use single character keys in js objects"
Does cljs somehow encode type information as bit flag integer properties?
I think so, yes.
this problem isn't specific to implements? - you could encounter issues like this calling to Google Closure Library etc.
Oh, right... Is it feasible to add all such fields to the externs? Or use whatever other way might exist to avoid renaming of such fields. I wouldn't expect for there to be many such fields (or field generation locations in the case of automatically generated fields).
maybe - but this is just one of the hard facts about advanced compilation.
I don't think people normally complain about this because you're not generally passing random JS objects in to ClojureScript standard library code.
looking at the thread, I'm missing context as to why that's desirable in this case?
#js {:m 10} is not too unlikely to not be a random JS object.
I haven't encountered this issue myself yet, but just anecdotally - I noticed today that GET returns a bunch of JS objects with single-letter keys in them.
Yea the context is that I'm getting JSON from CouchDB that contains SPICE parameters that are often single letter fields, which I call js->clj on for further handling.
Why is js->clj checking for clj types? Maybe there are cases where you have clj inside js types, in which case maybe a feasible workaround is to add a pure? flag in case you're passing a pure JS type without any sequences or colls
For simpler cases where you have plain JSON with zero potential of anything "special", it's trivial to implement your own js->clj that doesn't do any checks except for whether something is a plain JS object or array.
You'd imagine js->clj is the interface to random JS objects, and should be resilient to them in ways that random library code probably isn't
And there are probably already CLJS-compatible libraries that read JSON strings and output CLJS data.
In my case the string to js parsing is happening in JS land (PouchDB)
There might be a way to plug into it. But even if there isn't - as I said, it's trivial to implement on your own.
Of course, it's not really a solution to the js->clj footgun.
(defn json->clj [data {:keys [keywordize?] :as opts}]
(cond
(nil? data)
nil
(js/Array.isArray data)
(mapv #(json->clj % opts) data)
(= (.-constructor data) js/Object)
(into {}
(map (fn [[k v]]
[(if keywordize? (keyword k) k)
(json->clj v opts)]))
(js/Object.entries data))
:else
data))I might honestly just use something like this yeah but fixing the footgun would be nice of course