Fork me on GitHub
#shadow-cljs
<
2022-06-08
>
wombawomba10:06:45

I've noticed that using modules seems to increase build times. Is there a clean way to disable modules in certain circumstances (preferably via the command line)?

thheller11:06:18

that seems unlikely? I mean if you can prove it I'd be interested but I very much doubt :modules have any impact at all

thheller11:06:42

a few milliseconds at worst

thheller11:06:44

unless you are talking release builds? can't say how much more that side costs on the closure compiler side. wouldn't expect too much here either though

thheller11:06:49

just try the same build with one :modules but all the :entries it would otherwise have. and then one with regular multiple :modules?

wombawomba12:06:35

@thheller so I just performed this experiment, and what takes 3.5s with a single module (re-building a single ns) takes 25s with multiple modules

thheller12:06:12

@wombawomba please verify with --verbose where the actual time is spent?

thheller12:06:37

and please report the final Build Complete message? I mean if it ends up including thousands more files this is expected?

wombawomba12:06:24

FWIW the build messages without --verbose were:

[:app] Build completed. (1098 files, 1 compiled, 0 warnings, 2.80s)
and
[:app] Build completed. (1410 files, 1 compiled, 0 warnings, 22.36s)

wombawomba12:06:54

so it seems I missed a few files when joining everything together

wombawomba12:06:07

still, the time difference is disproportionate

wombawomba12:06:59

okay, so here's the verbose report with a single module:

[:app] Compiling ...
-> Resolving Module: :main
<- Resolving Module: :main (1386 ms)
-> build target: :browser stage: :compile-prepare
<- build target: :browser stage: :compile-prepare (0 ms)
-> Compile CLJS: my-app/views/site_home.cljs
<- Compile CLJS: my-app/views/site_home.cljs (743 ms)
-> Cache write: my-app/views/site_home.cljs
<- Cache write: my-app/views/site_home.cljs (92 ms)
-> build target: :browser stage: :compile-finish
-> build target: :browser stage: :flush
-> Flush: my-app/views/site_home.cljs
-> Flushing unoptimized modules
-> Flush: shadow/module/main/append.js
<- Flush: shadow/module/main/append.js (0 ms)
<- Flush: my-app/views/site_home.cljs (12 ms)
<- Flushing unoptimized modules (330 ms)
<- build target: :browser stage: :flush (350 ms)
<- build hook: [0 shadow.cljs.build-report/hook] stage: :flush (0 ms)
[:app] Build completed. (1098 files, 1 compiled, 0 warnings, 2.75s)

wombawomba12:06:54

...and here's the one with multiple modules:

wombawomba12:06:59

