When inferring the module system (esm vs cjs) the nearest pacakge.jso… (

#771)

* When inferring the module system (esm vs cjs) the nearest pacakge.json should be consulted from the point of the file. The current implemetation tracks the module system by rootContext but there may be package.json files deeper than the rootContext which alter the module system which should be used. If you have a folder inside an ESM project with a package.json containing `{ type: 'commonjs' }` then all .js files in that folder and descendents should be treated like cjs. This new implementation searches for package.json files from the point of the module being imported. to avoid doing any unecessary work the result is cached along the parent path of the module context (it's folder) so that you only need to look up as far as the nearest common folder that has already had a package.json resolved.

Additionally the change removes a recently added bailout where the module system was assumed to be cjs if the LoaderContext's `this.environment` property was null. I believe this misunderstands the environment property and suggests that if it is not set then commonjs is an appropriate target. However it is possible to have a webpack config that does not produce an environment for the LoaderContext but is still an ESM project from src. Since the Loader runs before other transforms that might convert from ESM to CJS the module detection is still needed.

* use cjs form for webpack4 because it does not support `import.meta`. Instead of feature testing with `this.environment` which was added to webpack 5 relatively recently use `typeof this.addMissingDepdencey === 'function'` which is in most (or all) webpack 5 releases but not included in webpack 4.

---------

Co-authored-by: Michael Mok <pmmmwh@gmail.com>

.eslintrc.json

@@ -55,6 +55,24 @@
         "commonjs": false,
         "es6": true
       }
+    },
+    {
+      "files": ["test/**/fixtures/cjs/esm/*.js"],
+      "parserOptions": {
+        "ecmaVersion": 2015,
+        "sourceType": "module"
+      },
+      "env": {
+        "commonjs": false,
+        "es6": true
+      }
+    },
+    {
+      "files": ["test/**/fixtures/esm/cjs/*.js"],
+      "env": {
+        "commonjs": true,
+        "es6": true
+      }
     }
   ]
 }

loader/index.js

