feat(endo): Infer module type from package.json "module" (#402)

If a package is not of "type": "module", but does provide a "module": e.g., "./main.js, the ".js" extension implies CommonJS but the property implies that "./main.js" is a ECMAScript module.  This change allows the module field to override the package type.

packages/endo/README.md

@@ -267,15 +267,6 @@ Node.js platform.
 > TODO A future version may introduce language plugins, so a package may state
 > that files with a particular extension are either parsed or linked with
 > another module.
-> TODO
->
-> Endo does not yet respect the `module` field as it currently
-> only recognizes ECMAScript modules.
-> For backard compatibility, be sure to indicate that any package is of `type`
-> `module` if it uses the `.js` extension for ECMAScript modules.
->
-> A future version of Endo will support CommonJS modules and enforce this
-> behavior.
 
 > TODO
 >

packages/endo/src/archive.js

@@ -45,7 +45,7 @@ const makeRecordingImportHookMaker = (read, baseLocation, manifest, errors) => {
           manifest[packageLocation] = packageManifest;
           packageManifest[moduleSpecifier] = moduleBytes;
 
-          return parse(moduleSource, moduleLocation);
+          return parse(moduleSource, moduleSpecifier, moduleLocation);
         }
       }
       return new StaticModuleRecord("// Module not found", moduleSpecifier);
