Fork me on GitHub
#shadow-cljs
<
2019-11-01
>
p-himik17:11:26

I started writing this post as a question but it ended up as just a rant about JS and an appreciation for shadow-cljs. 🙂 I was trying to help a friend that uses TypeScript with a plotting library that I use in CLJS called BokehJS. Since I don't include BokehJS in my build at all (I just put some CDN links into <script>s and use the created global object), I have never had any problems with it. The thing is, BokehJS relies on another library called FlatBush and requires it like this: - TS: import FlatBush = require("flatbush") - Distributed JS: var FlatBush = require("flatbush") - Compiled JS: var FlatBush = __webpack_require__(/*! flatbush */ "./node_modules/flatbush/index.js"); - Compiled JS in CDN: var FlatBush = require(394) On the other hand, FlatBush uses: - Source JS: export default class Flatbush - Distributed JS: a bunch of boilerplate that tries to use module.exports, define, and just global - Compiled JS: __webpack_require__.d(__webpack_exports__, "default", function() { return Flatbush; }); - Distributed JS in BokehJS CDN: same boilerplate as in just distributed JS. Because of this difference, it's easy to use BokehJS as a CDN library but not that easy when you want to have it in your build. Namely, it results in TypeError: FlatBush is not a constructor because __webpack_require__(/*! flatbush */ "./node_modules/flatbush/index.js") returns the whole module, which is an object with a field default. We spent a few hours on this. Apparently, one must use a Babel plugin called add-module-exports in such situations, but it also proves to be not an easy task for all sorts of reasons. Finally, I decided to give it a go in shadow-cljs, expecting to come up with a few new questions. And it... just worked? Heh. Awesome, thank you! BTW would it work in vanilla CLJS, without shadow-cljs?

Filipe Silva19:11:31

vanilla js doesn't have a module loader

Filipe Silva19:11:14

this kinda boils down to different handling of commonjs/import interop, especially in the case of namespace imports

coder4633418:11:16

Hi! When I try to evaluate a file with calva or cider, I get an error like this

coder4633418:11:16

Evaluating file: nodemap2.cljs
symbol re-frame2.items.nodemap2 already provided by [:shadow.cljs.repl/resource "re_frame2/items/nodemap2.cljs"], conflict with [:shadow.build.classpath/resource "re_frame2/items/nodemap2.cljs"]
{:provide re-frame2.items.nodemap2, :conflict [:shadow.cljs.repl/resource "re_frame2/items/nodemap2.cljs"], :resource-id [:shadow.build.classpath/resource "re_frame2/items/nodemap2.cljs"]}
ExceptionInfo: symbol re-frame2.items.nodemap2 already provided by [:shadow.cljs.repl/resource "re_frame2/items/nodemap2.cljs"], conflict with [:shadow.build.classpath/resource "re_frame2/items/nodemap2.cljs"]
	shadow.build.data/add-provide (data.clj:91)

coder4633418:11:29

I couldn't find anything on google, any hints?

thheller19:11:11

@p-himik I don't quite follow what you described? just including a bunch of stuff via <script> tags will work with vanilla CLJS yes?

p-himik21:11:53

I meant that including a fully-fledged NPM module has worked with shadow-cljs, despite that a plain JS+Webpack project has some serious issues with, if I understand correctly, combining CommonJS and ES6 modules. Would the same work with vanilla CLJS?

thheller21:11:18

vanilla CLJS doens't support npm at all

thheller21:11:49

that is generating output that only runs in node

thheller21:11:03

(it doesn't actually process anything from npm)

p-himik22:11:52

Oh, I see. Thanks!

thheller22:11:40

:npm-deps tries to but is basically never works

thheller19:11:37

@publicz hmm that looks like the namespace was first defined in the REPL and than loaded via the filesystem. I thought I fixed that a while ago though? are you an on older version maybe?

Nolan20:11:39

does anyone have any recommendations on externs debugging? ive worked through a few in the past, and generate-extern/the webclient always seem to more or less do the trick, but ive encountered a new one that seems like it should be straightforward, but is failing spectacularly. whats strange to me is that its failing at b.createReactClass. reagent/react was obviously working just fine prior to adding the new library (which is ES6, no less!), so its throwing me off that it seems like that would be any problem. for some detail:

;; imports look like:
(ns some.ns
  (:require ,,,
    ["gsap/TextPlugin" :refer [TextPlugin]] ;; these are coming from npm/node_modules
    ["gsap" :refer [gsap]]))

;; auto-generated externs look like:
var gsap = {
    // ... long, seemingly correct list
}

;; node_modules/gsap/dist/gsap.js module looks like:
// ...
exports.default = gsap;
exports.gsap = gsap;

Object.defineProperty(exports, '__esModule', { value: true });
let me know if anything pops out.

Nolan20:11:27

i suppose im looking for a model of how to interpret the output of :infer-externs

thheller20:11:08

@nolan what output/error do you get?

thheller20:11:14

to debug externs related things you pretty much don't need to look at node_modules code at all

👍 4
Nolan20:11:20

let me get the exact error. its basically b is nil where b is some munged name, and then when i click into the error, the line that triggers the error (at least says firefox) is a call to b.createReactClass

thheller20:11:59

no I meant the :infer-externs output

thheller20:11:02

not the runtime error

thheller20:11:24

the runtime error you can sort of figure out by compiling with shadow-cljs release app --pseudo-names

Nolan20:11:48

ah, well, with :all there are ~245 of them (as you warn in the manual 😂), but let me get that instead

thheller20:11:02

yeah don't use :all

Nolan21:11:25

so here is the (single) output of :infer-externs :auto:

------ WARNING #1 - :infer-warning ---------------------------------------------
 File: /Users/nolan/dev/nuid/site/src/nuid/site/routes.cljs:59:16
--------------------------------------------------------------------------------
  56 |                     js/location.search
  57 |                     js/location.hash)]
  58 |          (js/ga "send" "pageview" #js {"page" p}))
  59 |        (route! (.-token event))))
