feat(compartment-mapper): Make script or functor bundles #2707

kriskowal · 2025-01-30T23:40:41Z

Description

Toward better preserving the format of original source through the ModuleSource and censorship evasion transforms, we upgraded Babel and use a new code generator. The new code generator does not compose with Rollup. However, in the intervening years, we have reïmplemented every part of Rollup we find desirable and better in keeping with our integrity requirements. So, we are poised to reïmplement the nestedEvaluate and getExport formats using our own implementation. To that end, this change closes the feature parity gap needed to undertake that refactor.

We refactor makeBundle into makeScript and makeFuctor. We effectively rename makeBundle to makeScript for clarity. The script form surfaces as the endoScript bundler format and we expect functor to surface in an endoFunctor format, in addition to replacing the implementation of nestedEvaluate and getExport.

Both of these functions are extended to accept compile time useEvaluate, sourceUrlPrefix, and format, to exit to CommonJS require. We leave open the possibility of an esm or mjs format that would exit to import for host modules.

The new makeFunctor is analogous but isn’t suitable for <script> tags and instead allows the user to supply runtime options, including require for CommonJS format, evaluate if compiled with useEvaluate to override indirect eval, and sourceUrlPrefix which also in combination with useEvaluate overrides the compile time option by the same name.

Captures sourceDirname for each packaged compartment for improved sourceURL generation. So, if the directory name does not match the package name, the source URL will be more likely to united with the original sources if they are open in the developer’s IDE.

Moves bundle use strict pragma into evaluated function expression bodies for better composition with eval.

Improve error message for misconfigured require option.

Security Considerations

No impact.

Scaling Considerations

No impact.

Documentation Considerations

The new bundling features necessitate new API reference documentation, both those generated from TypeScript, and our hand-crafted README.

Testing Considerations

This change includes coverage for all essential combinations of the new bundler options, including compile time and runtime options and cases where the latter overrides the former.

Compatibility Considerations

Documentation included for was that the new features do not have parity with Rollup. This change is purely additive, but the upcoming changes to @endo/bundle-source will require a major version bump for not being comfortably equivalent to former behavior. We have confirmed that the new behavior produces a passing CI run in composition with usage patterns in Agoric SDK, with a couple preparatory changes to make the transition.

Upgrade Considerations

These changes will impact the formation of the Agoric SwingSet XSnap Supervisor Lockdown (bootstrap) script. They should not produce observable differences in behavior.

kriskowal · 2025-01-30T23:41:37Z

packages/compartment-mapper/src/bundle.js

@@ -6,7 +6,7 @@
 *   PrecompiledStaticModuleInterface


Reviewers, expand this large diff.

kriskowal · 2025-01-30T23:43:29Z

packages/compartment-mapper/src/bundle-json.js

+${json},
 `,
      getCells: () => `\
    { default: cell('default') },
 `,
      getReexportsWiring: () => '',
      getFunctorCall: () => `\
-  functors[${index}](cells[${index}].default.set);
+  cells[${index}].default.set(functors[${index}]);


This is a simplification, made obvious by changes to the other formats. The respective entry in the functors array is a contract between the caller and callee on a per-language basis. So, for JSON, there is no need to complicate things by making the JSON “functor” a functor. The caller just needs to set the value for the default export cell.

kriskowal · 2025-01-30T23:44:19Z

packages/compartment-mapper/src/archive-lite.js

@@ -176,7 +176,7 @@ const digestFromMap = async (powers, compartmentMap, options = {}) => {
    searchSuffixes,
    entryCompartmentName,
    entryModuleSpecifier,
-    exitModuleImportHook: consolidatedExitModuleImportHook,
+    importHook: consolidatedExitModuleImportHook,


This is a simplification of an internal type signature that is better aligned with reusable type definitions.

kriskowal · 2025-01-30T23:46:16Z

packages/compartment-mapper/src/bundle-cjs.js

+      sourceDirname,
+      record: { cjsFunctor, exports: exportsList = {} },
+    },
+    { useEvaluate = false, sourceUrlPrefix = undefined },


Whether to embed a sourceURL at all hinges on whether we useEvaluate, and in that case, we now defer the construction of the sourceURL to runtime to account for a runtime sourceUrlPrefix option. We could conceivably continue in this direction and inject computeSourceLocation at runtime, using the same option signature we use for importArchive.

