Skip to content

Commit

Permalink
Merge pull request #1972 from endojs/module-namespace-object
Browse files Browse the repository at this point in the history
refactor(ses): Improve module namespace object representation and documentation

## Description

### Module namespace object representation

Adds a `Symbol.toStringTag` property to the `proxiedExports` object in order to make it appear more like an ESM Module Namespace object. This necessitated changes in `ses` and corresponding changes in `compartment-mapper`.

During testing, the output of `console.log()` is indistinguishable from an actual ESM Module Namespace object in Chromium. In Node, however, we would need to outsmart `node:util.inspect()` by implementing the `@@node:util.inspect.custom` method on our object. (The call chain for logging non-strings in Node being: `console.log()` -> `node:util.format()` -> `node:util.inspect()`.)

### Module namespace proxy naming and documentation

Renames deferred exports `proxiedExports` property to `exportsTarget` and adds a docstring to `getDeferredExports` to explain the purpose of the deferred export values: `exportsTarget`, `exportsProxy`, and `activate`.
  • Loading branch information
rekmarks authored Jan 17, 2024
2 parents 5792949 + 41fe702 commit 0d8bed8
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 52 deletions.
10 changes: 9 additions & 1 deletion packages/compartment-mapper/src/bundle-cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ const runtime = function wrapCjsFunctor(num) {
.filter(k => moduleCells[k] === undefined)
.map(k => (moduleCells[k] = cell(k, cModule.exports[k])));
// Update the star cell from all cells.
const starExports = Object.create(null);
const starExports = Object.create(null, {
// Make this appear like an ESM module namespace object.
[Symbol.toStringTag]: {
value: 'Module',
writable: false,
enumerable: false,
configurable: false,
},
});
Object.keys(moduleCells)
.filter(k => k !== '*')
.map(k => Object.defineProperty(starExports, k, moduleCells[k]));
Expand Down
11 changes: 10 additions & 1 deletion packages/compartment-mapper/src/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,16 @@ ${''.concat(...modules.map(m => m.bundlerKit.getCells()))}\
${''.concat(...modules.map(m => m.bundlerKit.getReexportsWiring()))}\
const namespaces = cells.map(cells => Object.freeze(Object.create(null, cells)));
const namespaces = cells.map(cells => Object.freeze(Object.create(null, {
...cells,
// Make this appear like an ESM module namespace object.
[Symbol.toStringTag]: {
value: 'Module',
writable: false,
enumerable: false,
configurable: false,
},
})));
for (let index = 0; index < namespaces.length; index += 1) {
cells[index]['*'] = cell('*', namespaces[index]);
Expand Down
11 changes: 11 additions & 0 deletions packages/compartment-mapper/test/scaffold.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,3 +420,14 @@ export function scaffold(
});
}
}

// Modifies the given object to make it appear to be an ESM module namespace object.
export const moduleify = obj => {
Object.defineProperty(obj, Symbol.toStringTag, {
value: 'Module',
writable: false,
enumerable: false,
configurable: false,
});
return Object.setPrototypeOf(obj, null);
};
17 changes: 9 additions & 8 deletions packages/compartment-mapper/test/test-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import url from 'url';
import test from 'ava';
import { makeBundle, makeArchive, parseArchive } from '../index.js';
import { makeReadPowers } from '../node-powers.js';
import { moduleify } from './scaffold.js';

