runtime namespace load order is independent from ordering in ns macro :require form

Description

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

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:

  • 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

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.

Environment

None

Activity

Show:
Chance Russell
March 24, 2019, 6:04 PM

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.)

Mike Fikes
March 24, 2019, 6:48 PM

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

David Nolen
March 29, 2019, 4:09 PM

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.

Chance Russell
April 4, 2019, 5:10 AM

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.

Mike Fikes
May 12, 2019, 1:37 AM

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

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

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

Assignee

Seçkin Kükrer

Reporter

Chance Russell

Labels

Approval

None

Patch

Code

Affects versions

Priority

Minor
Configure