module: clarify cjs global-like error on ModuleJobSync · nodejs/node@72fac71 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 633
• Pull requests 747
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 72fac71

Browse filesBrowse the repository at this point in the historyBrowse files

Ceres6

authored and

aduh95

committed

module: clarify cjs global-like error on ModuleJobSync

PR-URL: #56491 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>

1 parent 3b85773 commit 72fac71
Copy full SHA for 72fac71

2 files changed

+40-24Lines changed: 40 additions & 24 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• lib/internal/modules/esm
  • module_job.js
• test/es-module
  • test-require-module-error-catching.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js

+39-23Lines changed: 39 additions & 23 deletions

Original file line number	Diff line number	Diff line change
@@ -58,6 +58,37 @@ const isCommonJSGlobalLikeNotDefinedError = (errorMessage) =>
58	58	(globalLike) => errorMessage === `${globalLike} is not defined`,
59	59	);
60	60
	61	+
	62	+ /**
	63	+ *
	64	+ * @param {Error} e
	65	+ * @param {string} url
	66	+ * @returns {void}
	67	+ */
	68	+ const explainCommonJSGlobalLikeNotDefinedError = (e, url) => {
	69	+ if (e?.name === 'ReferenceError' &&
	70	+ isCommonJSGlobalLikeNotDefinedError(e.message)) {
	71	+ e.message += ' in ES module scope';
	72	+
	73	+ if (StringPrototypeStartsWith(e.message, 'require ')) {
	74	+ e.message += ', you can use import instead';
	75	+ }
	76	+
	77	+ const packageConfig =
	78	+ StringPrototypeStartsWith(url, 'file://') &&
	79	+ RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, url) !== null &&
	80	+ require('internal/modules/package_json_reader')
	81	+ .getPackageScopeConfig(url);
	82	+ if (packageConfig.type === 'module') {
	83	+ e.message +=
	84	+ '\nThis file is being treated as an ES module because it has a ' +
	85	+ `'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
	86	+ '"type": "module". To treat it as a CommonJS script, rename it ' +
	87	+ 'to use the \'.cjs\' file extension.';
	88	+ }
	89	+ }
	90	+ };
	91	+
61	92	class ModuleJobBase {
62	93	constructor(url, importAttributes, isMain, inspectBrk) {
63	94	this.importAttributes = importAttributes;
@@ -273,27 +304,7 @@ class ModuleJob extends ModuleJobBase {
273	304	try {
274	305	await this.module.evaluate(timeout, breakOnSigint);
275	306	} catch (e) {
276		- if (e?.name === 'ReferenceError' &&
277		- isCommonJSGlobalLikeNotDefinedError(e.message)) {
278		- e.message += ' in ES module scope';
279		-
280		- if (StringPrototypeStartsWith(e.message, 'require ')) {
281		- e.message += ', you can use import instead';
282		- }
283		-
284		- const packageConfig =
285		- StringPrototypeStartsWith(this.module.url, 'file://') &&
286		- RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) !== null &&
287		- require('internal/modules/package_json_reader')
288		- .getPackageScopeConfig(this.module.url);
289		- if (packageConfig.type === 'module') {
290		- e.message +=
291		- '\nThis file is being treated as an ES module because it has a ' +
292		- `'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
293		- '"type": "module". To treat it as a CommonJS script, rename it ' +
294		- 'to use the \'.cjs\' file extension.';
295		- }
296		- }
	307	+ explainCommonJSGlobalLikeNotDefinedError(e, this.module.url);
297	308	throw e;
298	309	}
299	310	return { __proto__: null, module: this.module };
@@ -397,8 +408,13 @@ class ModuleJobSync extends ModuleJobBase {
397	408	throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
398	409	}
399	410	setHasStartedUserESMExecution();
400		- const namespace = this.module.evaluateSync(filename, parentFilename);
401		- return { __proto__: null, module: this.module, namespace };
	411	+ try {
	412	+ const namespace = this.module.evaluateSync(filename, parentFilename);
	413	+ return { __proto__: null, module: this.module, namespace };
	414	+ } catch (e) {
	415	+ explainCommonJSGlobalLikeNotDefinedError(e, this.module.url);
	416	+ throw e;
	417	+ }
402	418	}
403	419	}
404	420

Collapse file

‎test/es-module/test-require-module-error-catching.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-require-module-error-catching.js

+1-1Lines changed: 1 addition & 1 deletion

Original file line number	Diff line number	Diff line change
@@ -17,5 +17,5 @@ assert.throws(() => {
17	17	require('../fixtures/es-modules/reference-error-esm.js');
18	18	}, {
19	19	name: 'ReferenceError',
20		- message: 'exports is not defined'
	20	+ message: 'exports is not defined in ES module scope'
21	21	});

0 commit comments

Comments

0 (0)