const fixture = new URL(
'fixtures-0/node_modules/bundle/main.js',
Expand All @@ -17,40 +18,40 @@ const expectedLog = [
'are other fingers.',
'dependency',
'foo',
{
moduleify({
c: 'sea',
i: 'eye',
q: 'cue',
k: 'que',
u: 'you',
y: 'why',
},
{
}),
moduleify({
c: 'sea',
i: 'eye',
q: 'cue',
k: 'que',
u: 'you',
y: 'why',
},
}),
'fizz',
'buzz',
'blue',
'qux',
'#777',
{
moduleify({
red: '#f00',
green: '#0f0',
blue: '#00f',
},
{
}),
moduleify({
default: {
zzz: 1,
fromMjs: 'foo',
},
fromMjs: 'foo',
zzz: 1,
},
}),
];

test('bundles work', async t => {
Expand Down
14 changes: 7 additions & 7 deletions packages/compartment-mapper/test/test-policy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// import "./ses-lockdown.js";
import 'ses';
import test from 'ava';
import { scaffold, sanitizePaths } from './scaffold.js';
import { moduleify, scaffold, sanitizePaths } from './scaffold.js';

function combineAssertions(...assertionFunctions) {
return async (...args) => {
Expand Down Expand Up @@ -86,7 +86,7 @@ const anyPolicy = {
};

const defaultExpectations = {
namespace: {
namespace: moduleify({
alice: {
bluePill: 'undefined',
redPill: 'number',
Expand All @@ -102,23 +102,23 @@ const defaultExpectations = {
scopedBob: { scoped: 1 },
builtins: '{"a":1,"b":2,"default":{"a":1,"b":2}}',
builtins2: '{"c":3,"default":{"c":3}}',
},
}),
};
const anyExpectations = {
namespace: {
namespace: moduleify({
...defaultExpectations.namespace,
carol: { bluePill: 'number', redPill: 'number', purplePill: 'number' },
},
}),
};
const powerlessCarolExpectations = {
namespace: {
namespace: moduleify({
...defaultExpectations.namespace,
carol: {
bluePill: 'undefined',
redPill: 'undefined',
purplePill: 'undefined',
},
},
}),
};

const makeResultAssertions =
Expand Down
20 changes: 10 additions & 10 deletions packages/ses/src/module-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const makeThirdPartyModuleInstance = (
moduleSpecifier,
resolvedImports,
) => {
const { exportsProxy, proxiedExports, activate } = getDeferredExports(
const { exportsProxy, exportsTarget, activate } = getDeferredExports(
compartment,
weakmapGet(compartmentPrivateFields, compartment),
moduleAliases,
Expand All @@ -51,7 +51,7 @@ export const makeThirdPartyModuleInstance = (
);
}
arrayForEach(staticModuleRecord.exports, name => {
let value = proxiedExports[name];
let value = exportsTarget[name];
const updaters = [];

const get = () => value;
Expand All @@ -63,7 +63,7 @@ export const makeThirdPartyModuleInstance = (
}
};

defineProperty(proxiedExports, name, {
defineProperty(exportsTarget, name, {
get,
set,
enumerable: true,
Expand All @@ -75,9 +75,9 @@ export const makeThirdPartyModuleInstance = (
update(value);
};
});
// This is enough to support import * from cjs - the '*' field doesn't need to be in exports nor proxiedExports because import will only ever access it via notifiers
// This is enough to support import * from cjs - the '*' field doesn't need to be in exports nor exportsTarget because import will only ever access it via notifiers
notifiers['*'] = update => {
update(proxiedExports);
update(exportsTarget);
};
}

Expand All @@ -97,7 +97,7 @@ export const makeThirdPartyModuleInstance = (
try {
// eslint-disable-next-line @endo/no-polymorphic-call
staticModuleRecord.execute(
proxiedExports,
exportsTarget,
compartment,
resolvedImports,
);
Expand Down Expand Up @@ -143,7 +143,7 @@ export const makeModuleInstance = (

const { __shimTransforms__, importMetaHook } = compartmentFields;

const { exportsProxy, proxiedExports, activate } = getDeferredExports(
const { exportsProxy, exportsTarget, activate } = getDeferredExports(
compartment,
compartmentFields,
moduleAliases,
Expand Down Expand Up @@ -342,7 +342,7 @@ export const makeModuleInstance = (
);

const notifyStar = update => {
update(proxiedExports);
update(exportsTarget);
};
notifiers['*'] = notifyStar;

Expand Down Expand Up @@ -435,10 +435,10 @@ export const makeModuleInstance = (
// and the string must correspond to a valid identifier, sorting these
// properties works for this specific case.
arrayForEach(arraySort(keys(exportsProps)), k =>
defineProperty(proxiedExports, k, exportsProps[k]),
defineProperty(exportsTarget, k, exportsProps[k]),
);

freeze(proxiedExports);
freeze(exportsTarget);
activate();
}

Expand Down
56 changes: 42 additions & 14 deletions packages/ses/src/module-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
reflectHas,
reflectIsExtensible,
reflectPreventExtensions,
toStringTagSymbol,
weakmapSet,
} from './commons.js';
import { assert } from './error/assert.js';
Expand All @@ -51,13 +52,21 @@ const { quote: q } = assert;
//
export const deferExports = () => {
let active = false;
const proxiedExports = create(null);
const exportsTarget = create(null, {
// Make this appear like an ESM module namespace object.
[toStringTagSymbol]: {
value: 'Module',
writable: false,
enumerable: false,
configurable: false,
},
});
return freeze({
activate() {
active = true;
},
proxiedExports,
exportsProxy: new Proxy(proxiedExports, {
exportsTarget,
exportsProxy: new Proxy(exportsTarget, {
get(_target, name, receiver) {
if (!active) {
throw TypeError(
Expand All @@ -66,7 +75,7 @@ export const deferExports = () => {
)} of module exports namespace, the module has not yet begun to execute`,
);
}
return reflectGet(proxiedExports, name, receiver);
return reflectGet(exportsTarget, name, receiver);
},
set(_target, name, _value) {
throw TypeError(
Expand All @@ -81,7 +90,7 @@ export const deferExports = () => {
)}, the module has not yet begun to execute`,
);
}
return reflectHas(proxiedExports, name);
return reflectHas(exportsTarget, name);
},
deleteProperty(_target, name) {
throw TypeError(
Expand All @@ -94,7 +103,7 @@ export const deferExports = () => {
'Cannot enumerate keys, the module has not yet begun to execute',
);
}
return ownKeys(proxiedExports);
return ownKeys(exportsTarget);
},
getOwnPropertyDescriptor(_target, name) {
if (!active) {
Expand All @@ -104,23 +113,23 @@ export const deferExports = () => {
)}, the module has not yet begun to execute`,
);
}
return reflectGetOwnPropertyDescriptor(proxiedExports, name);
return reflectGetOwnPropertyDescriptor(exportsTarget, name);
},
preventExtensions(_target) {
if (!active) {
throw TypeError(
'Cannot prevent extensions of module exports namespace, the module has not yet begun to execute',
);
}
return reflectPreventExtensions(proxiedExports);
return reflectPreventExtensions(exportsTarget);
},
isExtensible() {
if (!active) {
throw TypeError(
'Cannot check extensibility of module exports namespace, the module has not yet begun to execute',
);
}
return reflectIsExtensible(proxiedExports);
return reflectIsExtensible(exportsTarget);
},
getPrototypeOf(_target) {
return null;
Expand All @@ -147,11 +156,30 @@ export const deferExports = () => {
});
};

// `getDeferredExports` memoizes the creation of a deferred module exports
// namespace proxy for any abritrary full specifier in a compartment.
// It also records the compartment and specifier affiliated with that module
// exports namespace proxy so it can be used as an alias into another
// compartment when threaded through a compartment's `moduleMap` argument.
/**
* @typedef {object} DeferredExports
* @property {Record<string, any>} exportsTarget - The object to which a
* module's exports will be added.
* @property {Record<string, any>} exportsProxy - A proxy over the `exportsTarget`,
* used to expose its "exports" to other compartments.
* @property {() => void} activate - Activate the `exportsProxy` such that it can
* be used as a module namespace object.
*/

/**
* Memoizes the creation of a deferred module exports namespace proxy for any
* arbitrary full specifier in a compartment. It also records the compartment
* and specifier affiliated with that module exports namespace proxy so it
* can be used as an alias into another compartment when threaded through
* a compartment's `moduleMap` argument.
*
* @param {*} compartment - The compartment to retrieve deferred exports from.
* @param {*} compartmentPrivateFields - The private fields of the compartment.
* @param {*} moduleAliases - The module aliases of the compartment.
* @param {string} specifier - The module specifier to retrieve deferred exports for.
* @returns {DeferredExports} - The deferred exports for the module specifier of
* the compartment.
*/
export const getDeferredExports = (
compartment,
compartmentPrivateFields,
Expand Down
Loading

0 comments on commit 0d8bed8

Please sign in to comment.