kriskowal · 2025-01-30T23:47:03Z

packages/compartment-mapper/src/bundle-mjs.js

@@ -41,7 +43,7 @@ function observeImports(map, importName, importIndex) {
  for (const [name, observers] of map.get(importName)) {
    const cell = cells[importIndex][name];
    if (cell === undefined) {
-      throw new ReferenceError(\`Cannot import name \${name}\`);
+      throw new ReferenceError(\`Cannot import name \${name} (has \${Object.getOwnPropertyNames(cells[importIndex]).join(', ')})\`);


This proved useful in debugging in integration with Agoric SDK and might again one day.

kriskowal · 2025-01-30T23:56:57Z

packages/compartment-mapper/src/bundle.js

+  const namespaceCells = namespace => Object.fromEntries(
+    Object.getOwnPropertyNames(namespace)
+    .map(name => [name, {
+      get() {
+        return Reflect.get(namespace, name);
+      },
+      set() {
+        throw new TypeError('Non-writable export');
+      },
+      observe(observer) {
+        observer(Reflect.get(namespace, name));
+      },
+      enumerable: true,
+    }])
+  );


It’s not possible to perfectly lift a JavaScript object to a virtual module exports namespace object. This is a good place to question.

kriskowal · 2025-01-30T23:59:01Z

packages/compartment-mapper/src/bundle.js

+${
+  curryRuntimeOptions
+    ? ''
+    : `({
+${
+  format !== 'cjs'
+    ? ''
+    : `\
+  require: typeof require === 'function' ? require : undefined,
+`
+}\
+})`


There’s a design choice here, whether to curry both functors and options in the case where the options will be a fixed shape. I went with this approach, where we apply options in the generated code if curryRuntimeOptions is not specified, rather that inlining the options higher up. It could be argued.

kriskowal · 2025-01-30T23:59:41Z

packages/compartment-mapper/src/import-hook.js

- * @param {object} options
- * @param {Sources} [options.sources]
- * @param {Record<string, CompartmentDescriptor>} [options.compartmentDescriptors]
- * @param {boolean} [options.archiveOnly]
- * @param {HashFn} [options.computeSha512]
- * @param {Array<string>} [options.searchSuffixes] - Suffixes to search if the
- * unmodified specifier is not found.
- * Pass [] to emulate Node.js' strict behavior.
- * The default handles Node.js' CommonJS behavior.
- * Unlike Node.js, the Compartment Mapper lifts CommonJS up, more like a
- * bundler, and does not attempt to vary the behavior of resolution depending
- * on the language of the importing module.
- * @param {string} options.entryCompartmentName
- * @param {string} options.entryModuleSpecifier
- * @param {ExitModuleImportHook} [options.exitModuleImportHook]
- * @param {SourceMapHook} [options.sourceMapHook]


Drive-by refactor from unfinished typescript consolidation.

kriskowal · 2025-01-31T00:01:12Z

packages/compartment-mapper/src/import-hook.js

@@ -720,6 +712,7 @@ export function makeImportNowHookMaker(
          packageLocation,
          packageSources,
          readPowers,
+          archiveOnly,


I did not need to reveal a new option to makeBundle in order to indicate whether a sourceURL comment should be generated at the time a module is parsed off disk, because we already had archiveOnly, which is not orthogonal. I did get caught surprised that we do need to inject a sourceURL at parse time in order to preserve stack traces for importLocation, since that doesn’t go through the archiver or bundler.

kriskowal · 2025-01-31T00:02:49Z

packages/compartment-mapper/src/parse-cjs.js

@@ -41,7 +41,7 @@ export const parseCjs = (
   */
  const execute = (moduleEnvironmentRecord, compartment, resolvedImports) => {
    const functor = compartment.evaluate(
-      `(function (require, exports, module, __filename, __dirname) { ${source} //*/\n})\n//# sourceURL=${location}`,
+      `(function (require, exports, module, __filename, __dirname) { 'use strict'; ${source} })\n`,


Moving the pragma into each individual functor and the generated linker was needed to make bundles suitable inlined anywhere an expression is accepted, like (${source}).

gibson042

Initial feedback on the documentation. I have not yet reviewed any code changes.

gibson042 · 2025-02-02T18:46:28Z

packages/compartment-mapper/README.md

+import url from 'url';
+import fs from 'fs';
+import { makeBundle } from "@endo/compartment-mapper/bundle.js";
+import { makeReadPowers } from '../node-powers.js';
+const readPowers = makeReadPowers({ fs, url });
+const bundle = await makeBundle(readPowers, moduleLocation, options);


What is the origin and purpose of options in this snippet?

Leaving a stub:

const bundleOptions = {}; // if any

gibson042 · 2025-02-02T18:46:52Z

packages/compartment-mapper/README.md

+that subsumes `ses`, `@endo/compartment-mapper/import-archive.js`, and other
+parts of Endo, but is not as feature-complete as `importArchive`.
+
+```


Suggested change

```

```js

gibson042 · 2025-02-02T18:49:28Z

packages/compartment-mapper/README.md

+It should be sufficient to evaluate `bundle`.
+The completion value of evaluation is the emulated exports namespace of the
+entry module.


Suggested change

It should be sufficient to evaluate `bundle`.

The completion value of evaluation is the emulated exports namespace of the

entry module.

Evaluation of `bundle` returns the emulated exports namespace of the entry

module (e.g., `const namespace = eval(bundle);`).

gibson042 · 2025-02-02T18:51:46Z

packages/compartment-mapper/README.md

+:warning: Bundles do not support [live
+bindings](https://developer.mozilla.org/en-US/docs/Glossary/Binding), dynamic
+`import`, or `import.meta`.
+Bundles do not isolate modules to a compartment.


Suggested change

:warning: Bundles do not support [live

bindings](https://developer.mozilla.org/en-US/docs/Glossary/Binding), dynamic

`import`, or `import.meta`.

Bundles do not isolate modules to a compartment.

> [!WARNING]

> Bundles do not support [live

> bindings](https://developer.mozilla.org/en-US/docs/Glossary/Binding), dynamic

> `import`, or `import.meta`.

> Bundles do not isolate modules to a compartment.

(GFM Alerts)

gibson042 · 2025-02-02T18:58:42Z

packages/compartment-mapper/README.md

+  The default is `require` on `globalThis`.
+  The `require` function can be overridden with a curried runtime option.
+- `useEvaluate` (`boolean`, default `false`):
+  By default, a bundle has an array of functions for each bundled module.


I'm not sure how to interpret this sentence... I assume "array of functions" refers to module exports, but if a bundle is "a string of JavaScript suitable for eval" then what does it mean for a bundle to have such an array?

packages/compartment-mapper/README.md

gibson042 · 2025-02-02T19:37:16Z

packages/compartment-mapper/README.md

+- `curryRuntimeOptions` (`boolean`, default `false`):
+  By default, the completion value of a bundle is the exports namespace of
+  the entry module after all the modules have executed.
+  This is suitable for shims like `ses`, where the completion value is
+  discarded and any runtime options must be communicated through the
+  global environment, like `process.env` or the presence of a `require` function.
+  Currying runtime options changes the completion value to a function that receives
+  runtime options and then returns the exports namespace of the entry module.


I don't think "currying" is technically correct here, nor does it communicate to me what's really going on (the indirection from "bundle represents a [non-function] module namespace" to "bundle represents a function that returns a module namespace"). I don't have a better name suggestion, but I wonder if the indirection might be better represented by a sibling function distinct from makeBundle rather than an option for it (and I really an wondering; the answer might be "no").

Per out-of-band conversation, we found that we can avoid the option name by splitting makeBundle into makeScript and makeFunctor, where the latter provides an opportunity to apply runtime options.

packages/compartment-mapper/README.md

gibson042

It's always difficult to review code like this, and I don't think I'm sufficiently familiar with compartment-mapper to actually detect problems. If you'd like a more substantial review, we should probably pair up for a walkthrough.

packages/compartment-mapper/src/types/external.ts

packages/compartment-mapper/src/types/compartment-map-schema.ts

packages/compartment-mapper/src/parse-archive-cjs.js

packages/compartment-mapper/test/bundle.test.js

kriskowal · 2025-02-03T23:28:20Z

packages/compartment-mapper/README.md

+import url from 'url';
+import fs from 'fs';
+import { makeBundle } from "@endo/compartment-mapper/bundle.js";
+import { makeReadPowers } from '../node-powers.js';
+const readPowers = makeReadPowers({ fs, url });
+const bundle = await makeBundle(readPowers, moduleLocation, options);


Leaving a stub:

const bundleOptions = {}; // if any

packages/compartment-mapper/NEWS.md

packages/compartment-mapper/src/bundle-cjs.js

gibson042

❤️
LGTM after minor tweaks.

packages/compartment-mapper/README.md

packages/compartment-mapper/src/bundle-lite.js

…Bundle In service to reimplementing the internals of the `nestedEvaluate` and `getExport` bundle formats in `@endo/bundle-source`: Add `makeBundle` options for `useEvaluate`, `sourceUrlPrefix`, exit to CommonJS `require`, and `curryRuntimeOptions`, with curried runtime options for `evaluate`, `require`, and `sourceUrlPrefix`. Capture `sourceDirname` for each packaged compartment for improved `sourceURL` generation. Move bundle `use strict` pragma into evaluated function expression bodies for better composition with `eval`. Improve error message for misconfigured `require` option.

…dial replacement/manipulation Ref #2707 (comment)

kriskowal requested a review from gibson042 January 30, 2025 23:40

kriskowal commented Jan 31, 2025

View reviewed changes

kriskowal force-pushed the kriskowal-curry-bundle branch from d893bbf to 3805a35 Compare January 31, 2025 00:05

kriskowal mentioned this pull request Jan 31, 2025

feat(compartment-mapper): Make bundle using named evaluator #2705

Closed

gibson042 reviewed Feb 2, 2025

View reviewed changes

gibson042 reviewed Feb 3, 2025

View reviewed changes

kriskowal commented Feb 6, 2025

View reviewed changes

kriskowal changed the title ~~feat(compartment-mapper): Curried bundles~~ feat(compartment-mapper): Make script or functor bundles Feb 6, 2025

kriskowal force-pushed the kriskowal-curry-bundle branch from 3805a35 to e9c7eef Compare February 6, 2025 05:03

kriskowal requested a review from gibson042 February 6, 2025 05:04

kriskowal force-pushed the kriskowal-curry-bundle branch from e9c7eef to b5907d6 Compare February 6, 2025 05:07

gibson042 approved these changes Feb 12, 2025

View reviewed changes

kriskowal force-pushed the kriskowal-curry-bundle branch from b5907d6 to 90f7846 Compare February 15, 2025 01:12

kriskowal force-pushed the kriskowal-curry-bundle branch from 90f7846 to ee87476 Compare February 15, 2025 01:20

kriskowal enabled auto-merge February 15, 2025 01:24

kriskowal merged commit bc34d40 into master Feb 15, 2025
15 checks passed

kriskowal deleted the kriskowal-curry-bundle branch February 15, 2025 01:31

gibson042 added a commit that referenced this pull request Feb 15, 2025

fix(compartment-mapper): Make bundled code more robust against primor…

3afc66c

…dial replacement/manipulation Ref #2707 (comment)

gibson042 mentioned this pull request Feb 15, 2025

fix(compartment-mapper): Make bundled code more robust against primordial replacement/manipulation #2725

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compartment-mapper): Make script or functor bundles #2707

feat(compartment-mapper): Make script or functor bundles #2707

kriskowal commented Jan 30, 2025 •

edited

Loading

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 30, 2025

kriskowal Jan 31, 2025

kriskowal Jan 31, 2025

gibson042 left a comment

gibson042 Feb 2, 2025

kriskowal Feb 3, 2025

gibson042 Feb 2, 2025

gibson042 Feb 2, 2025

gibson042 Feb 2, 2025

gibson042 Feb 2, 2025

kriskowal Feb 6, 2025

gibson042 Feb 2, 2025

kriskowal Feb 6, 2025

gibson042 left a comment

kriskowal Feb 3, 2025

gibson042 left a comment

-:warning: Bundles do not support [live
-bindings](https://developer.mozilla.org/en-US/docs/Glossary/Binding), dynamic
-`import`, or `import.meta`.
-Bundles do not isolate modules to a compartment.
+> [!WARNING]
+> Bundles do not support [live
+> bindings](https://developer.mozilla.org/en-US/docs/Glossary/Binding), dynamic
+> `import`, or `import.meta`.
+> Bundles do not isolate modules to a compartment.

feat(compartment-mapper): Make script or functor bundles #2707

feat(compartment-mapper): Make script or functor bundles #2707

Conversation

kriskowal commented Jan 30, 2025 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Compatibility Considerations

Upgrade Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

kriskowal commented Jan 30, 2025 •

edited

Loading