fix(compartment-mapper,bundle-source): async parser support

[boneskull]

This fixes the half-implemented type defs and support for async parsers by introducing AsyncParserImplementation.

We needed sync-and-async-specific parse-mappers in map-parser.js and some type guards.

A sync-parser-specific SyncParserForLanguage is introduced as well.

Bonus: added some tests for import() in ESM to @endo/compartment-mapper.

[boneskull]

📚 Pull Request Stack

• branch: master
  • fix(compartment-mapper,bundle-source): async parser support #3186 - branch: boneskull/async-parser (this PR)
    • feat(evasive-transform): expose makeEvasiveTransformVisitor #3217 - branch: boneskull/evasive-pipeline2
      • fix(compartment-mapper): policy, parser type fixes #3247 - branch: boneskull/fix-module-source-types2
        • feat(module-source): fix types, expose analyzer.js subpath #3218 - branch: boneskull/module-source-pipeline2
          • feat(module-source): expose AST-based CJS parser #3220 - branch: boneskull/cjs-ast-parser2
            • feat(compartment-mapper): expose findUnknownCanonicalNames #3221 - branch: boneskull/export-find-canonical-names2
              • feat(parser-pipeline): create @endo/parser-pipeline #3158 - branch: boneskull/ast-service

---

Managed by gh-stack

[changeset-bot]

🦋 Changeset detected

Latest commit: d5be9d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@endo/compartment-mapper	Patch
@endo/bundle-source	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

This PR completes and formalizes async-parser support in @endo/compartment-mapper and @endo/bundle-source, splitting sync vs async parser implementations/types and updating the parser-mapping pipeline accordingly. It also adds a fixture-based test covering import() in ESM for compartment-mapper.

Changes:

• Introduces AsyncParserImplementation (and related type updates) to properly model async parsers alongside sync parsers.
• Refactors map-parser.js to generate distinct sync vs async extension parsers and adds type guards to select the correct pipeline.
• Adds a new ESM dynamic import() fixture and AVA test in @endo/compartment-mapper.

Reviewed changes

Copilot reviewed 8 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/compartment-mapper/test/fixtures-dynamic-import-esm/node_modules/app/package.json	Adds ESM fixture package metadata for dynamic import test.
packages/compartment-mapper/test/fixtures-dynamic-import-esm/node_modules/app/index.js	Adds ESM entry that uses import() dynamically.
packages/compartment-mapper/test/fixtures-dynamic-import-esm/node_modules/app/foo.js	Adds simple default export for the dynamic import target.
packages/compartment-mapper/test/dynamic-import-esm.test.js	Adds AVA test exercising ESM dynamic import via scaffold.
packages/compartment-mapper/src/types/internal.ts	Narrows internal operator type export surface while preserving unions.
packages/compartment-mapper/src/types/external.ts	Defines AsyncParserImplementation, tightens sync parser typing, and adds SyncParserForLanguage.
packages/compartment-mapper/src/map-parser.js	Splits sync/async parser generation and adds shared language resolution + guards.
packages/compartment-mapper/src/link.js	Simplifies access to heuristicImports without unnecessary cast.
packages/compartment-mapper/src/import-hook.js	Adds a parse-fn type guard and uses it to enforce sync-only importNow paths.
packages/bundle-source/src/endo.js	Updates TypeScript-related parsers to be explicitly async parser implementations.
.changeset/fuzzy-hornets-shout.md	Declares patch releases for affected packages.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[naugtur]

(should) reuse these from commons

[boneskull]

can be another PR

[naugtur]

(untrusted input) result of resolveLanguage is trusted later on as if it was an internal key, but it's input from untrusted source. index.toString or index.__proto__ are possible filenames

[boneskull]

index--the Record of transform functions--is also untrusted input, so I'm not sure what you're proposing we do.

[naugtur]

Totally different category. Extension is untrusted input for the user of Endo, it's coming from untrusted packages. We need to either use a fixed listy of what is available and throw early here, or very carefully handle the return value to not match prototype fields. The former would be my choice

