Skip to content

Commit

Permalink
fix: let map-parser decide sync/async behavior upfront
Browse files Browse the repository at this point in the history
  • Loading branch information
naugtur authored and boneskull committed Sep 20, 2024
1 parent 9373202 commit 951f843
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 94 deletions.
22 changes: 18 additions & 4 deletions packages/compartment-mapper/src/import-lite.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,33 @@ import {
} from './import-hook.js';
import { isSyncReadPowers } from './powers.js';

const { assign, create, freeze } = Object;
const { assign, create, freeze, entries } = Object;

/**
* Returns `true` if `value` is a {@link SyncImportLocationOptions}.
*
* The only requirements here are:
* The requirements here are:
* - `moduleTransforms` _is not_ present in `value`
* - `parserForLanguage` - if set, contains synchronous parsers only
*
* @param {ImportLocationOptions|SyncImportLocationOptions} value
* @returns {value is SyncImportLocationOptions}
*/
const isSyncOptions = value =>
!value || (typeof value === 'object' && !('moduleTransforms' in value));
const isSyncOptions = value => {
if (!value || (typeof value === 'object' && !('moduleTransforms' in value))) {
if (value.parserForLanguage) {
for (const [_language, { synchronous }] of entries(
value.parserForLanguage,
)) {
if (!synchronous) {
return false;
}
}
}
return true;
}
return false;
};

/**
* @overload
Expand Down
21 changes: 9 additions & 12 deletions packages/compartment-mapper/src/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,26 @@

// @ts-check

/** @import {ImportNowHook, ModuleMapHook} from 'ses' */
/** @import {ModuleMapHook} from 'ses' */
/**
* @import {
* CompartmentDescriptor,
* CompartmentMapDescriptor,
* ImportNowHookMaker,
* ImportNowHookMakerParams,
* LanguageForExtension,
* LinkOptions,
* LinkResult,
* ModuleDescriptor,
* ModuleTransforms,
* ParseFn,
* ParseFnAsync,
* ParserForLanguage,
* ParserImplementation,
* ShouldDeferError,
* SyncModuleTransforms
* } from './types.js'
*/
/** @import {ERef} from '@endo/eventual-send' */

