Regression on scoped npm-modules

Question

I have regression in npm-deps resolution in 1.10.x for {{material-components-web}}. For sub-dependencies it doesn't resolve the module path correctly. When I require eg. {{@material/snackbar}} it fails with {{Uncaught Error: Undefined nameToPath for module$$material$base$component}}.

Steps to reproduce:


yarn add "@cljs-oss/module-deps" "@material/snackbar"

cat <<EOF > deps.edn
{:deps {org.clojure/clojurescript {:mvn/version "1.10.145"}}}
EOF

clj -m cljs.main -d out -e "(require '[\"@material/snackbar\"])"


Resolution worked in 1.9.946

Answer 1

Comment made by: dnolen

This ticket just needs more information. A good first step in this report would be a git bisect. Then someone needs to determine wether this is because of ClojureScript or Google Closure. If the later there's not much we can do.

Answer 2

Comment made by: rarous

I can reproduce the regression even on 1.10.63, to narrow the window. I'll prepare backwards compatible repro and bisect after that.

Answer 3

Comment made by: rarous

Bisected it and it was introduced in #CLJS-2389 (GCC update)

Answer 4

Comment made by: rarous

Code for reproduction

Answer 5

Comment made by: rarous

When head -2 out/node_modules/@material/snackbar/index.js ends with {{goog.require("module$$material$base$index");}} it is the broken case.

Answer 6

Comment made by: dnolen

I can reproduce and see that the dependency index file cljs_deps.js does not look right, so not surprised it's not working.

Answer 7

Comment made by: dnolen

Digging in further this may be a Closure issue and we'll have to wait for an upstream fix.

Answer 8

Comment made by: dnolen

This is a Closure Compiler issue. We need to submit a patch and then make the necessary changes for the next Closure release, this change https://github.com/swannodette/closure-compiler/commit/58012d3f1068aa588a47dc34ec6f39413aa59e62 fixes the module name issue for me.

Answer 9

Comment made by: rarous

Excellent, thank you David.

Answer 10

Comment made by: dnolen

PR Closure Compiler https://github.com/google/closure-compiler/pull/2847

Answer 11

Comment made by: dnolen

Can we get a non-trivial expression added to this ticket that should work after the require?

Answer 12

Comment made by: dnolen

I figured out how to test this - it does in fact seemed resolved with master + my Closure Compiler PR.

Answer 13

Comment made by: rarous

Sorry, I didn't responded earlier, I had Code Retreat yesterday. It is DOM component, so the non-trivial example needs to run in browser REPL with modified {{index.html}}. I think, if emitted {{goog.require}} is correct, everything else will work fine. Thank you very much for the fix. I hope your GCC PR will be accepted soon. {{1.10}} looks very solid and fast.

Answer 14

Comment made by: dnolen

The Closure Compiler PR now has a test case. It might need some small tweaks after it gets reviewed but hopefully this fix can make it into the May release at the latest.

Answer 15

Comment made by: tkjone

It seems that the PR to google closure has been reviewed https://github.com/google/closure-compiler/pull/2847