Welcome! Please see the About page for a little more info on how this works.

+1 vote
in ClojureScript by
The order that namespaces are loaded at runtime is independent of the order that the namespaces are {{require}}d in user code. This seems to affect all optimization levels. This means that developers can't rely on side-effectful code in a given namespace to be run before another namespace that depends on the side effect is loaded.

Reproduction steps:

Running the command


clj --main cljs.main --compile-opts '{:target :nodejs :main a}' --compile a


will compile the {{a}} namespace defined in {{a.cljs}}. That namespace requires—via the {{:require}} form of the {{ns}} macro—namespaces {{b}} through {{g}} in alphabetical order.

After compiling, observe the following:

* The {{goog.addDependency}} call for {{a.js}} in the output {{out/cljs_deps.js}} file includes its dependencies in an arbitrary order. For example, one run on my machine resulted in:


goog.addDependency("../a.js", ['a'], ['cljs.core', 'e', 'c', 'g', 'b', 'd', 'f']);


* The console output given by running the compiled code with {{node out/main.js}} indicates that the runtime load order is identical to the ordering reflected in {{out/cljs_deps.js}}


body of e

body of c

body of g

body of b

body of d

body of f


Analysis:

It looks like some work to ensure the ordering at compile time was done as part of [CLJS-1453](https://dev.clojure.org/jira/browse/CLJS-1453), but it seems that things can get out of order by the time {{cljs_deps.js}} is emitted.

It seems that the output in {{cljs_deps.js}} for a given ClojureScript file is ultimately determined by the output of {{cljs.compiler/emit-source}}. The code there removes duplicate deps in the same way that the {{'ns}} parse method in {{cljs.analyzer}} did prior to CLJS-1453.

The attached patch resolves the issue for my narrow use case, but I believe the underlying functions are passing the `:uses` and `:requires` around as maps, so more work may be needed for a full solution.

11 Answers

0 votes
by

Comment made by: chancerussell

Apologies for cluttering up the ticket with markdown—thought I would be able to clean it up after submission.

0 votes
by

Comment made by: mfikes

CLJS-3056.patch passes CI and Canary (/)

0 votes
by

Comment made by: thheller

The attached patch still uses the {{deps}} map which will only maintain insertion order until the array-map threshold is hit and once it starts using a hash-map things will be not be ordered again. In shadow-cljs I handle this by maintaining an addition {{:deps}} vector in addition to the usual {{:requires}} map.

0 votes
by
_Comment made by: chancerussell_

That’s probably the best solution—I didn’t want to attempt to modify the type of the maps in the existing code.

I’ll take a look at this this evening—it seems like the patch for CLJS-1453 may  have the same problem.

That said, I wonder what the semantics for ordering should be when an ns form contains both :require and :use?
0 votes
by

Comment made by: thheller

Ah looks like a {{:deps}} vector is already maintained in the analyzer parse 'ns and 'ns**. Just need to use that in the cljs.compiler/emit-source fn instead taking the vals of the requires/uses maps.

0 votes
by

Comment made by: chancerussell

Thomas was right, the {{:deps}} vector is present and in the expected order, so I've submitted a new patch that uses that.

One question—what should the ordering of {{cljs.core}} and {{cljs.core.constants}} be when both are present? Obviously the ordering wasn't guaranteed with the previous code, but I'd like to get that right.

(The test in test-cljs-1882-constants-table-is-sorted would seem to imply that cljs.core should go first.)

0 votes
by

Comment made by: mfikes

CLJS-3056.patch of 24/Mar/19 12:58 PM passes CI and Canary (/)

0 votes
by

Comment made by: dnolen

{{cljs.core}} must come before {{cljs.core.constants}} as cljs.core.contants won't work without the keyword construct being defined. The ordering was most definitely guaranteed otherwise we would have heard a lot of complaints about failing advanced builds.

0 votes
by

Comment made by: chancerussell

David, when you say "ordering was most definitely guaranteed", are you referring to just {{cljs.core}} and {{cljs.core.constants}}, or the dependencies as a whole? The current code is {{conj}}ing onto sets, which isn't guaranteed to maintain any ordering AFAIK.

0 votes
by

Comment made by: mfikes

Oops, I was mistaken: CLJS-3056.patch fails Windows CI (x)

In particular, it causes a stack overflow in {{cljs-2077-test-loader}}:

ERROR in (cljs-2077-test-loader) (core.clj:6077) 397Uncaught exception, not in assertion. 398expected: nil 399 actual: java.lang.StackOverflowError: null 400 at clojure.core$get_in.invokeStatic (core.clj:6077) 401 cljs.module_graph$deps_for.invokeStatic (module_graph.cljc:147) 402 cljs.module_graph$deps_for.invoke (module_graph.cljc:147) 403 clojure.lang.AFn.applyToHelper (AFn.java:160) 404 clojure.lang.AFn.applyTo (AFn.java:144) 405 clojure.core$apply.invokeStatic (core.clj:657) 406 clojure.core$memoize$fn__6665.doInvoke (core.clj:6288) 407 clojure.lang.RestFn.invoke (RestFn.java:436) 408 cljs.module_graph$deps_for$fn__7089.invoke (module_graph.cljc:151) 409 clojure.core$map$fn__5665.invoke (core.clj:2745) 410 clojure.lang.LazySeq.sval (LazySeq.java:40) 411 clojure.lang.LazySeq.seq (LazySeq.java:49) 412 clojure.lang.RT.seq (RT.java:528) 413 clojure.core$seq__5202.invokeStatic (core.clj:137) 414 clojure.core$apply.invokeStatic (core.clj:652) 415 clojure.core$mapcat.invokeStatic (core.clj:2775) 416 cljs.module_graph$deps_for.invokeStatic (module_graph.cljc:147) 417 cljs.module_graph$deps_for.invoke (module_graph.cljc:147) 418 clojure.lang.AFn.applyToHelper (AFn.java:160) 419 clojure.lang.AFn.applyTo (AFn.java:144) 420 clojure.core$apply.invokeStatic (core.clj:657)

CI build: https://ci.appveyor.com/project/mfikes/clojurescript/builds/24478525

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJS-3056 (reported by chancerussell)
...