Fork me on GitHub
#cljs-dev
<
2019-05-05
>
mhuebert11:05:39

After bumping cljs from 1.10.439 to 1.10.520, we are seeing warnings in Maria related to certain calls to implements?, including those that happen during destructuring. Example: https://dev.maria.cloud/gist/c901fbc959e0ca43a903d33a46f637a7?eval=true Does anyone have an idea why this might be the case? I’ve verified that no other changes to Maria are involved (toggling the cljs version is enough to fix it) but haven’t tried creating a more isolated self-host example. Note that the URL above is our dev build, I reverted the prod build to 439.

mhuebert11:05:23

ah, that does look like it could be the issue. will try now

mhuebert11:05:56

it doesn’t appear to be related to let

mhuebert11:05:08

most minimal repro so far is

(def x nil)
(if (implements? IDeref x) true false)

mfikes12:05:39

@mhuebert I'd recommend git bisect

mfikes14:05:11

Cool, so it is predicate-induced, which smells right

mhuebert14:05:39

there is a jump of 2 commits there, the 2nd one i can verify the actual error i experience now, the 1st one broke in a different way

mfikes14:05:50

I wonder if there is a way to write a JIRA that repros it without Maria

mhuebert14:05:24

it looks like there are some basic bootstrap/selfhost tests, i can try modifying one of those

mhuebert14:05:18

hmm odd, when i try those instructions I get Namespace cljs.repl.nashorn does not exist. {:cljs.main/error :invalid-arg}

mfikes14:05:39

@mhuebert You can do this in any REPL, no need for it to be the Nashorn REPL.

mhuebert14:05:14

yeah i just left off the -m arg, and a browser repl opens

mfikes14:05:32

@mhuebert My hunch: When the compiler sees an expression like

(if (implements? IAtom x) ,,,)
it is going to analyze the first thing in that list to see if it is cljs.core/implements?, and Maria might not have the analysis metadata related to the core macros namespace loaded or somesuch. (Something close to the point of that particular analysis step, which is being done in cljs.analyzer/type-check-induced-tag)

mfikes14:05:16

Tracking down the above on the side with @mhuebert At its core is that Maria repros the error if you evaluate implements? (as opposed to indicating that you can't take the value of a macro, and this implies the analysis meta for the core macros namespace isn't available)

mhuebert17:05:17

thanks to @mfikes for helping me track this down. I didn’t realize that to implement a warning handler, one should check for a warning’s :warning-type in the dynamically-bound cljs.analyzer/*cljs-warnings* map. In this case, the warning about implements? was inside a no-warn expression, so it was signalled to be suppressed, but we weren’t checking for that. We still have an issue re: wrong error messages about macros (should be “can’t take the value of a macro” not “undeclared var”) but we can already push this fix and continue using the latest cljs version.

mhuebert18:05:36

turns out our incorrect macro messages were the result of our own error-message wrapping

lilactown21:05:18

found a weird thing: a particular JS library react-apollo returns results without a constructor property

lilactown21:05:22

the results looks like { "rates" [{ "currency": "USD", rate: "1.00"}, ...] } but because it doesn’t have a constructor property, (type results) returns nil

lilactown21:05:24

which means that js->clj does nothing since it uses (identical? (type x) js/Object) to check if it should convert it to a map

richiardiandrea22:05:38

This problem has always driven me nuts. All sorts of "Js objects" can't really get converted with js->clj

lilactown21:05:35

I wonder if there’s a way to handle something like this in js->clj? or if this is even common?

thheller22:05:48

just don't use js->clj. it has many other known quirks.