@@ -69,7 +69,7 @@ const renameCompartments = compartments => {
 const renameCompartmentMap = (compartments, renames) => {
   const result = {};
   for (const [name, compartment] of entries(compartments)) {
-    const { label, parsers } = compartment;
+    const { label, parsers, types } = compartment;
     const modules = {};
     for (const [name, module] of entries(compartment.modules || {})) {
       const compartment = module.compartment
@@ -84,7 +84,8 @@ const renameCompartmentMap = (compartments, renames) => {
       label,
       location: renames[name],
       modules,
-      parsers
+      parsers,
+      types
     };
   }
   return result;

packages/endo/src/assemble.js

@@ -59,7 +59,7 @@ export const assemble = ({
     modules[inner] = compartment.module(moduleSpecifier);
   }
 
-  const parse = mapParsers(descriptor.parsers);
+  const parse = mapParsers(descriptor.parsers, descriptor.types);
 
   const compartment = new Compartment(endowments, modules, {
     resolveHook: resolve,

packages/endo/src/compartmap.js

@@ -83,9 +83,15 @@ const inferParsers = (type, location) => {
   if (type === "module") {
     return moduleParsers;
   }
-  throw new Error(
-    `Cannot infer parser map for package of type ${type} at ${location}`
-  );
+  if (type === "commonjs") {
+    return commonParsers;
+  }
+  if (type !== undefined) {
+    throw new Error(
+      `Cannot infer parser map for package of type ${type} at ${location}`
+    );
+  }
+  return commonParsers;
 };
 
 // graphPackage and gatherDependency are mutually recursive functions that
@@ -136,7 +142,8 @@ const graphPackage = async (
   const { version = "" } = packageDescriptor;
   result.label = `${name}@${version}`;
   result.dependencies = dependencies;
-  result.exports = inferExports(packageDescriptor, tags, packageLocation);
+  result.types = {};
+  result.exports = inferExports(packageDescriptor, tags, result.types);
   result.parsers = inferParsers(packageDescriptor.type, packageLocation);
 
   return Promise.all(children);
@@ -218,9 +225,10 @@ const translateGraph = (mainPackagePath, graph) => {
   // The full map includes every exported module from every dependencey
   // package and is a complete list of every external module that the
   // corresponding compartment can import.
-  for (const [packageLocation, { label, parsers, dependencies }] of entries(
-    graph
-  )) {
+  for (const [
+    packageLocation,
+    { label, parsers, dependencies, types }
+  ] of entries(graph)) {
     const modules = {};
     for (const packageLocation of dependencies) {
       const { exports } = graph[packageLocation];
@@ -235,7 +243,8 @@ const translateGraph = (mainPackagePath, graph) => {
       label,
       location: packageLocation,
       modules,
-      parsers
+      parsers,
+      types
     };
   }
 

packages/endo/src/import-archive.js

@@ -15,7 +15,7 @@ const makeArchiveImportHookMaker = archive => {
       const moduleLocation = join(packageLocation, moduleSpecifier);
       const moduleBytes = await archive.read(moduleLocation);
       const moduleSource = decoder.decode(moduleBytes);
-      return parse(moduleSource, `file:///${moduleLocation}`);
+      return parse(moduleSource, moduleSpecifier, `file:///${moduleLocation}`);
     };
     return importHook;
   };

packages/endo/src/import.js

@@ -18,7 +18,12 @@ const makeImportHookMaker = (read, baseLocation) => {
       const moduleLocation = resolveLocation(moduleSpecifier, packageLocation);
       const moduleBytes = await read(moduleLocation);
       const moduleSource = decoder.decode(moduleBytes);
-      return parse(moduleSource, moduleLocation);
+      return parse(
+        moduleSource,
+        moduleSpecifier,
+        moduleLocation,
+        packageLocation
+      );
     };
     return importHook;
   };

packages/endo/src/infer-exports.js

@@ -46,15 +46,22 @@ function* interpretExports(name, exports, tags) {
 // ascending priority order, and the caller should use the last one that exists.
 export function* inferExportsEntries(
   { name, main, module, browser, exports },
-  tags
+  tags,
+  types
 ) {
   // From lowest to highest precedence, such that later entries override former
   // entries.
   if (main !== undefined) {
     yield [name, relativize(main)];
   }
   if (module !== undefined && tags.has("import")) {
-    yield [name, relativize(module)];
+    // In this one case, the key "module" has carried a hint that the
+    // referenced module is an ECMASCript module, and that hint is necessary to
+    // override whatever type might be inferred from the module specifier
+    // extension.
+    const spec = relativize(module);
+    types[spec] = "mjs";
+    yield [name, spec];
   }
   if (browser !== undefined && tags.has("browser")) {
     yield* interpretBrowserExports(name, browser);
@@ -81,5 +88,5 @@ export function* inferExportsEntries(
 // every package.
 // That manifest will also prove useful for resolving aliases, like the
 // implicit index.js modules within a package.
-export const inferExports = (descriptor, tags, location) =>
-  fromEntries(inferExportsEntries(descriptor, tags, location));
+export const inferExports = (descriptor, tags, types) =>
+  fromEntries(inferExportsEntries(descriptor, tags, types));

packages/endo/src/parse-requires.js

@@ -23,7 +23,7 @@ import traverse from "@babel/traverse";
  * @param {string} location
  * @return {Array<ImportSpecifier>}
  */
-export const parseRequires = (source, location) => {
+export const parseRequires = (source, location, packageLocation) => {
   try {
     const ast = parser.parse(source);
     const required = new Set();
@@ -54,6 +54,11 @@ export const parseRequires = (source, location) => {
     });
     return Array.from(required).sort();
   } catch (error) {
+    if (/import/.exec(error.message) !== null) {
+      throw new Error(
+        `Cannot parse CommonJS module at ${location}, consider adding "type": "module" to package.json in ${packageLocation}: ${error}`
+      );
+    }
     throw new Error(`Cannot parse CommonJS module at ${location}: ${error}`);
   }
 };

packages/endo/src/parse.js

@@ -11,11 +11,11 @@ const q = JSON.stringify;
 // TODO: parsers should accept bytes and perhaps even content-type for
 // verification.
 
-export const parseMjs = (source, location) => {
+export const parseMjs = (source, _specifier, location, _packageLocation) => {
   return new StaticModuleRecord(source, location);
 };
 
-export const parseCjs = (source, location) => {
+export const parseCjs = (source, _specifier, location, packageLocation) => {
   if (typeof source !== "string") {
     throw new TypeError(
       `Cannot create CommonJS static module record, module source must be a string, got ${source}`
@@ -27,7 +27,7 @@ export const parseCjs = (source, location) => {
     );
   }
 
-  const imports = parseRequires(source, location);
+  const imports = parseRequires(source, location, packageLocation);
   const execute = (exports, compartment, resolvedImports) => {
     const functor = compartment.evaluate(
       `(function (require, exports, module, __filename, __dirname) { ${source} //*/\n})\n//# sourceURL=${location}`
@@ -64,7 +64,7 @@ export const parseCjs = (source, location) => {
   return freeze({ imports, execute });
 };
 
-export const parseJson = (source, location) => {
+export const parseJson = (source, _specifier, location, _packageLocation) => {
   const imports = freeze([]);
   const execute = exports => {
     try {
@@ -78,16 +78,21 @@ export const parseJson = (source, location) => {
   return freeze({ imports, execute });
 };
 
-export const makeExtensionParser = extensions => {
-  return (source, location) => {
-    const extension = parseExtension(location);
+export const makeExtensionParser = (extensions, types) => {
+  return (source, specifier, location, packageLocation) => {
+    let extension;
+    if (Object(types) === types && hasOwnProperty.call(types, specifier)) {
+      extension = types[specifier];
+    } else {
+      extension = parseExtension(location);
+    }
     if (!hasOwnProperty.call(extensions, extension)) {
       throw new Error(
-        `Cannot parse module at ${location}, no parser configured for that extension`
+        `Cannot parse module ${specifier} at ${location}, no parser configured for that extension`
       );
     }
     const parse = extensions[extension];
-    return parse(source, location);
+    return parse(source, specifier, location, packageLocation);
   };
 };
 
@@ -97,7 +102,7 @@ const parserForLanguage = {
   json: parseJson
 };
 
-export const mapParsers = parsers => {
+export const mapParsers = (parsers, types) => {
   const parserForExtension = [];
   const errors = [];
   for (const [extension, language] of entries(parsers)) {
@@ -111,5 +116,5 @@ export const mapParsers = parsers => {
   if (errors.length > 0) {
     throw new Error(`No parser available for language: ${errors.join(", ")}`);
   }
-  return makeExtensionParser(fromEntries(parserForExtension));
+  return makeExtensionParser(fromEntries(parserForExtension), types);
 };

packages/endo/test/main.test.js

@@ -26,7 +26,15 @@ const endowments = {
 };
 
 const assertFixture = (t, namespace) => {
-  const { avery, brooke, builtin, endowed, typecommon, typemodule } = namespace;
+  const {
+    avery,
+    brooke,
+    builtin,
+    endowed,
+    typecommon,
+    typemodule,
+    typehybrid
+  } = namespace;
   t.equal(avery, "Avery", "exports avery");
   t.equal(brooke, "Brooke", "exports brooke");
   t.equal(builtin, "builtin", "exports builtin");
@@ -41,9 +49,10 @@ const assertFixture = (t, namespace) => {
     [42, 42, 42, 42],
     "type=module package carries exports"
   );
+  t.equal(typehybrid, 42, "type=module and module= package carries exports");
 };
 
-const fixtureAssertionCount = 6;
+const fixtureAssertionCount = 7;
 
 // The "create builtin" test prepares a builtin module namespace object that
 // gets threaded into all subsequent tests to satisfy the "builtin" module