Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ses): Opt out of namespace boxes #2380

Merged
merged 4 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ User-visible changes in SES:
the stacktrace line-numbers point back into the original
source, as they do on Node without SES.

- Adds a `__noNamespaceBox__` option that aligns the behavior of the `import`
method on SES `Compartment` with the behavior of XS and the behavior we will
champion for compartment standards.
All use of `Compartment` should migrate to use this option as the standard
behavior will be enabled by default with the next major version of SES.

# v1.5.0 (2024-05-06)

- Adds `importNowHook` to the `Compartment` options. The compartment will invoke the hook whenever it encounters a missing dependency while running `compartmentInstance.importNow(specifier)`, which cannot use an asynchronous `importHook`.
Expand Down
17 changes: 16 additions & 1 deletion packages/ses/src/compartment.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ export const CompartmentPrototype = {
},

async import(specifier) {
const { noNamespaceBox } = weakmapGet(privateFields, this);

if (typeof specifier !== 'string') {
throw TypeError('first argument of import() must be a string');
}
Expand All @@ -117,6 +119,11 @@ export const CompartmentPrototype = {
/** @type {Compartment} */ (this),
specifier,
);
if (noNamespaceBox) {
return namespace;
}
// Legacy behavior: box the namespace object so that thenable modules
// do not get coerced into a promise accidentally.
return { namespace };
},
);
Expand Down Expand Up @@ -172,7 +179,11 @@ export const makeCompartmentConstructor = (
markVirtualizedNativeFunction,
parentCompartment = undefined,
) => {
function Compartment(endowments = {}, moduleMap = {}, options = {}) {
function Compartment(
endowmentsOption = {},
moduleMapOption = {},
options = {},
) {
if (new.target === undefined) {
throw TypeError(
"Class constructor Compartment cannot be invoked without 'new'",
Expand All @@ -189,8 +200,11 @@ export const makeCompartmentConstructor = (
importNowHook,
moduleMapHook,
importMetaHook,
__noNamespaceBox__: noNamespaceBox = false,
} = options;
const globalTransforms = [...transforms, ...__shimTransforms__];
const endowments = { __proto__: null, ...endowmentsOption };
const moduleMap = { __proto__: null, ...moduleMapOption };

// Map<FullSpecifier, ModuleCompartmentRecord>
const moduleRecords = new Map();
Expand Down Expand Up @@ -249,6 +263,7 @@ export const makeCompartmentConstructor = (
deferredExports,
instances,
parentCompartment,
noNamespaceBox,
});
}

Expand Down
17 changes: 13 additions & 4 deletions packages/ses/test/compartment-transforms.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,13 @@ test('transforms do not apply to imported modules', async t => {
const resolveHook = () => '';
const importHook = () =>
new ModuleSource('export default "Farewell, World!";');
const c = new Compartment({}, {}, { transforms, resolveHook, importHook });
const c = new Compartment(
{},
{},
{ transforms, resolveHook, importHook, __noNamespaceBox__: true },
);

const { namespace } = await c.import('any-string-here');
const namespace = await c.import('any-string-here');
const { default: greeting } = namespace;

t.is(greeting, 'Farewell, World!');
Expand Down Expand Up @@ -82,10 +86,15 @@ test('__shimTransforms__ do apply to imported modules', async t => {
const c = new Compartment(
{},
{},
{ __shimTransforms__: transforms, resolveHook, importHook },
{
__shimTransforms__: transforms,
__noNamespaceBox__: true,
resolveHook,
importHook,
},
);

const { namespace } = await c.import('any-string-here');
const namespace = await c.import('any-string-here');
const { default: greeting } = namespace;

t.is(greeting, 'Hello, World!');
Expand Down
104 changes: 65 additions & 39 deletions packages/ses/test/import-cjs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,13 @@ test('import a CommonJS module with exports assignment', async t => {
);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const module = compartment.module('.');
const {
namespace: { meaning },
} = await compartment.import('.');
const { meaning } = await compartment.import('.');

t.is(meaning, 42, 'exports seen');
t.is(module.meaning, 42, 'exports seen through deferred proxy');
Expand All @@ -109,11 +111,13 @@ test('import a CommonJS module with exports replacement', async t => {
);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const module = compartment.module('.');
const {
namespace: { default: meaning },
} = await compartment.import('.');
const { default: meaning } = await compartment.import('.');

t.is(meaning, 42, 'exports seen');
t.is(module.default, 42, 'exports seen through deferred proxy');
Expand Down Expand Up @@ -144,10 +148,12 @@ test('CommonJS module imports CommonJS module by name', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -178,8 +184,12 @@ test('CommonJS module imports CommonJS module as default', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const { namespace } = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const namespace = await compartment.import('./odd');
const { default: odd } = namespace;

t.is(odd(1), true);
Expand Down Expand Up @@ -211,10 +221,12 @@ test('ESM imports CommonJS module as default', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { default: odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { default: odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -245,10 +257,12 @@ test('ESM imports CommonJS module as star', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { default: odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { default: odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -281,10 +295,12 @@ test('ESM imports CommonJS module with replaced exports as star', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { default: odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { default: odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -315,10 +331,12 @@ test('ESM imports CommonJS module by name', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { default: odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { default: odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -349,10 +367,12 @@ test('CommonJS module imports ESM as default', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { default: odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { default: odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -385,10 +405,12 @@ test('CommonJS module imports ESM by name', async t => {
throw Error(`Cannot load module ${specifier}`);
};

const compartment = new Compartment({}, {}, { resolveHook, importHook });
const {
namespace: { odd },
} = await compartment.import('./odd');
const compartment = new Compartment(
{},
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
const { odd } = await compartment.import('./odd');

t.is(odd(1), true);
t.is(odd(2), false);
Expand Down Expand Up @@ -440,7 +462,11 @@ test('cross import ESM and CommonJS modules', async t => {
throw Error(`Cannot load module for specifier ${specifier}`);
};

const compartment = new Compartment({ t }, {}, { resolveHook, importHook });
const compartment = new Compartment(
{ t },
{},
{ resolveHook, importHook, __noNamespaceBox__: true },
);
await compartment.import('./src/main.js');
});

Expand Down
18 changes: 12 additions & 6 deletions packages/ses/test/import-gauntlet.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,11 @@ test('import all from module', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.default.a, 10);
t.is(namespace.default.b, 20);
Expand All @@ -78,10 +79,11 @@ test('import named exports from me', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.default.fizz, 10);
t.is(namespace.default.buzz, 20);
Expand All @@ -106,10 +108,11 @@ test('import color from module', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.color, 'blue');
});
Expand All @@ -132,10 +135,11 @@ test('import and reexport', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.qux, 42);
});
Expand All @@ -159,10 +163,11 @@ test('import and export all', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.alpha, 0);
t.is(namespace.omega, 23);
Expand All @@ -189,10 +194,11 @@ test('live binding', async t => {
{
resolveHook: resolveNode,
importHook: makeImportHook('https://example.com'),
__noNamespaceBox__: true,
},
);

const { namespace } = await compartment.import('./main.js');
const namespace = await compartment.import('./main.js');

t.is(namespace.default, 'Hello, World!');
});
Expand Down
Loading
Loading