Fork me on GitHub
#shadow-cljs
<
2023-02-15
>
pesterhazy12:02:04

Hello! I'm trying to figure out a perplexing problem. It's difficult to debug but I managed to find a minimal repro pair. We have a test that looks like this:

(ns pitch.hello-test
  (:require [clojure.test :refer [is deftest]]
            #_[taoensso.timbre :as t]))

(deftest test-greeting
  (is (= "hello" "hello")))
So we build a bundle using the :karma target and then run karma on this bundle, which works. However, if you re-enable the timbre require (by removing #_), test-greeting isn't run and the test reporter reports 0 tests run.

pesterhazy12:02:36

With timbre disabled, karma prints out this:

15 02 2023 13:18:51.974:INFO [karma-server]: Karma v6.4.1 server started at 
15 02 2023 13:18:51.978:INFO [launcher]: Launching browsers ChromeHeadless with concurrency unlimited
15 02 2023 13:18:51.982:INFO [launcher]: Starting browser ChromeHeadless
15 02 2023 13:18:52.192:INFO [Chrome Headless 110.0.5481.100 (Mac OS 10.15.7)]: Connected on socket K4bpZfHFvl_Kcd0JAAAB with id 12774804
LOG LOG: 'Testing pitch.hello-test'

  pitch.hello-test
    ✓ test-greeting

Chrome Headless 110.0.5481.100 (Mac OS 10.15.7): Executed 1 of 1 SUCCESS (0.002 secs / 0.001 secs)
TOTAL: 1 SUCCESS

pesterhazy12:02:35

But when I re-enable timbre (which shouldn't make any difference), this is what I get instead:

15 02 2023 13:23:10.715:INFO [karma-server]: Karma v6.4.1 server started at 
15 02 2023 13:23:10.717:INFO [launcher]: Launching browsers ChromeHeadless with concurrency unlimited
15 02 2023 13:23:10.719:INFO [launcher]: Starting browser ChromeHeadless
15 02 2023 13:23:10.981:INFO [Chrome Headless 110.0.5481.100 (Mac OS 10.15.7)]: Connected on socket kzfx4Y3uilCpBBl9AAAB with id 14837417

Chrome Headless 110.0.5481.100 (Mac OS 10.15.7): Executed 0 of 0 SUCCESS (0.001 secs / 0 secs)
TOTAL: 0 SUCCESS

pesterhazy12:02:41

Not sure what I can try or how to debug this very unexpected behavior (Is it a problem in our configuration? karma-cljs-test? in shadow?)

thheller12:02:44

quick guess is that timbre somehow messes with *print-fn* on load and all the output is lost?

pesterhazy12:02:38

Could that make the test function not run at all? We can see that it never gets executed by karma

thheller12:02:11

been years since I looked at karma. what happens if you load the build in a regular browser?

thheller12:02:17

could be throwing an error on load? not sure that would get reported

pesterhazy13:02:15

Trying that now

pesterhazy13:02:14

I don't see anything in the browser logs. I added a console log that shows that the namespace is loaded but the test fn is not run

pesterhazy13:02:39

One thing I observed is that I can reliably repro the problem only if I rm -rf .shadow-cljs before recompiling

pesterhazy13:02:28

So basically the issue appears only if I'm starting from a clean slate (this is what happens in CI). If I re-compile in shadow a second time, then the test is picked up as exepcted

pesterhazy13:02:17

My assumption was that compiling twice shouldn't make a difference. This leads me to suspect that there's something not 100% right with the shadow compile output, but I'm having a hard time figuring out the difference

thheller13:02:11

I have never looked at timbre. does it happen only with timbre or any other namespace as well?

thheller13:02:25

I have no clue what it does, so can't rule out funky stuff

thheller13:02:00

can you make a repro? happy to take a look

thheller13:02:13

or does it only reproduce on specific machines?

thheller13:02:58

since you say CI it could be a memory issue? although that should be noticable before it ever gets to running karma

thheller13:02:33

there is a history of CI envs killing the shadow-cljs compilation since the default memory limits are ... well the default and CI likes capped things

thheller13:02:11

could be that by default it gets by with memory, but adding timbre takes it over the limit?

thheller13:02:35

but if you can repro on your local machine outside CI it likely is something else

pesterhazy13:02:11

I was able to make it repro on my local machine after some digging

pesterhazy13:02:12

I just found something interesting. Basically • on first compile it's broken • if I recompile it starts working I copied the output and did a diff of the two

thheller13:02:16

I assume you don't have a OOM killer 😉

pesterhazy13:02:25

No, it's not about memory pressure

pesterhazy13:02:36

The diff looks like this

-shadow.test.env.reset_test_data_BANG_(cljs.core.PersistentArrayMap.EMPTY);
+shadow.test.env.reset_test_data_BANG_(new cljs.core.PersistentArrayMap(null, 1, [new cljs.core.Symbol(null,"pitch.hello-test","pitch.hello-test",-1071328279,null),new cljs.core.PersistentArrayMap(null, 1, [new cljs.core.Keyword(null,"vars","vars",-2046957217),new cljs.core.PersistentVector(null, 1, 5, cljs.core.PersistentVector.EMPTY_NODE, [new cljs.core.Var(function(){return pitch.hello_test.test_greeting;},new cljs.core.Symbol("pitch.hello-test","test-greeting","pitch.hello-test/test-greeting",1200222568,null),cljs.core.PersistentHashMap.fromArrays([new cljs.core.Keyword(null,"ns","ns",441598760),new cljs.core.Keyword(null,"name","name",1843675177),new cljs.core.Keyword(null,"file","file",-1269645878),new cljs.core.Keyword(null,"end-column","end-column",1425389514),new cljs.core.Keyword(null,"column","column",2078222095),new cljs.core.Keyword(null,"line","line",212345235),new cljs.core.Keyword(null,"end-line","end-line",1837326455),new cljs.core.Keyword(null,"arglists","arglists",1661989754),new cljs.core.Keyword(null,"doc","doc",1913296891),new cljs.core.Keyword(null,"test","test",577538877)],[new cljs.core.Symbol(null,"pitch.hello-test","pitch.hello-test",-1071328279,null),new cljs.core.Symbol(null,"test-greeting","test-greeting",-363084393,null),"pitch/hello_test.cljs",(23),(1),(7),(7),cljs.core.List.EMPTY,null,(cljs.core.truth_(pitch.hello_test.test_greeting)?pitch.hello_test.test_greeting.cljs$lang$test:null)]))], null)], null)], null));

thheller13:02:15

ah, so the tests are missing

pesterhazy13:02:00

it contains a reference to pitch.hello-test. The fact that this is missing probably explains why the test doesn't run

thheller13:02:32

do you use a custom runner-ns?

pesterhazy13:02:09

(ns pitch.karma.main
  (:require [cljs.test :as ct]
            [shadow.test.karma :as sk]))

;; make matcher-combinators work in karma CI test runner
(defmethod ct/report [:shadow.test.karma/karma :matcher-combinators/mismatch] [m]
  (let [fail-method (get (methods ct/report) [:shadow.test.karma/karma :fail])]
    (fail-method (assoc m :type :fail))))

(defn ^:export init []
  (sk/init))

thheller13:02:39

that doesn't work (unfortunately, yet)

thheller13:02:13

so the problem is that this namespace does not depend on the test namespaces

thheller13:02:36

therefore it'll be compiled whenever parallel compile has all its requires finished

thheller13:02:55

to make sure all tests are compiled first the runner-ns gets compiled last

pesterhazy13:02:05

so there's a kind of compile-time race condition?

thheller13:02:33

BUT you are using shadow.test.karma, which then does not get that protection

thheller13:02:54

so currently the only way to customize runners is my copying them not referring them

thheller13:02:43

this is the problem case. this is a macro that collects all known tests

thheller13:02:56

if it expands before tests are compiled it can't find them

pesterhazy13:02:20

that makes a lot more sense now. I thought I was going crazy

pesterhazy13:02:40

timbre had nothing to do with it except that it changes the timing of (parallel?) compiles

pesterhazy13:02:04

thanks so much

pesterhazy13:02:24

> so currently the only way to customize runners is my copying them not referring them what do you mean by this - just copy and paste the entire namespace?

thheller13:02:28

as long as the two lines above are in the actual runner-ns and the runner-ns has :dev/always true you should be ok

pesterhazy13:02:00

ok. maybe we can remove the custom test runner altogether

pesterhazy13:02:07

we had this in our config

:browser-test-karma {:target :karma
                               ;; using karma.main as runner-ns here so it wouldn't be eliminated by cljs compiler
                               :runner-ns pitch.karma.main
                               :compiler-options {:output-feature-set :es8
                                                  :warnings-as-errors true
                                                  :static-fns false}
                               :output-to "target/browser-test/public/static/app/719998b2-df4b-4b86-9793-3f821d281369/js/browser_test_karma.js"
                               :ns-regexp "-test(s)*$"}

pesterhazy13:02:40

> using karma.main as runner-ns here so it wouldn't be eliminated by cljs compiler Looks like someone was wrestling with something like this before

thheller13:02:21

hmm what do you mean by eliminated?

pesterhazy13:02:56

I don't know, I didn't write that 🙂 someone probably hypothesized (incorrectly?) that cljs compiler eliminates code

pesterhazy13:02:43

Still don't understand 100% why only this one namespace showed this race condition in my tests, but that's not as important as understanding the underlying cause

pesterhazy13:02:22

(Our CI ran 1000 tests, while locally the runner ran 994 tests, and we couldn't find what caused the diff)

thheller13:02:45

parallel compile is funky like that

pesterhazy13:02:47

Compiler race condition wasn't my first thought

thheller13:02:57

it sorts all build sources in dependency order

thheller13:02:12

then fires off all compilation. thread pool takes over and compiles the next source

thheller13:02:25

each thread waits will its dependencies are filled

thheller13:02:47

in your custom runner it fills before all the final tests are compiled. so shadow.test.karma is compiled "early" because it has no requires

thheller13:02:47

994 it then queue size - potential cpu core parallelism

thheller13:02:08

so 6 other cores are still compiling when the macro executes or so

pesterhazy13:02:10

yeah for sure - my macbook has more cores than the circle ci machine

thheller13:02:04

thats why requires are usually so important, only way to ensure everything is as expected

thheller13:02:21

but for tests it sucks to specify them, thus this hack of injected extra requires 😛

pesterhazy13:02:39

the clojurescript discover-tests-via-macro "hack" is kind of breaking the require assumption

thheller13:02:23

its a general problem, for which I have a branch called late-cljs waiting 😛

pesterhazy13:02:41

late as in late binding?

thheller13:02:13

(defn init []
  (macro-that-collects-meta))

(defn foo {:with-cool-meta true} [...])

thheller13:02:26

no matter what you do this macro can never find the foo meta

thheller13:02:50

in CLJ this is totally fine since metadata is collectable at runtime

thheller13:02:52

in CLJS it is not

thheller13:02:36

so the idea is to expand macros that require this data "late", as in at end of compilation

thheller13:02:20

but that brings with it a whole bunch of other problems, that why it has been sitting there for a year 😛

pesterhazy13:02:11

sounds like some deeper changes to the cljs compiler

thheller14:02:00

might be solvable at macro level, but I'm always hesitant to add stuff that won't behave as expected when using the regular compiler

👍 2
souenzzo15:02:14

Hello, I have a cljc function that works in dev, but fails to run in release mode

(defn replace-char
  ^String
  [^String s ^Character match ^Character replacement]
  #?(:cljs (.replaceAll s match replacement)
     :clj  (.replace s match replacement)))
Inspecting the generated code (with --debug)
function $app$core$replace_char$$($s$jscomp$128$$, $match$jscomp$9$$, $replacement$jscomp$4$$) {
  return $s$jscomp$128$$.$replaceAll$($match$jscomp$9$$, $replacement$jscomp$4$$);
}
The issue is the $ in the start of the .replaceAll method I'm able to do some code-golfing, probably removing the ^String from JS part of CLJC. But I would like to understand better why this extra $ is here.

thheller15:02:52

that is to tell you that .replaceAll was renamed to .$replaceAll$ as part of the advanced optimizations. without pseudo-names it would be .aX or something short, which would make it hard to identify

thheller15:02:33

by tagging them all with type hints, the compiler assumes it save to rename them

thheller15:02:21

(defn replace-char
  ^String
  [s match replacement]
  #?(:cljs (.replaceAll s match replacement)
     :clj  (.replace ^String s ^Character match ^Character replacement)))

thheller15:02:39

not much golfing involved. the type hints won't do anything for CLJS anyways, except confuse the compiler

souenzzo15:02:17

What does ^String means to GCC? In my mind, as it means nothing to GCC, it is equivalent to not having a tag.

thheller15:02:35

gcc doesn't care and doesnt see it

thheller15:02:05

but it interferes with cljs externs inference. since it disregards anything tagged

thheller15:02:20

so in a way you are silencing the inference warning you'd otherwise get

souenzzo15:02:23

without any tag:

204 |   #?(:cljs (.replaceAll s match replacement)
------------------^-------------------------------------------------------------
 Cannot infer target type in expression (. s replaceAll match replacement)
--------------------------------------------------------------------------------
with ^js s it works

thheller15:02:51

I'm not sure when String.replaceAll was added, I guess after the version you have

thheller15:02:27

(.replaceAll ^js s ...) would be correct here

👍 2
thheller15:02:55

although I'm not entirely sure why replaceAll is getting renamed here. that should exist in the default externs?

souenzzo15:02:57

btw, Im using an old (2.15.9) shadow version

thheller15:02:21

even in that version it should exist

jeaye19:02:48

Hi! 🙂 I'm looking to provide a vanilla JS interface to a CLJS app I've written. Basically making the app's functionality available as a JS lib. To do this, I'll need to control munging, export some certain symbols, and presumably do some js->clj and clj->js. I'm wondering: 1. What's the most accurate target for this? It's looking like :esm , but I wanted to be certain. 2. Has anyone seen any good examples come by of JS/TS interfaces for CLJS libs? I didn't find much in my searching. 3. Any gotchas I need to know about doing this? My biggest concern is the perf hit of JS->CLJS transformations, since my data can be quite large.

thheller20:02:58

:esm and :npm-module are your options. :esm is recommended

thheller20:02:14

interface depends on what you want to do

thheller20:02:26

and yes serializing a lot can be slow

jeaye20:02:43

👍 Appreciate the guidance. I'll dig into it.

bringe22:02:15

Hello! Is it possible to have a connected repl session to the JS runtime (nodejs in my case) when using the :npm-module target? I’ve built a VS Code extension using the :npm-module target, but when I run the extension my repl doesn’t seem to connect to the JS runtime as it does when I use the :node-library target. So when I try to eval something I get “No available JS runtime.“. I notice https://shadow-cljs.github.io/docs/UsersGuide.html#missing-js-runtime. that :npm-module is not listed, so maybe it’s not intended to be used in this way? As I said, :node-library works, but I’m experimenting to find a solution to a problem for which :npm-module was looking promising.

thheller05:02:16

npm-module just doesn't have a defined entry point, so it cannot prepend the repl client ns.

thheller05:02:23

you can set :runtime :node in the build config

🙏 2
thheller05:02:57

and then somehow from the JS side load the ./<output-dir>/shadow.cljs.devtools.client.node ns

thheller05:02:53

so say you require("./out/your.entry") from a JS file

thheller05:02:14

add require("./out/shadow.cljs.devtools.client.node") before or after it

thheller05:02:40

maybe conditionally if there is such a thing in the env, eg. process.env.NODE_ENV == 'development'

👍 2
bringe19:02:20

Thanks for the info! I’ll try this when I get back to it.

bringe20:02:47

I’ve now tried this, and I get an error - SHADOW_IMPORTED is not defined. To give a bit more info on what I’m trying to do: I have a minimal project set up to test building Calva with shadow-cljs while still being able to utilize the TS and CLJS we already have. The idea is that it could/should work like in the image (arrows denote reference direction). The top-level cljs references the compiled TS (I can make this work), and the TS references the lower-level / existing CLJS. The left and middle part are existing already in Calva. The right part is the thing I’m adding. This :npm-module single-build approach is just one approach I’ve tried. The shadow-cljs.edn looks like:

{:deps true
 :builds       {:extension
                {:target :npm-module
                 :runtime :node
                 :output-dir "node_modules/shadow-cljs"
                 :entries [calva-cljs.extension
                           calva.foo]
                 :devtools {:before-load-async calva-cljs.extension/before-load-async
                            :after-load calva-cljs.extension/after-load}}}}
The extension is using "./node_modules/shadow-cljs/calva_cljs.extension.js" as its entrypoint. The extension works like this, at least without importing/using calva.foo (haven’t verified if that works yet - just trying to get a repl connection to the runtime). Now, going by what you said, I required shadow.cljs.devtools.client.node in my entry point which is calva-cljs.extension.
(ns calva-cljs.extension
  (:require ["vscode" :as vscode :refer [window]]
            ["/foo.js" :as foo]
            ["shadow-cljs/shadow.cljs.devtools.client.node" :as node-client]))

bringe20:02:15

This results in the SHADOW_IMPORTED is not defined error.

bringe20:02:20

Maybe there’s something else I need to require? There may be a better approach to this altogether - I’m just exploring this approach for now. Another thing I haven’t tried is two separate node-library builds with two separate shadow-cljs.edn files - the thought is that I can run two shadow-cljs servers and not have the conflicts/errors I saw before.

bringe20:02:36

Using two :node-library builds in the same shadow-cljs.edn file, I think I can partly achieve what I want but only by doing a release build of the calva-lib build, which means I can’t get hot reload of the changes made in that build, I think. Otherwise, there are seemingly conflicts between cljs (duplicate things being loaded) or some other errors.

thheller20:02:36

if you want to require it from CLJS code just require it as a regular NS

thheller20:02:57

["shadow-cljs/shadow.cljs.devtools.client.node" :as node-client] this is very likely blowing shit up

thheller20:02:14

[shadow.cljs.devtools.client.node] if anything

thheller20:02:38

but I used JS in the example I gave you specifically since you can do conditional requires in JS, but not in CLJS

thheller20:02:57

doing it in the entry point just means the release build will include it as well

thheller20:02:18

what I would recommend is that your extension entry point is a JS file

thheller20:02:25

that you write manually

thheller20:02:33

with just two requires

thheller20:02:44

one for the CLJS entry ns, one for the devtools

thheller20:02:33

using two builds is just a recipe for pain and suffering. do not do that

bringe19:02:13

Thanks for the info and advice 👍 gratitude

bringe20:02:47

I’ve tried a js entry point now that looks like this:

console.log("Loading shadow devtools for node client");
require("shadow-cljs/shadow.cljs.devtools.client.node");
console.log("Loading extension entry point");
const extension = require("shadow-cljs/calva_cljs.extension");

exports.activate = extension.activate;
exports.deactivate = extension.deactivate;
The extension works as expected, and when I save the extension.cljs file and the shadow watch process runs the compilation again, it succeeds, but a JS error is thrown that I can see in the debug console: JS reload failed ReferenceError: SHADOW_IMPORT is not defined. At this point the extension seems broken. From what I can tell, SHADOW_IMPORT is provided by using node_bootstrap.txt in shadow.build.node/flush-unoptimized here: https://github.com/thheller/shadow-cljs/blob/49fb078b834e64f63122e3a8ad3ddcd1f4485969/src/main/shadow/build/node.clj#L174. I see that shadow.build.node/flush-unoptimized is called in shadow.build.target.node-library and shadow.build.target.node-script, but not in shadow.build.target.npm-module. I’m guessing that might be related to why SHADOW_IMPORT isn’t found, but I haven’t dug deeper into the source to understand what I might need to do to to make this work.

thheller21:02:38

hmm I guess nobody uses npm-module in a watch. I guess you can try to put the contents of the node_bootstrap.txt into your JS file

👍 2