Skip to content

Commit c2900ce

Browse files
gnoffpmmmwh
andauthored
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 <[email protected]>
1 parent ddbb7aa commit c2900ce

File tree

10 files changed

+138
-28
lines changed

10 files changed

+138
-28
lines changed

.eslintrc.json

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,24 @@
5555
"commonjs": false,
5656
"es6": true
5757
}
58+
},
59+
{
60+
"files": ["test/**/fixtures/cjs/esm/*.js"],
61+
"parserOptions": {
62+
"ecmaVersion": 2015,
63+
"sourceType": "module"
64+
},
65+
"env": {
66+
"commonjs": false,
67+
"es6": true
68+
}
69+
},
70+
{
71+
"files": ["test/**/fixtures/esm/cjs/*.js"],
72+
"env": {
73+
"commonjs": true,
74+
"es6": true
75+
}
5876
}
5977
]
6078
}

loader/index.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,7 @@ function ReactRefreshLoader(source, inputSourceMap, meta) {
6262
*/
6363
async function _loader(source, inputSourceMap) {
6464
/** @type {'esm' | 'cjs'} */
65-
let moduleSystem = 'cjs';
66-
// Only try to resolve the module system if the environment is known to support ES syntax
67-
if (this.environment != null) {
68-
moduleSystem = await getModuleSystem.call(this, ModuleFilenameHelpers, options);
69-
}
65+
const moduleSystem = await getModuleSystem.call(this, ModuleFilenameHelpers, options);
7066

7167
const RefreshSetupRuntime = RefreshSetupRuntimes[moduleSystem];
7268
const RefreshModuleRuntime = getRefreshModuleRuntime(Template, {

loader/utils/getModuleSystem.js

Lines changed: 72 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
const { promises: fsPromises } = require('fs');
22
const path = require('path');
3-
const commonPathPrefix = require('common-path-prefix');
4-
const findUp = require('find-up');
53

64
/** @type {Map<string, string | undefined>} */
75
let packageJsonTypeMap = new Map();
@@ -43,29 +41,81 @@ async function getModuleSystem(ModuleFilenameHelpers, options) {
4341
if (/\.mjs$/.test(this.resourcePath)) return 'esm';
4442
if (/\.cjs$/.test(this.resourcePath)) return 'cjs';
4543

46-
// Load users' `package.json` -
47-
// We will cache the results in a global variable so it will only be parsed once.
48-
let packageJsonType = packageJsonTypeMap.get(this.rootContext);
49-
if (!packageJsonType) {
44+
if (typeof this.addMissingDependency !== 'function') {
45+
// This is Webpack 4 which does not support `import.meta` and cannot use ESM anwyay. We assume .js files
46+
// are commonjs because the output cannot be ESM anyway
47+
return 'cjs';
48+
}
49+
50+
// We will assume commonjs if we cannot determine otherwise
51+
let packageJsonType = '';
52+
53+
// We begin our search for relevant package.json files at the directory of the
54+
// resource being loaded.
55+
// These paths should already be resolved but we resolve them again to ensure
56+
// we are dealing with an aboslute path
57+
const resourceContext = path.dirname(this.resourcePath);
58+
let searchPath = resourceContext;
59+
let previousSearchPath = '';
60+
// We start our search just above the root context of the webpack compilation
61+
const stopPath = path.dirname(this.rootContext);
62+
63+
// if the module context is a resolved symlink outside the rootContext path then we will never find
64+
// the stopPath so we also halt when we hit the root. Note that there is a potential that the wrong
65+
// package.json is found in some pathalogical cases like if a folder that is conceptually a package
66+
// but does not have an ancestor package.json but there is a package.json higher up. This might happen if
67+
// you have a folder of utility js files that you symlink but did not organize as a package. We consider
68+
// this an edge case for now
69+
while (searchPath !== stopPath && searchPath !== previousSearchPath) {
70+
// If we have already determined the package.json type for this path we can stop searching. We do however
71+
// still need to cache the found value from the resourcePath folder up to the matching searchPath to avoid
72+
// retracing these steps when processing sibling resources.
73+
if (packageJsonTypeMap.has(searchPath)) {
74+
packageJsonType = packageJsonTypeMap.get(searchPath);
75+
76+
let currentPath = resourceContext;
77+
while (currentPath !== searchPath) {
78+
// We set the found type at least level from this.resourcePath folder up to the matching searchPath
79+
packageJsonTypeMap.set(currentPath, packageJsonType);
80+
currentPath = path.dirname(currentPath);
81+
}
82+
break;
83+
}
84+
85+
let packageJsonPath = path.join(searchPath, 'package.json');
5086
try {
51-
const commonPath = commonPathPrefix([this.rootContext, this.resourcePath], '/');
52-
const stopPath = path.resolve(commonPath, '..');
53-
54-
const packageJsonPath = await findUp(
55-
(dir) => {
56-
if (dir === stopPath) return findUp.stop;
57-
return 'package.json';
58-
},
59-
{ cwd: path.dirname(this.resourcePath) }
60-
);
61-
62-
const buffer = await fsPromises.readFile(packageJsonPath, { encoding: 'utf-8' });
63-
const rawPackageJson = buffer.toString('utf-8');
64-
({ type: packageJsonType } = JSON.parse(rawPackageJson));
65-
packageJsonTypeMap.set(this.rootContext, packageJsonType);
87+
const packageSource = await fsPromises.readFile(packageJsonPath, 'utf-8');
88+
try {
89+
const packageObject = JSON.parse(packageSource);
90+
91+
// Any package.json is sufficient as long as it can be parsed. If it does not explicitly have a type "module"
92+
// it will be assumed to be commonjs.
93+
packageJsonType = typeof packageObject.type === 'string' ? packageObject.type : '';
94+
packageJsonTypeMap.set(searchPath, packageJsonType);
95+
96+
// We set the type in the cache for all paths from the resourcePath folder up to the
97+
// matching searchPath to avoid retracing these steps when processing sibling resources.
98+
let currentPath = resourceContext;
99+
while (currentPath !== searchPath) {
100+
packageJsonTypeMap.set(currentPath, packageJsonType);
101+
currentPath = path.dirname(currentPath);
102+
}
103+
} catch (e) {
104+
// package.json exists but could not be parsed. we track it as a dependency so we can reload if
105+
// this file changes
106+
}
107+
this.addDependency(packageJsonPath);
108+
109+
break;
66110
} catch (e) {
67-
// Failed to parse `package.json`, do nothing.
111+
// package.json does not exist. We track it as a missing dependency so we can reload if this
112+
// file is added
113+
this.addMissingDependency(packageJsonPath);
68114
}
115+
116+
// try again at the next level up
117+
previousSearchPath = searchPath;
118+
searchPath = path.dirname(searchPath);
69119
}
70120

71121
// Check `package.json` for the `type` field -

test/helpers/compilation/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ async function getCompilation(subContext, options = {}) {
4747
module: {
4848
rules: [
4949
{
50+
exclude: /node_modules/,
5051
test: /\.js$/,
5152
use: [
5253
{
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default 'esm';
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"type": "module"
3+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = 'cjs';
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"type": "commonjs"
3+
}

test/loader/loader.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,13 +384,14 @@ describe('loader', () => {
384384
\\\\******************/
385385
/***/ ((__webpack_module__, __webpack_exports__, __webpack_require__) => {
386386
387+
var react_refresh__WEBPACK_IMPORTED_MODULE_0___namespace_cache;
387388
__webpack_require__.r(__webpack_exports__);
388389
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
389390
/* harmony export */ \\"default\\": () => (__WEBPACK_DEFAULT_EXPORT__)
390391
/* harmony export */ });
391392
/* harmony import */ var react_refresh__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! react-refresh */ \\"../../../../node_modules/react-refresh/runtime.js\\");
392393
393-
__webpack_require__.$Refresh$.runtime = react_refresh__WEBPACK_IMPORTED_MODULE_0__;
394+
__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)));
394395
395396
/* harmony default export */ const __WEBPACK_DEFAULT_EXPORT__ = ('Test');
396397

test/loader/unit/getModuleSystem.test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,23 @@ describe('getModuleSystem', () => {
6464
{
6565
resourcePath: path.resolve(__dirname, '..', 'fixtures/esm', 'index.js'),
6666
rootContext: path.resolve(__dirname, '..', 'fixtures/esm'),
67+
addDependency: () => {},
68+
addMissingDependency: () => {},
69+
},
70+
ModuleFilenameHelpers,
71+
{}
72+
)
73+
).resolves.toBe('esm');
74+
});
75+
76+
it('should return `esm` when `package.json` uses the `module` type nested inside a cjs package', async () => {
77+
await expect(
78+
getModuleSystem.call(
79+
{
80+
resourcePath: path.resolve(__dirname, '..', 'fixtures/cjs/esm', 'index.js'),
81+
rootContext: path.resolve(__dirname, '..', 'fixtures/cjs'),
82+
addDependency: () => {},
83+
addMissingDependency: () => {},
6784
},
6885
ModuleFilenameHelpers,
6986
{}
@@ -77,6 +94,23 @@ describe('getModuleSystem', () => {
7794
{
7895
resourcePath: path.resolve(__dirname, '..', 'fixtures/cjs', 'index.js'),
7996
rootContext: path.resolve(__dirname, '..', 'fixtures/cjs'),
97+
addDependency: () => {},
98+
addMissingDependency: () => {},
99+
},
100+
ModuleFilenameHelpers,
101+
{}
102+
)
103+
).resolves.toBe('cjs');
104+
});
105+
106+
it('should return `cjs` when `package.json` uses the `commonjs` type nexted insdie an esm package', async () => {
107+
await expect(
108+
getModuleSystem.call(
109+
{
110+
resourcePath: path.resolve(__dirname, '..', 'fixtures/esm/cjs', 'index.js'),
111+
rootContext: path.resolve(__dirname, '..', 'fixtures/esm'),
112+
addDependency: () => {},
113+
addMissingDependency: () => {},
80114
},
81115
ModuleFilenameHelpers,
82116
{}
@@ -90,6 +124,8 @@ describe('getModuleSystem', () => {
90124
{
91125
resourcePath: path.resolve(__dirname, '..', 'fixtures/auto', 'index.js'),
92126
rootContext: path.resolve(__dirname, '..', 'fixtures/auto'),
127+
addDependency: () => {},
128+
addMissingDependency: () => {},
93129
},
94130
ModuleFilenameHelpers,
95131
{ esModule: {} }

0 commit comments

Comments
 (0)