----------------------^---------------------------------------------------------
 Cannot infer target type in expression (. event -token)
--------------------------------------------------------------------------------
  60 |     (.setEnabled true)))
  61 | 
--------------------------------------------------------------------------------
which i suppose is relatively straightforward, but whats interesting is that that code has been the same for a long time and hasnt caused release issues in the past

Nolan21:11:47

im hoping its not as simple as hinting that expr…ill feel very stupid

thheller21:11:16

warnings will warn you about situations cannot figure out if externs are needed or not

thheller21:11:55

in this case event looks to be from goog.History or so. which means no externs are needed, but the compiler can't tell

thheller21:11:05

this likely has nothing to do with your issue

Nolan21:11:10

right. it would make sense that this wouldnt be the cause

Nolan21:11:16

thats good to hear, at least

thheller21:11:33

shadow-cljs release app --pseudo-names will likely help more

Nolan21:11:01

alright ill look into that

Nolan21:11:37

alright, so ive got that output—its kind of a monster but its definitely readable.

Nolan21:11:04

looking through to see if anything particularly “identifying” pops out, so far it looks like a lot of react/reagent calls

Nolan21:11:30

function $reagent$impl$template$vec_to_elem$$ is the top-level function that fails

thheller21:11:30

the error alone should contain all the info you need

Nolan21:11:53

TypeError: "$JSCompiler_temp$jscomp$1970_JSCompiler_temp$jscomp$2730_JSCompiler_temp_const$jscomp$1971_c$jscomp$inline_1109_comp$jscomp$12_f$jscomp$inline_2750_jsprops$jscomp$inline_1100_n$jscomp$186_props$jscomp$inline_1098_tag$jscomp$18_temp__5737__auto__$jscomp$inline_1105$$ is undefined"
is what im seeing

thheller21:11:10

whaaaaaaaaat the heck is that? 😛

thheller21:11:14

never seen that name before 😛

😂 4
Nolan21:11:21

id doubt it, but ill try to see if chrome shows anything more interesting

thheller21:11:48

I've never seen it generate such a long name

thheller21:11:30

so undefined errors always point to something being undefined BEFORE

👍 4
Nolan21:11:37

im starting to get a feeling it has to do with something perf related that gsap is doing. its pretty hsyterical how different the package size is with and without pseudo-names

Nolan21:11:53

all the more incentive to root this out!

thheller21:11:08

pseudo-names is only for debugging 🙂

thheller21:11:37

open the source and look for that variable name maybe. should be easy to find 😛

thheller21:11:31

is this a browser build?

Nolan21:11:47

no doubt 😄. ill take a look. and yeah, this is a browser build

Nolan21:11:53

not much interesting use of compiler options, either—just :externs and :infer-externs:

:compiler-options {:externs ["externs/isotope.ext.js"
                             "externs/gsap.ext.js"
                             "externs/gsap.textplugin.ext.js"
                             "externs/gsap.easepack.ext.js"]
                  :infer-externs :auto}

thheller21:11:42

just remove all of them

thheller21:11:56

do not ever use :auto. it'll just confuse you more than actually help.

thheller21:11:12

don't ever use generated externs

thheller21:11:26

thats mostly just noise that doesn't actually help anything

Nolan21:11:38

this is golden information

Nolan21:11:14

ive just sort of thrown together what seems to have a shot at working, and ship once it does

Nolan21:11:32

solid procedure 😂

thheller21:11:44

:infer-externs :auto and work through them

thheller21:11:01

the token error you can fix by adding (.-token ^goog event) or (.-token ^js event)

👍 4
thheller21:11:21

doesn't matter really

Nolan21:11:34

ill recompile and check it out now that ive removed the manual externs

Nolan21:11:07

so without the manaul externs, release warns about the first lib (`isotope`) that i think will get rid of the need for that first manual extern entry. but doesnt seem to find any problems with the rest. let me see what the error looks like now

thheller21:11:27

in general you only need externs for the stuff you actually use

thheller21:11:41

so the problem with generated externs is just that they generate too much

