Node resolver: Refactor, add tests, and improve edge cases

.changeset/clean-gorillas-carry.md

@@ -0,0 +1,5 @@
+---
+'pleasantest': patch
+---
+
+Improve node_modules and relative paths resolution

jest.config.js

@@ -4,7 +4,10 @@ module.exports = {
     '^pleasantest$': '<rootDir>/dist/cjs/index.cjs',
   },
   testRunner: 'jest-circus/runner',
-  watchPathIgnorePatterns: ['<rootDir>/src/', '<rootDir>/.cache'],
+  watchPathIgnorePatterns: [
+    '<rootDir>/.cache',
+    '<rootDir>/src/.*(?<!\\.test)\\.ts',
+  ],
   transform: {
     '^.+\\.[jt]sx?$': ['esbuild-jest', { sourcemap: true }],
   },

package.json

@@ -22,7 +22,7 @@
     "@testing-library/dom": "8.1.0",
     "@testing-library/jest-dom": "5.14.1",
     "@types/jest": "26.0.24",
-    "@types/node": "16.0.0",
+    "@types/node": "^12.20.16",
     "@types/polka": "0.5.3",
     "@types/puppeteer": "5.4.4",
     "ansi-regex": "6.0.0",

src/module-server/bundle-npm-module.ts

@@ -0,0 +1,118 @@
+import type { Plugin, RollupCache } from 'rollup';
+import { rollup } from 'rollup';
+import { promises as fs } from 'fs';
+import commonjs from '@rollup/plugin-commonjs';
+import { processGlobalPlugin } from './plugins/process-global-plugin';
+import * as esbuild from 'esbuild';
+import { parse } from 'cjs-module-lexer';
+// @ts-expect-error @types/node@12 doesn't like this import
+import { createRequire } from 'module';
+import { isBareImport, npmPrefix } from './extensions-and-detection';
+let npmCache: RollupCache | undefined;
+
+/**
+ * Any package names in this set will need to have their named exports detected manually via require()
+ * because the export names cannot be statically analyzed
+ */
+const dynamicCJSModules = new Set(['prop-types', 'react-dom', 'react']);
+
+/**
+ * Bundle am npm module entry path into a single file
+ * @param mod The full path of the module to bundle, including subpackage/path
+ * @param id The imported identifier
+ * @param optimize Whether the bundle should be a minified/optimized bundle, or the default quick non-optimized bundle
+ */
+export const bundleNpmModule = async (
+  mod: string,
+  id: string,
+  optimize: boolean,
+) => {
+  let namedExports: string[] = [];
+  if (dynamicCJSModules.has(id)) {
+    let isValidCJS = true;
+    try {
+      const text = await fs.readFile(mod, 'utf8');
+      // Goal: Determine if it is ESM or CJS.
+      // Try to parse it with cjs-module-lexer, if it fails, assume it is ESM
+      // eslint-disable-next-line @cloudfour/typescript-eslint/await-thenable
+      await parse(text);
+    } catch {
+      isValidCJS = false;
+    }
+
+    if (isValidCJS) {
+      const require = createRequire(import.meta.url);
+      // eslint-disable-next-line @cloudfour/typescript-eslint/no-var-requires
+      const imported = require(mod);
+      if (typeof imported === 'object' && !imported.__esModule)
+        namedExports = Object.keys(imported);
+    }
+  }
+
+  const virtualEntry = '\0virtualEntry';
+  const hasSyntheticNamedExports = namedExports.length > 0;
+  const bundle = await rollup({
+    input: hasSyntheticNamedExports ? virtualEntry : mod,
+    cache: npmCache,
+    shimMissingExports: true,
+    treeshake: true,
+    preserveEntrySignatures: 'allow-extension',
+    plugins: [
+      hasSyntheticNamedExports &&
+        ({
+          // This plugin handles special-case packages whose named exports cannot be found via static analysis
+          // For these packages, the package is require()'d, and the named exports are determined that way.
+          // A virtual entry exports the named exports from the real entry package
+          name: 'cjs-named-exports',
+          resolveId(id) {
+            if (id === virtualEntry) return virtualEntry;
+          },
+          load(id) {
+            if (id === virtualEntry) {
+              const code = `export * from '${mod}'
+export {${namedExports.join(', ')}} from '${mod}'
+export { default } from '${mod}'`;
+              return code;
+            }
+          },
+        } as Plugin),
+      pluginNodeResolve(),
+      processGlobalPlugin({ NODE_ENV: 'development' }),
+      commonjs({
+        extensions: ['.js', '.cjs', ''],
+        sourceMap: false,
+        transformMixedEsModules: true,
+      }),
+      (optimize && {
+        name: 'esbuild-minify',
+        renderChunk: async (code) => {
+          const output = await esbuild.transform(code, {
+            minify: true,
+            legalComments: 'none',
+          });
+          return { code: output.code };
+        },
+      }) as Plugin,
+    ].filter(Boolean),
+  });
+  npmCache = bundle.cache;
+  const { output } = await bundle.generate({
+    format: 'es',
+    indent: false,
+    exports: 'named',
+    preferConst: true,
+  });
+
+  return output[0].code;
+};
+
+const pluginNodeResolve = (): Plugin => {
+  return {
+    name: 'node-resolve',
+    resolveId(id) {
+      if (isBareImport(id)) return { id: npmPrefix + id, external: true };
+      // If requests already have the npm prefix, mark them as external
+      if (id.startsWith(npmPrefix)) return { id, external: true };
+    },
+  };
+};

