Skip to content

Commit 49b6c37

Browse files
committed
fix(ses): re-exported names available in imported modules
This fixes a situation in which module _A_ re-exports name _X_ from module _B_, and then imports module _C_, which in turn imports _X_ from _A_. Originally encountered in the source code for `terser`.
1 parent e7f0f29 commit 49b6c37

File tree

2 files changed

+50
-14
lines changed

2 files changed

+50
-14
lines changed

packages/ses/src/module-instance.js

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -372,26 +372,14 @@ export const makeModuleInstance = (
372372
// export * cannot export default.
373373
const candidateAll = create(null);
374374
candidateAll.default = false;
375-
for (const [specifier, importUpdaters] of updateRecord) {
375+
376+
for (const [specifier] of updateRecord) {
376377
const instance = mapGet(importedInstances, specifier);
377378
// The module instance object is an internal literal, does not bind this,
378379
// and never revealed outside the SES shim.
379380
// There are two instantiation sites for instances and they are both in
380381
// this module.
381-
// eslint-disable-next-line @endo/no-polymorphic-call
382-
instance.execute(); // bottom up cycle tolerant
383382
const { notifiers: importNotifiers } = instance;
384-
for (const [importName, updaters] of importUpdaters) {
385-
const importNotify = importNotifiers[importName];
386-
if (!importNotify) {
387-
throw SyntaxError(
388-
`The requested module '${specifier}' does not provide an export named '${importName}'`,
389-
);
390-
}
391-
for (const updater of updaters) {
392-
importNotify(updater);
393-
}
394-
}
395383
if (arrayIncludes(exportAlls, specifier)) {
396384
// Make all these imports candidates.
397385
// Note names don't change in reexporting all
@@ -433,6 +421,26 @@ export const makeModuleInstance = (
433421
}
434422
}
435423

424+
// this must happen _after_ the previous loop so that re-exported names
425+
// have associated import notifiers
426+
for (const [specifier, importUpdaters] of updateRecord) {
427+
const instance = mapGet(importedInstances, specifier);
428+
// eslint-disable-next-line @endo/no-polymorphic-call
429+
instance.execute(); // bottom up cycle tolerant
430+
const { notifiers: importNotifiers } = instance;
431+
for (const [importName, updaters] of importUpdaters) {
432+
const importNotify = importNotifiers[importName];
433+
if (!importNotify) {
434+
throw SyntaxError(
435+
`The requested module '${specifier}' does not provide an export named '${importName}'`,
436+
);
437+
}
438+
for (const updater of updaters) {
439+
importNotify(updater);
440+
}
441+
}
442+
}
443+
436444
// Sort the module exports namespace as per spec.
437445
// The module exports namespace will be wrapped in a module namespace
438446
// exports proxy which will serve as a "module exports namespace exotic

packages/ses/test/import-gauntlet.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,31 @@ test('this in module scope must be undefined', async t => {
406406

407407
await compartment.import('./index.js');
408408
});
409+
410+
test('re-exported names should available in imported modules', async t => {
411+
t.plan(1);
412+
413+
const makeImportHook = makeNodeImporter({
414+
'https://example.com/index.js': `
415+
export { a } from './a.js';
416+
import './c.js';
417+
`,
418+
'https://example.com/a.js': `
419+
export const a = 'a';
420+
`,
421+
'https://example.com/c.js': `
422+
import { a } from './index.js';
423+
t.is(a, 'a', 're-exported name "a" from importing module should be string "a"');
424+
`,
425+
});
426+
const importHook = makeImportHook('https://example.com');
427+
428+
const compartment = new Compartment({
429+
globals: { t },
430+
resolveHook: resolveNode,
431+
importHook,
432+
__options__: true,
433+
});
434+
435+
await compartment.import('./index.js');
436+
});

0 commit comments

Comments
 (0)