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

nodejs / node Public

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

• Code
• Issues 632
• Pull requests 754
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit 72f2a22

Browse filesBrowse the repository at this point in the historyBrowse files

Ceres6

authored and

targos

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 9f46cda commit 72f2a22
Copy full SHA for 72f2a22

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
@@ -65,6 +65,37 @@ const isCommonJSGlobalLikeNotDefinedError = (errorMessage) =>
65	65	(globalLike) => errorMessage === `${globalLike} is not defined`,
66	66	);
67	67
	68	+
	69	+ /**
	70	+ *
	71	+ * @param {Error} e
	72	+ * @param {string} url
	73	+ * @returns {void}
	74	+ */
	75	+ const explainCommonJSGlobalLikeNotDefinedError = (e, url) => {
	76	+ if (e?.name === 'ReferenceError' &&
	77	+ isCommonJSGlobalLikeNotDefinedError(e.message)) {
	78	+ e.message += ' in ES module scope';
	79	+
	80	+ if (StringPrototypeStartsWith(e.message, 'require ')) {
	81	+ e.message += ', you can use import instead';
	82	+ }
	83	+
	84	+ const packageConfig =
	85	+ StringPrototypeStartsWith(url, 'file://') &&
	86	+ RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, url) !== null &&
	87	+ require('internal/modules/package_json_reader')
	88	+ .getPackageScopeConfig(url);
	89	+ if (packageConfig.type === 'module') {
	90	+ e.message +=
	91	+ '\nThis file is being treated as an ES module because it has a ' +
	92	+ `'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
	93	+ '"type": "module". To treat it as a CommonJS script, rename it ' +
	94	+ 'to use the \'.cjs\' file extension.';
	95	+ }
	96	+ }
	97	+ };
	98	+
68	99	class ModuleJobBase {
69	100	constructor(url, importAttributes, phase, isMain, inspectBrk) {
70	101	assert(typeof phase === 'number');
@@ -326,27 +357,7 @@ class ModuleJob extends ModuleJobBase {
326	357	try {
327	358	await this.module.evaluate(timeout, breakOnSigint);
328	359	} catch (e) {
329		- if (e?.name === 'ReferenceError' &&
330		- isCommonJSGlobalLikeNotDefinedError(e.message)) {
331		- e.message += ' in ES module scope';
332		-
333		- if (StringPrototypeStartsWith(e.message, 'require ')) {
334		- e.message += ', you can use import instead';
335		- }
336		-
337		- const packageConfig =
338		- StringPrototypeStartsWith(this.module.url, 'file://') &&
339		- RegExpPrototypeExec(/\.js(\?[^#]*)?(#.*)?$/, this.module.url) !== null &&
340		- require('internal/modules/package_json_reader')
341		- .getPackageScopeConfig(this.module.url);
342		- if (packageConfig.type === 'module') {
343		- e.message +=
344		- '\nThis file is being treated as an ES module because it has a ' +
345		- `'.js' file extension and '${packageConfig.pjsonPath}' contains ` +
346		- '"type": "module". To treat it as a CommonJS script, rename it ' +
347		- 'to use the \'.cjs\' file extension.';
348		- }
349		- }
	360	+ explainCommonJSGlobalLikeNotDefinedError(e, this.module.url);
350	361	throw e;
351	362	}
352	363	return { __proto__: null, module: this.module };
@@ -476,8 +487,13 @@ class ModuleJobSync extends ModuleJobBase {
476	487	throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
477	488	}
478	489	setHasStartedUserESMExecution();
479		- const namespace = this.module.evaluateSync(filename, parentFilename);
480		- return { __proto__: null, module: this.module, namespace };
	490	+ try {
	491	+ const namespace = this.module.evaluateSync(filename, parentFilename);
	492	+ return { __proto__: null, module: this.module, namespace };
	493	+ } catch (e) {
	494	+ explainCommonJSGlobalLikeNotDefinedError(e, this.module.url);
	495	+ throw e;
	496	+ }
481	497	}
482	498	}
483	499

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)