Skip to content

Commit 9504213

Browse files
adrian-niculescuNathanWalker
authored andcommitted
fix: normalize "." and ".." in resolved module paths to dedupe modules (#1977)
ResolveModuleCallback builds the on-disk path for a relative specifier without collapsing "." and ".." segments, so the same file imported as "./x" from /a/b and as "../x" from /a/b/c lands under two different registry keys. stat() resolves the segments so both locate the file, but the differing keys make V8 compile and evaluate the module twice: two instances with separate state, and import.meta.dirname becomes "/a/b/c/.." for the ".." spelling. Normalize the resolved path before it becomes the registry key, the same way the HTTP branch canonicalizes through CanonicalizeHttpUrlKey. The pass is lexical and runs for every on-disk candidate kind, and is a no-op for already-canonical paths.
1 parent a0859d4 commit 9504213

5 files changed

Lines changed: 45 additions & 0 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Singleton module: an ES module is evaluated once, so every importer must
2+
// observe this same `state` object regardless of how the specifier spelled the
3+
// path to this file.
4+
export const state = { count: 0 };
5+
6+
export function increment() {
7+
state.count++;
8+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Reaches the same counter.mjs one directory up: "../counter.mjs".
2+
import { state, increment } from "../counter.mjs";
3+
4+
increment();
5+
6+
export const seenState = state;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Reaches counter.mjs as a same-directory sibling: "./counter.mjs".
2+
import { state, increment } from "./counter.mjs";
3+
4+
increment();
5+
6+
export const seenState = state;

test-app/app/src/main/assets/app/tests/testESModules.mjs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,21 @@ describe("ES Modules", () => {
6161
(err) => { expect(err).toBeUndefined(); done(); }
6262
);
6363
});
64+
65+
it("resolves './x' and '../x' to a single shared module instance", (done) => {
66+
Promise.all([
67+
import("~/esm-dedup/viaSameDir.mjs"),
68+
import("~/esm-dedup/nested/viaParentDir.mjs"),
69+
]).then(
70+
([sameDir, parentDir]) => {
71+
// The same counter.mjs is reached as "./counter.mjs" and as
72+
// "../counter.mjs"; it must be one module instance sharing one state
73+
// object, incremented once per importer.
74+
expect(sameDir.seenState).toBe(parentDir.seenState);
75+
expect(sameDir.seenState.count).toBe(2);
76+
done();
77+
},
78+
(err) => { expect(err).toBeUndefined(); done(); }
79+
);
80+
});
6481
});

test-app/runtime/src/main/cpp/ModuleInternalCallbacks.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,14 @@ v8::MaybeLocal<v8::Module> ResolveModuleCallback(v8::Local<v8::Context> context,
13801380
if (found) break;
13811381
}
13821382

1383+
// Canonicalize "." / ".." segments so a file reached through different
1384+
// spellings (e.g. "./x" from /a/b and "../x" from /a/b/c both name /a/b/x)
1385+
// maps to one registry key and is compiled once. The HTTP branch
1386+
// canonicalizes via CanonicalizeHttpUrlKey.
1387+
if (found) {
1388+
absPath = NormalizeDotSegments(absPath);
1389+
}
1390+
13831391
// 6) Handle special cases if file not found
13841392
if (!found) {
13851393
// Check for Node.js built-in modules

0 commit comments

Comments
 (0)