Fork me on GitHub
#cljs-dev
<
2022-02-09
>
p-himik16:02:15

In this example:

(.-someProp (first #js [#js {:someProp 1}]))
the CLJS compiler loses the ^js tag because of (first ...) and replaces it with #{'any 'clj-nil}. This leads to two issues: ā€¢ -someProp gets renamed during :advanced compilation ā€¢ There's no "Cannot infer target type" warning Is it something expected or is it a bug? If it's expected, how can it be improved so that people become immediately aware of this issue during development?

dnolen16:02:34

it is expected, not a bug

dnolen16:02:09

I don't think there's anything we can do here that would be idiomatic to improve anything

dnolen16:02:09

externs inference intentionally works through interop forms only

dnolen16:02:53

but ClojureScript functions can and will erase stuff because in general - you cannot infer much

dnolen16:02:24

thus explicit type hinting

p-himik16:02:44

Hmm, that's unpleasant. Makes relying on the warning pretty much useless - better to type hint every definition and CLJS form involved in interop anyway. But the analyzer could theoretically infer the tag in this case, no? Although, it would probably have to be a special case for first and all such functions...

dnolen16:02:02

I don't know this is not a common complaint

dnolen16:02:26

your example very contrived

dnolen16:02:57

for example this is not a problem if I'm writing ClojureScript and I'm actually using #js and goog.object

dnolen17:02:39

in fact I cannot really understand it

dnolen17:02:15

because if this was a real interop situation then I would just goog.object

p-himik17:02:00

Well, with goog.object nothing would be a problem indeed, but it's quite cumbersome when you have to do heavy interop. The example is a simplified version of this actual code:

(defn replace-track-source! [^js playlist track-name src]
  (loop [tracks (.-tracks playlist)]
    (when-first [^js track tracks]
      (if (= (.-name track) track-name)
        (-> (to-audio-buffer src playlist)
            (.then (fn [audio-buffer]
                     (let [audio-buffer (audio-util/normalize-volume audio-buffer)]
                       (set! track -src audio-buffer)
                       (.setBuffer track audio-buffer)
                       (.setCues track 0 (.-duration audio-buffer))
                       (.setPlayout track (doto (Playout. (.-ac playlist) audio-buffer)
                                            (.setVolumeGainLevel (.-gain track))
                                            (.setStereoPanValue (.-stereoPan track))))
                       (.calculatePeaks track (.-samplesPerPixel playlist) (.-sampleRate playlist))
                       (.adjustDuration playlist)
                       (.drawRequest playlist)))))
        (recur (next tracks))))))
Notice that ^js track - without ^js all track-related fields would be renamed. Simply because when-first is effectively (let [track (first (seq tracks))] ...).

p-himik17:02:52

With goog.object, any (.. a -b -c -d) becomes incredibly more complex. Any function call becomes much less transparent. So I don't think it's a good solution.

dnolen17:02:08

right so this example is easier for me to understand - less contrived šŸ™‚

dnolen17:02:12

@p-himik we don't infer types of kind T<A>

dnolen17:02:24

that would be necessary here

dnolen17:02:34

because #js [cljs cljs cljs ...] is perfectly valid

p-himik17:02:34

Wouldn't the type of (aget #js [cljs] 0) be inferred as js? If no, than how does (aget #js [js] 0) work, if at all?

p-himik17:02:49

Also you say "we don't". But is it something that would be reasonable to do?

dnolen17:02:56

pretty sure aget is any

dnolen17:02:10

and no, interpreting the contents of arrays and object as ^js is not reasonable

dnolen17:02:34

we need inference for T<A>

dnolen17:02:41

but Clojure does not have type hinting pattern for this

p-himik17:02:47

Nah, I don't mean unconditionally interpreting as ^js. I did indeed mean inference.

dnolen17:02:11

Clojure doesn't support hinting of T<A> because of JVM type erasure

dnolen17:02:19

so there's not an attractive syntactical path

p-himik17:02:25

And I assume you don't want to have special cases for first, second, aget, etc?

dnolen17:02:32

absolutely not

p-himik17:02:59

OK, I'm just probing the ground here because I would definitely like to have a solution to that. Anything that doesn't require me or anybody else to get burned after a production release or to have an :advanced build just for that extra check of every related functionality. Are there any reasonably frequent instances where you would actually want to do JS interop on CLJS data? Maybe an extra [opt-in] warning about dot forms where the tag of the target is clj-any would be a solution?

dnolen17:02:41

@p-himik internally JS interop on CLJS data is how the data structures work

p-himik17:02:48

Well, that's from cljs.core or something like that I presume, so that's a special case. After all, you get a lot of Cannot infer target type warnings from that NS.

dnolen17:02:09

@p-himik or any other library that does this, which after 11 years is going to be more than you think

p-himik17:02:19

Alright, then remove [] around the "opt-in" part. :)

dnolen17:02:50

anyways just not interested in doing anything quickly here

dnolen17:02:30

we do index all externs, but I think our checking of deftype is not so good

dnolen17:02:56

probably the right way to get warning would be to check that property exists at all somewhere

dnolen17:02:38

a patch that does what would be interesting

dnolen17:02:51

because then if you can't find it - you can warn - and you can re-hint ^js

dnolen17:02:34

unfortunately I think it's not as simple as it sounds

dnolen17:02:59

one of the biggest problems with externs-inference was just having to suppress stuff deftype/defrecord

dnolen17:02:13

because that just wasn't relevant work at the time

p-himik17:02:47

Oh, I'm not talking about "do it now", of course not. If that's something that's reasonable, I might even fiddle around and come up with a patch for that opt-in warning - I just don't want to waste time if that's something that isn't worth doing at all. Also, I'm not sure whether it's the CLJS compiler that does that, but shadow-cljs documents :infer-externs :auto - it makes externs-related warnings to appear only for your own files, which is quite handy. Detection of existing properties would be quite tricky and full of false negatives. I see a lot of code where new properties are created by using string concatenation.

dnolen17:02:59

string concatenation of properties in cljs.core ?

dnolen17:02:19

I don't follow

dnolen17:02:32

Google Closure included all externs already - we index the all

dnolen17:02:28

the only that's left is correct collection of cljs.core properties - to know which ones exist

dnolen17:02:23

because I added Google Closure namespace parsing

p-himik17:02:33

No, not in cljs.core - in JS files that I use with CLJS, the sole reason I need so much interop.

dnolen17:02:44

those properties absolutely don't matter

dnolen17:02:53

because if it's not indexed then it must be external

dnolen17:02:32

what I'm saying is

dnolen17:02:49

ClojureScript is close to indexing all properties for anything to be compiled

dnolen17:02:55

regardless of CLJS or GCL

dnolen17:02:02

thus you can know if some property is external

dnolen17:02:33

I'm only talking about stuff like (.-foo ...)

p-himik17:02:25

Ah, so in "probably the right way to get warning would be to check that property exists at all somewhere" you meant the "Cannot infer target type" warning and not the opt-in warning that I suggested? So in your example with (.-foo ...) if that -foo is something that is indexed from CLJS or GCL, then there would be no warning, it would be considered an internal property that can be minified. And if it cannot be found, then it would be considered external and the warning would be issued? What about situations where -foo exists both in CLJS/GCL and in some JS file where it's not supposed to be renamed? A user should definitely be warned in such a case as well, but I'm not sure whether it's feasible.

dnolen17:02:37

in CLJS and GCL -foo is not a thing really - Type.foo

dnolen17:02:12

so it's possible that we could use that approach

dnolen17:02:51

at least for internal data structures I don't think this will be problem because it's written in such a low level way - either the type is know or could be trivial hinted

dnolen17:02:46

though now that you bring it up - the main issue if perfectly legitimate non-interop (deftype) code in some library

dnolen17:02:08

that does (.-foo (first ...)

dnolen17:02:26

because again we don't know about T<A>

p-himik17:02:21

Right. So at least to me (a layman when it comes to compilers, really) that opt-in warning about constructs like (.-foo (first ...)) stills sounds like a reasonable solution.

dnolen17:02:17

it's worth opening a JIRA ticket about the basic problem - stuff like this usually take some time to really think through all the options

p-himik17:02:39

I don't have access right now but I can write the initial version of the issue description if that's helpful.

dnolen18:02:47

@p-himik happy to paste it in if you drop a link to a gist or something

šŸ‘ 1
Alex Miller (Clojure team)18:02:49

You can always add it to ask Clojure too

šŸ‘ 1