src/module-server/extensions-and-detection.ts

@@ -0,0 +1,18 @@
+export const npmPrefix = '@npm/';
+
+export const isRelativeOrAbsoluteImport = (id: string) =>
+  id === '.' ||
+  id === '..' ||
+  id.startsWith('./') ||
+  id.startsWith('../') ||
+  id.startsWith('/');
+
+export const isBareImport = (id: string) =>
+  !(
+    isRelativeOrAbsoluteImport(id) ||
+    id.startsWith('\0') ||
+    id.startsWith(npmPrefix)
+  );
+
+export const cssExts = /\.(?:css|styl|stylus|s[ac]ss|less)$/;
+export const jsExts = /\.(?:[jt]sx?|[cm]js)$/;

src/module-server/index.ts

@@ -27,10 +27,7 @@ export const createModuleServer = async ({
   const plugins = [
     ...userPlugins,
     aliases && aliasPlugin({ entries: aliases }),
-    resolveExtensionsPlugin({
-      extensions: ['.ts', '.tsx', '.js', '.cjs'],
-      index: true,
-    }),
+    resolveExtensionsPlugin(),
     processGlobalPlugin({ NODE_ENV: 'development' }),
     npmPlugin({ root }),
     esbuildPlugin(),

src/module-server/middleware/css.ts

@@ -1,8 +1,9 @@
 import { posix, relative, resolve, sep } from 'path';
 import type polka from 'polka';
 import { promises as fs } from 'fs';
-import { cssExts, cssPlugin } from '../plugins/css';
+import { cssPlugin } from '../plugins/css';
 import type { PluginContext, TransformPluginContext } from 'rollup';
+import { cssExts } from '../extensions-and-detection';
 
 interface CSSMiddlewareOpts {
   root: string;

src/module-server/middleware/js.ts

@@ -9,16 +9,14 @@ import type {
   RawSourceMap,
 } from '@ampproject/remapping/dist/types/types';
 import MagicString from 'magic-string';
+import { jsExts } from '../extensions-and-detection';
 
 interface JSMiddlewareOpts {
   root: string;
   plugins: Plugin[];
   requestCache: Map<string, SourceDescription>;
 }
 
-// TODO: make this configurable
-export const jsExts = /\.(?:[jt]sx?|[cm]js)$/;
-
 // Minimal version of https://github.com/preactjs/wmr/blob/main/packages/wmr/src/wmr-middleware.js
 
 export const jsMiddleware = ({