import { mapParsers } from './map-parser.js';
import { makeMapParsers } from './map-parser.js';
import { resolve as resolveFallback } from './node-module-specifier.js';
import {
ATTENUATORS_COMPARTMENT,
Expand Down Expand Up @@ -289,6 +286,12 @@ export const link = (
assign(create(null), parserForLanguageOption),
);

const mapParsers = makeMapParsers({
parserForLanguage,
moduleTransforms,
syncModuleTransforms,
});

for (const [compartmentName, compartmentDescriptor] of entries(
compartmentDescriptors,
)) {
Expand Down Expand Up @@ -325,13 +328,7 @@ export const link = (
// TS is kind of dumb about this, so we can use a type assertion to avoid a
// pointless ternary.
const parse = /** @type {ParseFn|ParseFnAsync} */ (
mapParsers(
languageForExtension,
languageForModuleSpecifier,
parserForLanguage,
moduleTransforms,
syncModuleTransforms,
)
mapParsers(languageForExtension, languageForModuleSpecifier)
);

/** @type {ShouldDeferError} */
Expand Down
134 changes: 56 additions & 78 deletions packages/compartment-mapper/src/map-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,45 +51,7 @@ const extensionImpliesLanguage = extension => extension !== 'js';
* of the module content against the language implied by the extension or file
* name.
*
* @overload
* @param {Record<string, string>} languageForExtension - maps a file extension
* to the corresponding language.
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case,
* the type of a module is implied by package.json and should not be inferred
* from its extension.
* @param {ParserForLanguage} parserForLanguage
* @param {never} [moduleTransforms]
* @param {SyncModuleTransforms} [syncModuleTransforms]
* @returns {ParseFn}
*/

/**
* Produces a `parser` that parses the content of a module according to the
* corresponding module language, given the extension of the module specifier
* and the configuration of the containing compartment. We do not yet support
* import assertions and we do not have a mechanism for validating the MIME type
* of the module content against the language implied by the extension or file
* name.
* @overload
* @param {Record<string, string>} languageForExtension - maps a file extension
* to the corresponding language.
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case,
* the type of a module is implied by package.json and should not be inferred
* from its extension.
* @param {ParserForLanguage} parserForLanguage
* @param {ModuleTransforms|SyncModuleTransforms} [moduleTransforms]
* @param {SyncModuleTransforms} [syncModuleTransforms]
* @returns {ParseFnAsync}
*/

/**
* Produces a `parser` that parses the content of a module according to the
* corresponding module language, given the extension of the module specifier
* and the configuration of the containing compartment. We do not yet support
* import assertions and we do not have a mechanism for validating the MIME type
* of the module content against the language implied by the extension or file
* name.
*
* @param {boolean} preferSynchronous
* @param {Record<string, string>} languageForExtension - maps a file extension
* to the corresponding language.
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case,
Expand All @@ -101,6 +63,7 @@ const extensionImpliesLanguage = extension => extension !== 'js';
* @returns {ParseFnAsync|ParseFn}
*/
const makeExtensionParser = (
preferSynchronous,
languageForExtension,
languageForModuleSpecifier,
parserForLanguage,
Expand Down Expand Up @@ -202,12 +165,7 @@ const makeExtensionParser = (
syncParser.isSyncParser = true;

/**
* @param {Uint8Array} bytes
* @param {string} specifier
* @param {string} location
* @param {string} packageLocation
* @param {*} options
* @returns {Promise<ParseResult>}
* @type {ParseFnAsync}
*/
const asyncParser = async (
bytes,
Expand All @@ -226,51 +184,31 @@ const makeExtensionParser = (
);
};

// if we have nothing in the moduleTransforms object, then we can use the syncParser.
if (keys(moduleTransforms).length === 0) {
// Unfortunately, typescript was not smart enough to figure out the return type depending on a boolean in arguments, so it has to be ParseFnAsync|ParseFn
if (preferSynchronous) {
transforms = syncModuleTransforms;
return syncParser;
}
} else {
// we can fold syncModuleTransforms into moduleTransforms because
// async supports sync, but not vice-versa
transforms = {
...syncModuleTransforms,
...moduleTransforms,
};

// we can fold syncModuleTransforms into moduleTransforms because
// async supports sync, but not vice-versa
transforms = {
...syncModuleTransforms,
...moduleTransforms,
};

return asyncParser;
return asyncParser;
}
};

/**
* @overload
* @param {LanguageForExtension} languageForExtension
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case, the type of a module
* is implied by package.json and should not be inferred from its extension.
* @param {ParserForLanguage} parserForLanguage
* @param {undefined} [moduleTransforms]
* @param {SyncModuleTransforms} [syncModuleTransforms]
* @returns {ParseFn}
*/

/**
* @overload
* @param {LanguageForExtension} languageForExtension
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case, the type of a module
* is implied by package.json and should not be inferred from its extension.
* @param {ParserForLanguage} parserForLanguage
* @param {ModuleTransforms|SyncModuleTransforms} [moduleTransforms]
* @param {SyncModuleTransforms} [syncModuleTransforms]
* @returns {ParseFnAsync}
*/

/**
* @param {LanguageForExtension} languageForExtension
* @param {Record<string, string>} languageForModuleSpecifier - In a rare case, the type of a module
* is implied by package.json and should not be inferred from its extension.
* @param {ParserForLanguage} parserForLanguage
* @param {ModuleTransforms|SyncModuleTransforms} [moduleTransforms]
* @param {ModuleTransforms} [moduleTransforms]
* @param {SyncModuleTransforms} [syncModuleTransforms]
* @param {boolean} [preferSynchronous]
* @returns {ParseFnAsync|ParseFn}
*/
export const mapParsers = (
Expand All @@ -279,6 +217,7 @@ export const mapParsers = (
parserForLanguage,
moduleTransforms = {},
syncModuleTransforms = {},
preferSynchronous = false,
) => {
const languageForExtensionEntries = [];
const problems = [];
Expand All @@ -293,10 +232,49 @@ export const mapParsers = (
throw Error(`No parser available for language: ${problems.join(', ')}`);
}
return makeExtensionParser(
preferSynchronous,
fromEntries(languageForExtensionEntries),
languageForModuleSpecifier,
parserForLanguage,
moduleTransforms,
syncModuleTransforms,
);
};

/**
* Prepares a function to map parsers after verifying whether synchronous behavior is
* preferred. Synchronous behavior is selected if all parsers are synchronous and no
* async transforms are provided.
*
* @param {object} options
* @param {ParserForLanguage} options.parserForLanguage
* @param {ModuleTransforms} [options.moduleTransforms]
* @param {SyncModuleTransforms} [options.syncModuleTransforms]
* @returns {(languageForExtension: LanguageForExtension, languageForModuleSpecifier: Record<string, string>) => ParseFnAsync|ParseFn}
*/
export const makeMapParsers = ({
parserForLanguage,
moduleTransforms,
syncModuleTransforms,
}) => {
let preferSynchronous = true;
if (moduleTransforms && keys(moduleTransforms).length > 0) {
preferSynchronous = false;
} else {
for (const [_language, { synchronous }] of entries(parserForLanguage)) {
if (!synchronous) {
preferSynchronous = false;
break;
}
}
}
return (languageForExtension, languageForModuleSpecifier) =>
mapParsers(
languageForExtension,
languageForModuleSpecifier,
parserForLanguage,
moduleTransforms,
syncModuleTransforms,
preferSynchronous,
);
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-archive-cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,5 @@ export const parseArchiveCjs = (
export default {
parse: parseArchiveCjs,
heuristicImports: true,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-archive-mjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ export const parseArchiveMjs = (
export default {
parse: parseArchiveMjs,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-bytes.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ export const parseBytes = (bytes, _specifier, _location, _packageLocation) => {
export default {
parse: parseBytes,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,5 @@ export const parseCjs = (
export default {
parse: parseCjs,
heuristicImports: true,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ export const parseJson = (bytes, _specifier, location, _packageLocation) => {
export default {
parse: parseJson,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-mjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ export const parseMjs = (
export default {
parse: parseMjs,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-pre-cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,5 @@ export const parsePreCjs = (
export default {
parse: parsePreCjs,
heuristicImports: true,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-pre-mjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ export const parsePreMjs = (
export default {
parse: parsePreMjs,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/parse-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ export const parseText = (bytes, _specifier, _location, _packageLocation) => {
export default {
parse: parseText,
heuristicImports: false,
synchronous: true,
};
1 change: 1 addition & 0 deletions packages/compartment-mapper/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ export {};
*
* @typedef {object} ParserImplementation
* @property {boolean} heuristicImports
* @property {boolean} [synchronous]
* @property {ParseFn} parse
*/

Expand Down

0 comments on commit 951f843

Please sign in to comment.