Nolan21:11:46

yeah, they do

thheller21:11:53

like hundreds or thousands even if you just need one

Nolan21:11:03

and once i had an issue (back in my cljsjs days) where the generated extern was like 400k lines

Nolan21:11:13

and that isnt an exaggeration. it was just like pure nesting

Nolan21:11:27

was so brutal

Nolan21:11:00

:infer-externs output is way, way cleaner. its awesome to start getting a better understanding of how to act on this

Nolan21:11:12

so the error is still that opaque monster. my intuition is that something is being pulled out from under reagent/react by gsap. gsap tweens are described by JS objects, so i have quite a few #js { ,,, } in there.

Nolan21:11:30

id rejoice for an immutable-all-the-way-down version of gsap. or just an implementation in cljs. im sure transients would suffice for hot paths, more or less

Nolan21:11:40

anyway, will let you know if i find anything interesting

thheller21:11:46

you can also try shadow-cljs release app --debug which will give you pseudo-names + source maps

👍 4
4
Nolan21:11:09

ill try that. source maps might help

Nolan21:11:13

are there any pitfalls (beyond the obvious mutability) of using #js { } w.r.t. compilation and hinting? or does that play a role

Nolan21:11:18

not* play a role

thheller21:11:36

if you just create them that is fine and doesn't need anything extra

thheller21:11:53

if you actually modify the objects yourself that may need externs

thheller21:11:01

but :infer-externs will warn you about those

thheller21:11:09

so no warnings = all good

Nolan21:11:58

right on. it could be that gsap internally mutates them—which would actually make some sense as an explanation for this error

thheller21:11:06

btw make sure you have a shadow-cljs server instance running if you call shadow-cljs release repeatedly

😄 4
thheller21:11:12

it'll safe you a bunch of time 😛

Nolan21:11:50

the benefits of spending time in the shadow-cljs slack are just innumerable

Nolan21:11:07

so source maps help identify that it seems to have to do with a ratom, which is actually super helpful because that limits the space of where things could be going wrong pretty significantly. the bummer is that the trace doesnt even touch any of my files:

TypeError: "<that monster name> is undefined"
    $reagent$impl$template$vec_to_elem$$ component.cljs:338
    $reagent$impl$template$as_element$$ template.cljs:409
    $reagent$impl$template$make_element$$/</< template.cljs:483
    $cljs$core$IKVReduce$_kv_reduce$arity$3$ core.cljs:5649
    $cljs$core$_kv_reduce$$ core.cljs:700
    $cljs$core$reduce_kv$$ core.cljs:2562
    $reagent$impl$template$make_element$$ template.cljs:481
    $reagent$impl$template$native_element$$ template.cljs:359
    $reagent$impl$template$vec_to_elem$$ template.cljs:385
    $reagent$impl$template$as_element$$ template.cljs:409
    $reagent$impl$component$wrap_render$$ component.cljs:104
    $reagent$impl$component$static_fns$$</</< component.cljs:128
    $reagent$ratom$deref_capture$$ ratom.cljs:37
    $reagent$ratom$run_in_reaction$$ ratom.cljs:504
    $reagent$impl$component$static_fns$$< component.cljs:143
    React (12) ...

thheller21:11:51

maybe you have something that is [undefined ...]? so not a regular [:div ...]?

coder4633421:11:16

@thheller It was the version! The re-frame template used shadow-cljs 2.8.55, the .69 works. Thanks a lot!

👍 4
thheller21:11:28

@nolan which reagent version is this?

Nolan21:11:30

hmm. i wonder if it has to do with fragments. i dont dynamically compute any [<thing> ,,,] anywhere, but i do make pretty heavy use of fragments, which could be easily changed

Nolan21:11:42

and "create-react-class": "^15.6.3", "react": "^16.10.2", "react-dom": "^16.10.2"

thheller21:11:03

hmm so in that version it is doing a cache lookup on a JS object

thheller21:11:22

maybe try a different version?

Nolan21:11:55

oh no… hold on.

thheller21:11:58

hmm probably won't change much

thheller21:11:07

other versions also had that

Nolan21:11:13

i simultaneously hope and dont hope ive found it

Nolan21:11:53

your [undefined ,,,] comment got me thinking

thheller21:11:21

the code looks to be related to props name caching

thheller21:11:37

so maybe [something {undefined "foo"} ...]?

Nolan21:11:34

thats a great point, and will be the first thing i look into after i eliminate what i must admit would be the most classical of user errors

Nolan21:11:00

😂 ugh. ok. i see exactly what happened here. RESOLVED!

Nolan21:11:49

there was a part of a component that i had wrapped in (when ^boolean js/goog.DEBUG ,,,) and well…

Nolan22:11:09

nothing like feeling dumber than a box of rocks on a friday afternoon, but the upside is that the package size is down

Nolan22:11:25

thanks a million for your help @thheller, i would have gone down 1000x more rabbit holes otherwise

👍 4
Nolan22:11:42

and i get to get rid of these silly externs that were essentially silently doing nothing… thats the real victory