module: refactor to use `normalizeRequirableId` in the CJS module loader · nodejs/node@6f0458d · 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 6f0458d

Browse filesBrowse the repository at this point in the historyBrowse files

RaisinTen

authored and

targos

committed

module: refactor to use normalizeRequirableId in the CJS module loader

`BuiltinModule.normalizeRequirableId()` was introduced in #47779 to fix a bug in the require function of SEAs and Snapshots, so that built-in modules with the `node:` scheme could be required correctly. This change makes more use of this API instead of `BuiltinModule.canBeRequiredByUsers()` and `BuiltinModule.canBeRequiredWithoutScheme()` to reduce chances of such bugs. Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #47896 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

1 parent 12d731e commit 6f0458d
Copy full SHA for 6f0458d

3 files changed

+13-26Lines changed: 13 additions & 26 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• lib/internal
  • bootstrap
    • realm.js
  • modules/cjs
    • loader.js
• test/fixtures/errors
  • force_colors.snapshot

Expand file treeCollapse file tree

Open diff view settings

Collapse file

‎lib/internal/bootstrap/realm.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/realm.js

+7-8Lines changed: 7 additions & 8 deletions

Original file line number	Diff line number	Diff line change
@@ -300,17 +300,16 @@ class BuiltinModule {
300	300	}
301	301
302	302	static normalizeRequirableId(id) {
303		- let normalizedId = id;
304	303	if (StringPrototypeStartsWith(id, 'node:')) {
305		- normalizedId = StringPrototypeSlice(id, 5);
306		- }
307		-
308		- if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
309		- (id === normalizedId && !BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
310		- return undefined;
	304	+ const normalizedId = StringPrototypeSlice(id, 5);
	305	+ if (BuiltinModule.canBeRequiredByUsers(normalizedId)) {
	306	+ return normalizedId;
	307	+ }
	308	+ } else if (BuiltinModule.canBeRequiredWithoutScheme(id)) {
	309	+ return id;
311	310	}
312	311
313		- return normalizedId;
	312	+ return undefined;
314	313	}
315	314
316	315	static getSchemeOnlyModuleNames() {

Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js

+2-14Lines changed: 2 additions & 14 deletions

Original file line number	Diff line number	Diff line change
@@ -782,12 +782,7 @@ if (isWindows) {
782	782	}
783	783
784	784	Module._resolveLookupPaths = function(request, parent) {
785		- if ((
786		- StringPrototypeStartsWith(request, 'node:') &&
787		- BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
788		- ) || (
789		- BuiltinModule.canBeRequiredWithoutScheme(request)
790		- )) {
	785	+ if (BuiltinModule.normalizeRequirableId(request)) {
791	786	debug('looking for %j in []', request);
792	787	return null;
793	788	}
@@ -979,14 +974,7 @@ Module._load = function(request, parent, isMain) {
979	974	};
980	975
981	976	Module._resolveFilename = function(request, parent, isMain, options) {
982		- if (
983		- (
984		- StringPrototypeStartsWith(request, 'node:') &&
985		- BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
986		- ) || (
987		- BuiltinModule.canBeRequiredWithoutScheme(request)
988		- )
989		- ) {
	977	+ if (BuiltinModule.normalizeRequirableId(request)) {
990	978	return request;
991	979	}
992	980

Collapse file

‎test/fixtures/errors/force_colors.snapshot‎

Copy file name to clipboardExpand all lines: test/fixtures/errors/force_colors.snapshot

+4-4Lines changed: 4 additions & 4 deletions

Original file line number	Diff line number	Diff line change
@@ -4,10 +4,10 @@ throw new Error('Should include grayed stack trace')
4	4
5	5	Error: Should include grayed stack trace
6	6	at Object.<anonymous> (/test*force_colors.js:1:7)
7		-  at Module._compile (node:internal*modules*cjs*loader:1257:14)
8		-  at Module._extensions..js (node:internal*modules*cjs*loader:1311:10)
9		-  at Module.load (node:internal*modules*cjs*loader:1115:32)
10		-  at Module._load (node:internal*modules*cjs*loader:955:12)
	7	+  at Module._compile (node:internal*modules*cjs*loader:1245:14)
	8	+  at Module._extensions..js (node:internal*modules*cjs*loader:1299:10)
	9	+  at Module.load (node:internal*modules*cjs*loader:1103:32)
	10	+  at Module._load (node:internal*modules*cjs*loader:950:12)
11	11	 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:88:12)
12	12	 at node:internal*main*run_main_module:23:47
13	13

0 commit comments

Comments

0 (0)