Skip to content

Commit cbd41ac

Browse files
dygabodygabo
committed
loaders: fix package resolution for edge case
this commit solves a regression introduced with PR-40980. if a resolve call results in a script with .mjs extension the is automatically set to . This avoids the case where an additional in the same directory as the .mjs file would declare the to commonjs
1 parent a182a21 commit cbd41ac

File tree

2 files changed

+169
-78
lines changed

2 files changed

+169
-78
lines changed

‎lib/internal/modules/esm/resolve.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,12 @@ function resolvePackageTargetString(
467467
const composeResult = (resolved) => {
468468
let format;
469469
try {
470-
format = getPackageType(resolved);
470+
// extension has higher priority than package.json type descriptor
471+
if (StringPrototypeEndsWith(resolved.href, '.mjs')) {
472+
format = 'module';
473+
} else {
474+
format = getPackageType(resolved);
475+
}
471476
} catch (err) {
472477
if (err.code === 'ERR_INVALID_FILE_URL_PATH') {
473478
const invalidModuleErr = new ERR_INVALID_MODULE_SPECIFIER(

‎test/es-module/test-esm-resolve-type.js

Lines changed: 163 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -98,86 +98,172 @@ try {
9898
}
9999
};
100100

101-
// Create a dummy dual package
102-
//
103-
/**
104-
* this creates following directory structure:
105-
*
106-
* ./node_modules:
107-
* |-> my-dual-package
108-
* |-> es
109-
* |-> index.js
110-
* |-> package.json [2]
111-
* |-> lib
112-
* |-> index.js
113-
* |->package.json [1]
114-
*
115-
* [1] - main package.json of the package
116-
* - it contains:
117-
* - type: 'commonjs'
118-
* - main: 'lib/mainfile.js'
119-
* - conditional exports for 'require' (lib/index.js) and
120-
* 'import' (es/index.js)
121-
* [2] - package.json add-on for the import case
122-
* - it only contains:
123-
* - type: 'module'
124-
*
125-
* in case the package is consumed as an ESM by importing it:
126-
* import * as my-package from 'my-dual-package'
127-
* it will cause the resolve method to return:
128-
* {
129-
* url: '<base_path>/node_modules/my-dual-package/es/index.js',
130-
* format: 'module'
131-
* }
132-
*
133-
* following testcase ensures that resolve works correctly in this case
134-
* returning the information as specified above. Source for 'url' value
135-
* is [1], source for 'format' value is [2]
136-
*/
101+
function testDualPackageWithJsMainScriptAndModuleType() {
102+
// Create a dummy dual package
103+
//
104+
/**
105+
* this creates following directory structure:
106+
*
107+
* ./node_modules:
108+
* |-> my-dual-package
109+
* |-> es
110+
* |-> index.js
111+
* |-> package.json [2]
112+
* |-> lib
113+
* |-> index.js
114+
* |->package.json [1]
115+
*
116+
* [1] - main package.json of the package
117+
* - it contains:
118+
* - type: 'commonjs'
119+
* - main: 'lib/mainfile.js'
120+
* - conditional exports for 'require' (lib/index.js) and
121+
* 'import' (es/index.js)
122+
* [2] - package.json add-on for the import case
123+
* - it only contains:
124+
* - type: 'module'
125+
*
126+
* in case the package is consumed as an ESM by importing it:
127+
* import * as my-package from 'my-dual-package'
128+
* it will cause the resolve method to return:
129+
* {
130+
* url: '<base_path>/node_modules/my-dual-package/es/index.js',
131+
* format: 'module'
132+
* }
133+
*
134+
* following testcase ensures that resolve works correctly in this case
135+
* returning the information as specified above. Source for 'url' value
136+
* is [1], source for 'format' value is [2]
137+
*/
137138

138-
const moduleName = 'my-dual-package';
139-
140-
const mDir = rel(`node_modules/${moduleName}`);
141-
const esSubDir = rel(`node_modules/${moduleName}/es`);
142-
const cjsSubDir = rel(`node_modules/${moduleName}/lib`);
143-
const pkg = rel(`node_modules/${moduleName}/package.json`);
144-
const esmPkg = rel(`node_modules/${moduleName}/es/package.json`);
145-
const esScript = rel(`node_modules/${moduleName}/es/index.js`);
146-
const cjsScript = rel(`node_modules/${moduleName}/lib/index.js`);
147-
148-
createDir(nmDir);
149-
createDir(mDir);
150-
createDir(esSubDir);
151-
createDir(cjsSubDir);
152-
153-
const mainPkgJsonContent = {
154-
type: 'commonjs',
155-
main: 'lib/index.js',
156-
exports: {
157-
'.': {
158-
'require': './lib/index.js',
159-
'import': './es/index.js'
160-
},
161-
'./package.json': './package.json',
162-
}
163-
};
164-
const esmPkgJsonContent = {
165-
type: 'module'
166-
};
139+
const moduleName = 'my-dual-package';
167140

168-
fs.writeFileSync(pkg, JSON.stringify(mainPkgJsonContent));
169-
fs.writeFileSync(esmPkg, JSON.stringify(esmPkgJsonContent));
170-
fs.writeFileSync(esScript,
171-
'export function esm-resolve-tester() {return 42}');
172-
fs.writeFileSync(cjsScript,
173-
`module.exports = {
174-
esm-resolve-tester: () => {return 42}}`
175-
);
141+
const mDir = rel(`node_modules/${moduleName}`);
142+
const esSubDir = rel(`node_modules/${moduleName}/es`);
143+
const cjsSubDir = rel(`node_modules/${moduleName}/lib`);
144+
const pkg = rel(`node_modules/${moduleName}/package.json`);
145+
const esmPkg = rel(`node_modules/${moduleName}/es/package.json`);
146+
const esScript = rel(`node_modules/${moduleName}/es/index.js`);
147+
const cjsScript = rel(`node_modules/${moduleName}/lib/index.js`);
148+
149+
createDir(nmDir);
150+
createDir(mDir);
151+
createDir(esSubDir);
152+
createDir(cjsSubDir);
153+
154+
const mainPkgJsonContent = {
155+
type: 'commonjs',
156+
main: 'lib/index.js',
157+
exports: {
158+
'.': {
159+
'require': './lib/index.js',
160+
'import': './es/index.js'
161+
},
162+
'./package.json': './package.json',
163+
}
164+
};
165+
const esmPkgJsonContent = {
166+
type: 'module'
167+
};
168+
169+
fs.writeFileSync(pkg, JSON.stringify(mainPkgJsonContent));
170+
fs.writeFileSync(esmPkg, JSON.stringify(esmPkgJsonContent));
171+
fs.writeFileSync(esScript,
172+
'export function esm-resolve-tester() {return 42}');
173+
fs.writeFileSync(cjsScript,
174+
`module.exports = {
175+
esm-resolve-tester: () => {return 42}}`
176+
);
177+
178+
// test the resolve
179+
const resolveResult = resolve(`${moduleName}`);
180+
assert.strictEqual(resolveResult.format, 'module');
181+
assert.ok(resolveResult.url.includes('my-dual-package/es/index.js'));
182+
}
183+
184+
function testDualPackageWithMjsMainScriptAndCJSType() {
185+
186+
// Additional test for following scenario
187+
/**
188+
* this creates following directory structure:
189+
*
190+
* ./node_modules:
191+
* |-> dual-mjs-pjson
192+
* |-> subdir
193+
* |-> index.mjs [3]
194+
* |-> package.json [2]
195+
* |-> index.js
196+
* |->package.json [1]
197+
*
198+
* [1] - main package.json of the package
199+
* - it contains:
200+
* - type: 'commonjs'
201+
* - main: 'subdir/index.js'
202+
* - conditional exports for 'require' (subdir/index.js) and
203+
* 'import' (subdir/index.mjs)
204+
* [2] - package.json add-on for the import case
205+
* - it only contains:
206+
* - type: 'commonjs'
207+
* [3] - main script for the `import` case
208+
*
209+
* in case the package is consumed as an ESM by importing it:
210+
* import * as my-package from 'dual-mjs-pjson'
211+
* it will cause the resolve method to return:
212+
* {
213+
* url: '<base_path>/node_modules/dual-mjs-pjson/subdir/index.mjs',
214+
* format: 'module'
215+
* }
216+
*
217+
* following testcase ensures that resolve works correctly in this case
218+
* returning the information as specified above. Source for 'url' value
219+
* is [1], source for 'format' value is the file extension of [3]
220+
*/
221+
const moduleName = 'dual-mjs-pjson';
222+
223+
const mDir = rel(`node_modules/${moduleName}`);
224+
const subDir = rel(`node_modules/${moduleName}/subdir`);
225+
const pkg = rel(`node_modules/${moduleName}/package.json`);
226+
const subdirPkg = rel(`node_modules/${moduleName}/subdir/package.json`);
227+
const esScript = rel(`node_modules/${moduleName}/subdir/index.mjs`);
228+
const cjsScript = rel(`node_modules/${moduleName}/subdir/index.js`);
229+
230+
createDir(nmDir);
231+
createDir(mDir);
232+
createDir(subDir);
233+
234+
const mainPkgJsonContent = {
235+
type: 'commonjs',
236+
main: 'lib/index.js',
237+
exports: {
238+
'.': {
239+
'require': './subdir/index.js',
240+
'import': './subdir/index.mjs'
241+
},
242+
'./package.json': './package.json',
243+
}
244+
};
245+
const subdirPkgJsonContent = {
246+
type: 'commonjs'
247+
};
248+
249+
fs.writeFileSync(pkg, JSON.stringify(mainPkgJsonContent));
250+
fs.writeFileSync(subdirPkg, JSON.stringify(subdirPkgJsonContent));
251+
fs.writeFileSync(esScript,
252+
'export function esm-resolve-tester() {return 42}');
253+
fs.writeFileSync(cjsScript,
254+
`module.exports = {
255+
esm-resolve-tester: () => {return 42}}`
256+
);
257+
258+
// test the resolve
259+
const resolveResult = resolve(`${moduleName}`);
260+
assert.strictEqual(resolveResult.format, 'module');
261+
assert.ok(resolveResult.url.includes(`${moduleName}/subdir/index.mjs`));
262+
}
263+
264+
testDualPackageWithJsMainScriptAndModuleType();
265+
testDualPackageWithMjsMainScriptAndCJSType();
176266

177-
// test the resolve
178-
const resolveResult = resolve(`${moduleName}`);
179-
assert.strictEqual(resolveResult.format, 'module');
180-
assert.ok(resolveResult.url.includes('my-dual-package/es/index.js'));
181267
} finally {
182268
process.chdir(previousCwd);
183269
fs.rmSync(nmDir, { recursive: true, force: true });

0 commit comments

Comments
 (0)