[boneskull]

Where is it being used as if it is trusted? has omits unowned properties.

[boneskull]

I'm pretty sure this is not a problem in practice, but I've added some extra defense anyway:

const resolveLanguage = (
  specifier,
  location,
  languageForExtension,
  languageForModuleSpecifier,
) => {
  const extension = parseExtension(location);
  if (
    !extensionImpliesLanguage(extension) &&
    has(languageForModuleSpecifier, specifier)
  ) {
    return languageForModuleSpecifier[specifier];
  }
  // new check
  if (has(languageForExtension, extension)) {
    return languageForExtension[extension];
  }
  return extension;
};

So resolveLanguage cannot return anything untoward on languageForExtension. Further:

const validateLanguageForExtension = (
  languageForExtension,
  parserForLanguage,
) => {
  const languageForExtensionEntries = [];
  const problems = [];
  for (const [extension, language] of entries(languageForExtension)) {
    if (has(parserForLanguage, language)) {
      languageForExtensionEntries.push([extension, language]);
    } else {
      problems.push(`${q(language)} for extension ${q(extension)}`);
    }
  }
  if (problems.length > 0) {
    throw Error(`No parser available for language: ${problems.join(', ')}`);
  }
  // null-proto object
  return assign(create(null), fromEntries(languageForExtensionEntries));
};

Now languageForExtension shouldn't even have such properties.

[naugtur]

why put it on a trampoline if it only exists in an async implementation's scope? I doubt it's more efficient that way. Wait, there's 2 of these. they're functionally identical but the other one is typed to optionally return a promise, and that seems wrong (either wrong types or missing yield to remove the promise involvement)

[boneskull]

it's intentional...

[naugtur]

This is where I'd put things: 46f1fe4
(warning - pseudocode in types)

[boneskull]

This is where I'd put things: 46f1fe4
(warning - pseudocode in types)

Types are the problem here, though.

The problem is typing getParserGenerator properly; we need a getParserGenerator that returns a sync parser if we expect a sync parser, and an async parser or sync parser if we don't. And that depends on whether or not we hand it off to the sync or async trampoline. One way to ensure this is to provide a narrowly-typed getParserGenerator based on whether we expect it to return a sync parser or not necessarily—and that's most easily done with a factory function; now we have a factory for sync and a factory for async.

These two factories could be combined into a single polymorphic factory function w/ overloads and inner conditionals, but that looks like adding even more complexity to me.

So that type-fantasy matches reality, I threw in a guard or two to check we're actually working with sync parsers when we expect to be.

[boneskull]

(The alternative is just fudging the types and crossing fingers)

[naugtur]

Trampolines are in use here to make it possible for code to be reused between sync and async. Having two copies of the same exact function just to hang different types off of defeats the purpose.

The types for the generator are only consumed by trampolines and don't matter for correctness of what we do here.

If there is an unavoidable divergence in the two implementations, removed have to be removed

[boneskull]

@naugtur

Trampolines are in use here to make it possible for code to be reused between sync and async. Having two copies of the same exact function just to hang different types off of defeats the purpose.

I see what you're saying here. I'll take another look at it.

[boneskull]

@naugtur OK, I've reverted most of my changes to map-parser.js and added a type guard. The types are still not where I'd like them to be, but there's little we could do other than add a type assertion:

/**
 * @typedef GetSyncParserGeneratorFn
 * @param {ParserGeneratorConfig} config
 * @param {Uint8Array} bytes
 * @param {string} specifier
 * @param {string} location
 * @param {string} packageLocation
 * @param {*} options
 * @returns {Generator<ReturnType<SyncModuleTransform>, ParseResult, any>}
 */

const result = syncTrampoline(
  /** @type {GetSyncParserGeneratorFn} */(getParserGenerator),
  config,
  bytes,
  specifier,
  location,
  packageLocation,
  options,
);

I don't view the above as particularly valuable, so I didn't do it.

[naugtur]

looks good, I need one more pass tomorrow, but no comments yet.