@@ -62,11 +62,7 @@ function ReactRefreshLoader(source, inputSourceMap, meta) {
    */
   async function _loader(source, inputSourceMap) {
     /** @type {'esm' | 'cjs'} */
-    let moduleSystem = 'cjs';
-    // Only try to resolve the module system if the environment is known to support ES syntax
-    if (this.environment != null) {
-      moduleSystem = await getModuleSystem.call(this, ModuleFilenameHelpers, options);
-    }
+    const moduleSystem = await getModuleSystem.call(this, ModuleFilenameHelpers, options);
 
     const RefreshSetupRuntime = RefreshSetupRuntimes[moduleSystem];
     const RefreshModuleRuntime = getRefreshModuleRuntime(Template, {

loader/utils/getModuleSystem.js

@@ -1,7 +1,5 @@
 const { promises: fsPromises } = require('fs');
 const path = require('path');
-const commonPathPrefix = require('common-path-prefix');
-const findUp = require('find-up');
 
 /** @type {Map<string, string | undefined>} */
 let packageJsonTypeMap = new Map();
@@ -43,29 +41,81 @@ async function getModuleSystem(ModuleFilenameHelpers, options) {
   if (/\.mjs$/.test(this.resourcePath)) return 'esm';
   if (/\.cjs$/.test(this.resourcePath)) return 'cjs';
 
-  // Load users' `package.json` -
-  // We will cache the results in a global variable so it will only be parsed once.
-  let packageJsonType = packageJsonTypeMap.get(this.rootContext);
-  if (!packageJsonType) {
+  if (typeof this.addMissingDependency !== 'function') {
+    // This is Webpack 4 which does not support `import.meta` and cannot use ESM anwyay. We assume .js files
+    // are commonjs because the output cannot be ESM anyway
+    return 'cjs';
+  }
+
+  // We will assume commonjs if we cannot determine otherwise
+  let packageJsonType = '';
+
+  // We begin our search for relevant package.json files at the directory of the
+  // resource being loaded.
+  // These paths should already be resolved but we resolve them again to ensure
+  // we are dealing with an aboslute path
+  const resourceContext = path.dirname(this.resourcePath);
+  let searchPath = resourceContext;
+  let previousSearchPath = '';
+  // We start our search just above the root context of the webpack compilation
+  const stopPath = path.dirname(this.rootContext);
+
+  // if the module context is a resolved symlink outside the rootContext path then we will never find
+  // the stopPath so we also halt when we hit the root. Note that there is a potential that the wrong
+  // package.json is found in some pathalogical cases like if a folder that is conceptually a package
+  // but does not have an ancestor package.json but there is a package.json higher up. This might happen if
+  // you have a folder of utility js files that you symlink but did not organize as a package. We consider
+  // this an edge case for now
+  while (searchPath !== stopPath && searchPath !== previousSearchPath) {
+    // If we have already determined the package.json type for this path we can stop searching. We do however
+    // still need to cache the found value from the resourcePath folder up to the matching searchPath to avoid
+    // retracing these steps when processing sibling resources.
+    if (packageJsonTypeMap.has(searchPath)) {
+      packageJsonType = packageJsonTypeMap.get(searchPath);
+
+      let currentPath = resourceContext;
+      while (currentPath !== searchPath) {
+        // We set the found type at least level from this.resourcePath folder up to the matching searchPath
+        packageJsonTypeMap.set(currentPath, packageJsonType);
+        currentPath = path.dirname(currentPath);
+      }
+      break;
+    }
+
+    let packageJsonPath = path.join(searchPath, 'package.json');
     try {
-      const commonPath = commonPathPrefix([this.rootContext, this.resourcePath], '/');
-      const stopPath = path.resolve(commonPath, '..');
-
-      const packageJsonPath = await findUp(
-        (dir) => {
-          if (dir === stopPath) return findUp.stop;
-          return 'package.json';
-        },
-        { cwd: path.dirname(this.resourcePath) }
-      );
-
-      const buffer = await fsPromises.readFile(packageJsonPath, { encoding: 'utf-8' });
-      const rawPackageJson = buffer.toString('utf-8');
-      ({ type: packageJsonType } = JSON.parse(rawPackageJson));
-      packageJsonTypeMap.set(this.rootContext, packageJsonType);
+      const packageSource = await fsPromises.readFile(packageJsonPath, 'utf-8');
+      try {
+        const packageObject = JSON.parse(packageSource);
+
+        // Any package.json is sufficient as long as it can be parsed. If it does not explicitly have a type "module"
+        // it will be assumed to be commonjs.
+        packageJsonType = typeof packageObject.type === 'string' ? packageObject.type : '';
+        packageJsonTypeMap.set(searchPath, packageJsonType);
+
+        // We set the type in the cache for all paths from the resourcePath folder up to the
+        // matching searchPath to avoid retracing these steps when processing sibling resources.
+        let currentPath = resourceContext;
+        while (currentPath !== searchPath) {
+          packageJsonTypeMap.set(currentPath, packageJsonType);
+          currentPath = path.dirname(currentPath);
+        }
+      } catch (e) {
+        // package.json exists but could not be parsed. we track it as a dependency so we can reload if
+        // this file changes
+      }
+      this.addDependency(packageJsonPath);
+
+      break;
     } catch (e) {
-      // Failed to parse `package.json`, do nothing.
+      // package.json does not exist. We track it as a missing dependency so we can reload if this
+      // file is added
+      this.addMissingDependency(packageJsonPath);
     }
+
+    // try again at the next level up
+    previousSearchPath = searchPath;
+    searchPath = path.dirname(searchPath);
   }
 
   // Check `package.json` for the `type` field -

test/helpers/compilation/index.js

@@ -47,6 +47,7 @@ async function getCompilation(subContext, options = {}) {
     module: {
       rules: [
         {
+          exclude: /node_modules/,
           test: /\.js$/,
           use: [
             {

test/loader/fixtures/cjs/esm/index.js

@@ -0,0 +1 @@
+export default 'esm';

test/loader/fixtures/cjs/esm/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "module"
+}

test/loader/fixtures/esm/cjs/index.js

@@ -0,0 +1 @@
+module.exports = 'cjs';

test/loader/fixtures/esm/cjs/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "commonjs"
+}

test/loader/loader.test.js

@@ -384,13 +384,14 @@ describe('loader', () => {
           \\\\******************/
         /***/ ((__webpack_module__, __webpack_exports__, __webpack_require__) => {
 
+        var react_refresh__WEBPACK_IMPORTED_MODULE_0___namespace_cache;
         __webpack_require__.r(__webpack_exports__);
         /* harmony export */ __webpack_require__.d(__webpack_exports__, {
         /* harmony export */   \\"default\\": () => (__WEBPACK_DEFAULT_EXPORT__)
         /* harmony export */ });
         /* harmony import */ var react_refresh__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! react-refresh */ \\"../../../../node_modules/react-refresh/runtime.js\\");
 
-        __webpack_require__.$Refresh$.runtime = react_refresh__WEBPACK_IMPORTED_MODULE_0__;
+        __webpack_require__.$Refresh$.runtime = /*#__PURE__*/ (react_refresh__WEBPACK_IMPORTED_MODULE_0___namespace_cache || (react_refresh__WEBPACK_IMPORTED_MODULE_0___namespace_cache = __webpack_require__.t(react_refresh__WEBPACK_IMPORTED_MODULE_0__, 2)));
 
         /* harmony default export */ const __WEBPACK_DEFAULT_EXPORT__ = ('Test');
 

test/loader/unit/getModuleSystem.test.js

@@ -64,6 +64,23 @@ describe('getModuleSystem', () => {
         {
           resourcePath: path.resolve(__dirname, '..', 'fixtures/esm', 'index.js'),
           rootContext: path.resolve(__dirname, '..', 'fixtures/esm'),
+          addDependency: () => {},
+          addMissingDependency: () => {},
+        },
+        ModuleFilenameHelpers,
+        {}
+      )
+    ).resolves.toBe('esm');
+  });
+
+  it('should return `esm` when `package.json` uses the `module` type nested inside a cjs package', async () => {
+    await expect(
+      getModuleSystem.call(
+        {
+          resourcePath: path.resolve(__dirname, '..', 'fixtures/cjs/esm', 'index.js'),
+          rootContext: path.resolve(__dirname, '..', 'fixtures/cjs'),
+          addDependency: () => {},
+          addMissingDependency: () => {},
         },
         ModuleFilenameHelpers,
         {}
@@ -77,6 +94,23 @@ describe('getModuleSystem', () => {
         {
           resourcePath: path.resolve(__dirname, '..', 'fixtures/cjs', 'index.js'),
           rootContext: path.resolve(__dirname, '..', 'fixtures/cjs'),
+          addDependency: () => {},
+          addMissingDependency: () => {},
+        },
+        ModuleFilenameHelpers,
+        {}
+      )
+    ).resolves.toBe('cjs');
+  });
+
+  it('should return `cjs` when `package.json` uses the `commonjs` type nexted insdie an esm package', async () => {
+    await expect(
+      getModuleSystem.call(
+        {
+          resourcePath: path.resolve(__dirname, '..', 'fixtures/esm/cjs', 'index.js'),
+          rootContext: path.resolve(__dirname, '..', 'fixtures/esm'),
+          addDependency: () => {},
+          addMissingDependency: () => {},
         },
         ModuleFilenameHelpers,
         {}
@@ -90,6 +124,8 @@ describe('getModuleSystem', () => {
         {
           resourcePath: path.resolve(__dirname, '..', 'fixtures/auto', 'index.js'),
           rootContext: path.resolve(__dirname, '..', 'fixtures/auto'),
+          addDependency: () => {},
+          addMissingDependency: () => {},
         },
         ModuleFilenameHelpers,
         { esModule: {} }