Skip to content

Commit 9d8d371

Browse files
committed
Fix handling of orphans.
- deepest-common-parent should consider that the module under consideration may in fact be the deepest common parent. - orphan module computation was just wrong. we assign too early. we cannot begin to assign until we know about orphans. we now guess the assignments for all modules, for all module deps, for all orphans, and for all orphan deps. We then merge all these together and compute the final assignment. Update test case so that it reflects new more optimal assignment. In the test case cljs.core isn't moved to base but to the shared module.
1 parent 3a6e71e commit 9d8d371

File tree

2 files changed

+26
-25
lines changed

2 files changed

+26
-25
lines changed

src/main/clojure/cljs/module_graph.cljc

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,9 @@
163163
"Given a set of modules and a compiler :modules graph, compute the deepest
164164
common parent module."
165165
[modules all-modules]
166-
(let [common-parents (reduce set/intersection
167-
(map #(set (deps-for-module % all-modules)) modules))]
166+
(let [common-parents
167+
(reduce set/intersection
168+
(map #(conj (set (deps-for-module % all-modules)) %) modules))]
168169
(apply max-key
169170
(fn [p] (get-in all-modules [p :depth]))
170171
common-parents)))
@@ -213,31 +214,31 @@
213214
(first maybe-assigned)
214215
(deepest-common-parent maybe-assigned modules))])
215216
canon (fn [xs] (into #{} (map #(canonical-name % index)) xs))
216-
assigns (fn [f]
217-
(reduce-kv
218-
(fn [ret module-name {:keys [entries] :as module}]
219-
(let [entries' (canon entries)]
220-
(reduce
221-
(fn [ret entry]
222-
(update ret entry (fnil conj #{}) module-name))
223-
ret (canon (f entries')))))
224-
{} modules))
225-
e->ms (binding [deps-for (memoize deps-for)]
226-
(assigns identity))
227-
d->ms (binding [deps-for (memoize deps-for)]
228-
(assigns #(distinct (mapcat deps %))))
229-
assigned (merge
230-
(into {} (map assign1) d->ms)
231-
(into {} (map assign1) e->ms))
232-
orphans (zipmap
217+
assigns (fn [f ms]
218+
(binding [deps-for (memoize deps-for)]
219+
(reduce-kv
220+
(fn [ret module-name {:keys [entries] :as module}]
221+
(let [entries' (canon entries)]
222+
(reduce
223+
(fn [ret entry]
224+
(update ret entry (fnil conj #{}) module-name))
225+
ret (canon (f entries')))))
226+
{} ms)))
227+
e->ms (assigns identity modules)
228+
d->ms (assigns #(distinct (mapcat deps %)) modules)
229+
e&d->ms (merge-with into e->ms d->ms)
230+
orphans {:cljs-base
231+
{:entries
233232
(map (comp str comp/munge first :provides)
234233
(-> (reduce-kv
235234
(fn [m k _]
236235
(reduce dissoc m (get-in m [k :provides])))
237-
index assigned)
238-
vals set))
239-
(repeat :cljs-base))]
240-
(merge assigned orphans)))
236+
index e&d->ms)
237+
vals set))}}
238+
o->ms (assigns identity orphans)
239+
od->ms (assigns #(distinct (mapcat deps %)) orphans)
240+
all->ms (merge-with into e&d->ms o->ms od->ms)]
241+
(into {} (map assign1) all->ms)))
241242

242243
(defn expand-modules
243244
"Given compiler :modules and a dependency sorted list of compiler inputs return

src/test/clojure/cljs/module_graph_tests.clj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,10 @@
142142
{:output-dir (:output-dir opts)
143143
:asset-path "/asset/js"
144144
:optimizations :none})
145-
{:shared ["/asset/js/events.js" "/asset/js/shared/a.js" "/asset/js/shared/b.js"],
145+
{:shared ["/asset/js/cljs/core.js" "/asset/js/events.js" "/asset/js/shared/a.js" "/asset/js/shared/b.js"],
146146
:page1 ["/asset/js/cljs/reader.js" "/asset/js/page1/a.js" "/asset/js/page1/b.js"],
147147
:page2 ["/asset/js/page2/a.js" "/asset/js/page2/b.js"],
148-
:cljs-base ["/asset/js/goog/base.js" "/asset/js/cljs/core.js"]}))
148+
:cljs-base ["/asset/js/goog/base.js"]}))
149149
(is (= (module-graph/modules->module-uris (modules opts) (inputs opts)
150150
{:output-dir (:output-dir opts)
151151
:asset-path "/asset/js"

0 commit comments

Comments
 (0)