(FWIW disabling the build-report hook doesn't seem to make a difference)

wombawomba12:06:50

...interesting how "Resolving Module" is either instantaneous or takes hundreds of ms. A bug perhaps?

thheller12:06:35

what the heck? how many modules does this have?

thheller12:06:24

I'm guessing that the fast ones require fewer namespaces and it doesn't go chasing down 1000+ files

thheller12:06:32

which shadow-cljs version is this with?

wombawomba12:06:31

150 modules, of which 100 just require a single js file from an NPM package

wombawomba12:06:47

there seems to be no correlation between module size and the time a module takes to build

thheller12:06:15

how the hell do you get to 150 modules?

wombawomba12:06:44

ah no, wait, there is a correlation - my bad

wombawomba13:06:57

We've actually talked about this before 🙂 I'm packaging react-syntax-highlighter syntax files as separate modules

wombawomba13:06:15

regardless, those modules seem to be pretty quick to load

wombawomba13:06:32

there's also one module per view in the app, and it seems to be those that are taking a long time to resolve

wombawomba13:06:17

shadow-cljs 2.18.0

thheller13:06:18

what does the build report look like for this? 149 tiny modules and 1 huge chunky main module?

wombawomba13:06:46

no, it's fairly well distributed

thheller13:06:57

hmm I guess this is running into an unoptimized case an extreme amount of times

thheller13:06:06

guess that could be optimized a little bit

wombawomba13:06:27

in the meantime, is there something I can do to disable the modules from the command line when developing?

thheller13:06:03

no. I'd recommend just making two builds. one with modules one without

wombawomba13:06:14

@thheller any idea when/if this code might be optimized? Ideally I'd use modules for development as well, and I'm curious as to when I might be able to do so

thheller13:06:40

well you case seems rather extreme so I have no way to reproduce that

thheller13:06:50

maybe I can come up with a simple reproduction case

wombawomba13:06:10

if possible, something like "only resolve modules when involved files change" seems like it would take care of the problem

thheller13:06:26

thats not the issue. at least I don't think it is ... just guessing still at this point

wombawomba13:06:19

oh? looking at the build output, all the extra build time seems to come from "Resolving Module" steps

wombawomba13:06:33

the actual compilation doesn't seem to be affected

thheller13:06:40

yes, it is the resolve part

thheller13:06:49

just not the "when involved files change" part

thheller13:06:23

it needs to resolve modules and it does so by following the requires

thheller13:06:48

the issue (I'm guessing) is that that repeatedly for each module since it is touching the disk

thheller13:06:58

and not using info it may already have

wombawomba13:06:08

yeah, that's what I figure too

thheller13:06:22

and it is not using that info because node resolve rules are weird and requiring one thing from one place

thheller13:06:38

may yield a different result than requiring the same thing from a different place

wombawomba13:06:12

I have no idea how this works under the hood, but it seems like you should be able to keep a record of all files involved when resolving each given module, and re-resolve only when those files are touched

thheller13:06:58

yes if you scratch the "touched" part again. caching is not the issue here and is covered elsewhere.

thheller13:06:34

I'll figure something out when I have some time

wombawomba13:06:50

awesome, thanks

thheller14:06:10

@wombawomba I cannot reproduce things resolving slowly multiple times. so might be something you are doing in your setup I'm missing

thheller14:06:43

of course I'm testing with a reasonable number of modules so can't really say if that is a factor

thheller14:06:08

no more time. feel free to open an issue. a reproducible case would help though

p-himik15:06:26

An interesting issue. Not sure whether a genuine issue or a peculiarity. I'm vendoring in a library called waveform-playlist and in one of its files it has this:

import stateClasses from "./track/states";
Shadow-cljs complains:
FileNotFoundException: /home/p-himik/dev/git/ensemble/src/waveform-playlist/track/states (Is a directory)
This is, there's both a track/states directory and a track/states.js file. When using IDEA to navigate to the source of the "./track/states" import, it correctly goes to states.js.

thheller17:06:41

by "vendoring" I assume you mean putting it on the classpath? In general you should refer to exact files with extensions there.

thheller17:06:12

this doesn't follow the node resolve rules and to avoid ambiguities like that you should be using exact file names

thheller17:06:42

yes, they are optional in some places

thheller17:06:47

but that is a decision I already regret

p-himik17:06:27

I see, that's what I ended up doing. Thanks!

scarytom15:06:16

shadow-cljs watch seems to affect other commands run in separate terminals. I run my unit tests by first running shadow-cljs -A:test compile unit-tests, which has a target of :node-test and produces a unit-tests.js file that can then be executed via node. This all works fine unless I happen to be running shadow-cljs watch app in another terminal at the same time. In this case, the compile step still produces a unit-tests.js file, but it is missing a bunch of the imports and executing it runs 0 tests. Any ideas on this?

thheller16:06:40

@t.denley yes, the shadow-cljs command will re-use a running shadow-cljs instance if running. I recommend keeping you classpath the same all the time to avoid issues such as this. I assume you are adding extra-paths in the test aliases. there is no downside in always having them. you can run with --force-spawn to always get a new instance

scarytom16:06:01

Thanks, that's a big help

p-himik17:06:07

How should I actually vendor a JS library that consists of multiple files? The section at https://shadow-cljs.github.io/docs/UsersGuide.html#_language_support makes it seem pretty straightforward. So I have /waveform-playlist/track/loader/LoaderFactory.js on classpath with (abridged)

import BlobLoader from "./BlobLoader";
import IdentityLoader from "./IdentityLoader.js";
import XHRLoader from "./XHRLoader";
And I have /waveform-playlist/track/loader/IdentityLoader.js on classpath with
import Loader from "./Loader";

export default class IdentityLoader extends Loader {
  load() {
    return Promise.resolve(this.src);
  }
}
But in the browser, I get:
ReferenceError: IdentityLoader$$module$waveform_playlist$track$loader$IdentityLoader is not defined
    at eval (LoaderFactory.js:5:19)
    at eval (<anonymous>)
    at goog.globalEval (main.js:472:11)
    at env.evalLoad (main.js:1534:12)
    at main.js:2705:12
And the loaded LoaderFactory.js now looks like this:
import BlobLoader from "/waveform-playlist/track/loader/BlobLoader.js";
import IdentityLoader from "/waveform-playlist/track/loader/IdentityLoader.js";
import XHRLoader from "/waveform-playlist/track/loader/XHRLoader.js";
What should I do here?

thheller17:06:43

@p-himik js files on the classpath get different treatment than the regular node_modules files. this code gets rewritten by the closure compiler. there may be issues with that since not many people do this

thheller17:06:05

I can't say what exactly is the problem here but I recommend looking at the generated code

thheller17:06:20

my guess is just that it is a scoping issue

thheller17:06:28

I wish this worked more reliably but what the closure compiler is doing here is somewhat arbitrary and changes constantly

thheller17:06:42

its also not really meant to be used the way I'm using it. it is usually only use in the context of a whole program optimization pass where everything is in the same scope

thheller17:06:52

the way the dev builds are loaded might break that

p-himik17:06:59

Oh. So it sounds like something might work in prod but be broken in dev? And maybe vice versa?

thheller17:06:26

I haven't looked at this code in years

thheller17:06:32

dunno what the closure compiler generates these days

p-himik17:06:35

The compiled output has no import statements of any kind BTW. It's as if they don't exist at all.

thheller17:06:47

yes, they need to be removed. otherwise they'd break completely since you can't use import outside modules

p-himik17:06:46

Ah, right. But all usages of IdentityLoader become IdentityLoader$$module$waveform_playlist$track$loader$IdentityLoader. No clue how that's supposed to work.

thheller17:06:04

as I said. look at the generated code

thheller17:06:16

its the same rewriting it does for all the other code during advanced

thheller17:06:35

meaning it rewrites everything with the assumption everything is in the global scope

thheller17:06:40

no separate files

thheller17:06:53

so it just scopes all variables accordingly which is what that long name is

thheller17:06:09

basically just the IdentityLoader in the file named by that munged name

thheller17:06:28

$waveform_playlist$track$loader$IdentityLoader basically /waveform_playlist/track/loader/IdentityLoader

p-himik17:06:09

I am looking at the generated code. :) I'm not sure what exactly I should be looking for though. So I have target/dev/cljs-runtime/module$waveform_playlist$track$loader$LoaderFactory.js and in it I have console.log("IL", IdentityLoader$$module$waveform_playlist$track$loader$IdentityLoader); as the very first line.

thheller17:06:22

yes, and where is the line declaring that

p-himik17:06:43

It does not exist.

thheller17:06:51

not in that file. because it is not declared there

thheller17:06:00

/waveform_playlist/track/loader/IdentityLoader this file

thheller17:06:12

where the class is from

thheller17:06:55

target/dev/cljs-runtime/module$waveform_playlist$track$loader$IdentityLoader.js

p-himik17:06:12

Ah, right. In target/dev/cljs-runtime/module$waveform_playlist$track$loader$IdentityLoader.js I have:

class IdentityLoader$$module$waveform_playlist$track$loader$IdentityLoader extends $jscompDefaultExport$$module$waveform_playlist$track$loader$Loader {
  load() {
    return Promise.resolve(this.src);
  }
}
/** @const */ 
var module$waveform_playlist$track$loader$IdentityLoader = {};
/** @const */ 
module$waveform_playlist$track$loader$IdentityLoader.default = IdentityLoader$$module$waveform_playlist$track$loader$IdentityLoader;

$CLJS.module$waveform_playlist$track$loader$IdentityLoader=module$waveform_playlist$track$loader$IdentityLoader;
//# sourceMappingURL=module$waveform_playlist$track$loader$IdentityLoader.js.map

thheller17:06:06

ok there is the issue

thheller17:06:26

it is preserving class which is supposed to create this variable globally

thheller17:06:33

but doesn't due to being loaded via eval

thheller17:06:11

it might work if you switch to :devtools {:loader-mode :script}?

p-himik17:06:22

But also, seems like it could've been using $CLJS.module$waveform_playlist$track$loader$IdentityLoader.default where that class is supposed to be used.

p-himik17:06:57

Oh, about a year ago or so {:loader-mode :script} made a dev page reload insanely slow. I'd rather search for a different solution.

thheller17:06:10

well its the only suggestion I have

p-himik17:06:44

It's not possible to tell GCC to use that $CLJS.[...] instead of the name of a class, right?

thheller17:06:47

changing what the code references where is not exactly easy

thheller17:06:41

I have some code that is supposed to find global references and export them

thheller17:06:48

maybe that just doesn't find class? not exactly sure

thheller18:06:09

doesn't find class I guess

thheller18:06:45

can't check what the ast looks like for this now

thheller18:06:55

but thats likely the place it would need to find it

p-himik18:06:54

Out of hopeful curiosity - why exactly is eval needed? Why can't the initial code loading just load a single giant file without any eval statements and any further code reloads append new <script> tags?

thheller19:06:50

source maps basically

thheller19:06:27

and some other reason I can't remember right now

p-himik19:06:56

My level of expertise is dangerously close to 0 here, but I think at least on the initial load you can provide a single mangled JS file plus a single sourcemap file, and let it be mapped to multiple original CLJS sources. Right? If so - maybe there's a way to patch a source map without having to re-download it? Of course, even if all that is correct, it still doesn't address that other reason...

thheller19:06:16

thats what this is basically. it just uses eval to load the files to match the behavior of hot-reloaded files

thheller19:06:37

but that wasn't the reason. really can't remember but I spent a substantial amount of time on this so please assume that this is done for a reason

thheller19:06:09

feel free to try alternatives if you want to investigate a new loader-mode

thheller19:06:19

I don't have time to currently

p-himik19:06:49

Right. I myself am currently rewriting the vendored library to use goog.module. :D

p-himik19:06:29

A related question - how would I use an NPM dep inside a goog module?

thheller19:06:34

not supported (because the closure compiler doesn't know how to process it)

p-himik19:06:02

Ah. Crap x 2.

thheller19:06:10

but you can just write it as commonjs isntead

thheller19:06:14

no point in goog.module really

thheller19:06:29

just require/exports

thheller19:06:34

so no import or export

thheller19:06:08

commonjs gets the exact same treatment as npm libs

p-himik19:06:16

Thanks, will check out how it's done. Have never bothered with all JS :poop: ways of module management.

thheller19:06:31

just skip goog.module

👍 1
thheller19:06:37

and use require instead of goog.require

thheller19:06:49

exports.x can stay the same as in goog.module

p-himik20:06:26

Oh bloody hell... So if I have exports = {Playlist}; it doesn't work because Suspicious re-assignment of "exports" variable. Did you actually intend to export something? And if I have exports.Playlist = Playlist; or module.exports = {Playlist};, it doesn't work because

ExceptionInfo: failed to convert sources
[...]
Caused by:
NullPointerException: NAME h 11:8  [length: 1] [source_file: waveform-playlist/Playlist.js] [original_name: h]
And h there is just import {h, diff, patch} from "virtual-dom";.

thheller20:06:56

and why is there an import? that makes it ESM and breaks everything

p-himik20:06:46

Ah, of course. Brain refused to see it because it's not importing the vendored stuff.

thheller20:06:02

dunno if this would fix your issue. too tired to test right now.

thheller20:06:12

in theory it should though

p-himik20:06:02

Immaculate timing - just as I finished rewriting it into CommonJS! :D Thanks for all the guidance BTW. But good to know either way, I'll test it a bit later and let you know.

pinkfrog23:06:20

I see many resource not found errors, how’s the path determined?

thheller23:06:09

:asset-path in your build config. defaults to "/js"

thheller23:06:34

for the websocket error you probably need to set :devtools {:devtools-url ""}

pinkfrog23:06:28

If I do not specify devtools-url, how is it determined? The oh….pa part is weird

thheller23:06:50

by default it uses the document.location of the html document loading the JS

thheller23:06:01

thats just this weird thing in chrome extensions

thheller23:06:24

:devtools {:use-document-host false} also works

pinkfrog23:06:59

Also it sounds I have to put the :devtools under the build section. Putting it under top level is ignored.

thheller23:06:25

yes, part of the build config always