Skip to content

Commit d30dbbf

Browse files
committed
lib: optimize path to file conversion
It seems `wrapSafe` filename is always absolute path. Certain aspects for validating paths in `pathToFileURL` can be skipped in favor of better performance of CJS synchrnous loading while keeping the origin.
1 parent 1c75eb7 commit d30dbbf

File tree

3 files changed

+13
-12
lines changed

3 files changed

+13
-12
lines changed

lib/internal/modules/cjs/loader.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -980,12 +980,18 @@ Module.prototype.require = function(id) {
980980
// (needed for setting breakpoint when called with --inspect-brk)
981981
let resolvedArgv;
982982
let hasPausedEntry = false;
983+
const absolutePathToUrlConverter = new URL('file://');
983984

984985
function wrapSafe(filename, content, cjsModuleInstance) {
986+
let filenameUrl = filename;
987+
if(path.isAbsolute(filename)) {
988+
absolutePathToUrlConverter.pathname = filename;
989+
filenameUrl = absolutePathToUrlConverter.href;
990+
}
985991
if (patched) {
986992
const wrapper = Module.wrap(content);
987993
return vm.runInThisContext(wrapper, {
988-
filename,
994+
filename: filenameUrl,
989995
lineOffset: 0,
990996
displayErrors: true,
991997
importModuleDynamically: async (specifier) => {
@@ -995,10 +1001,6 @@ function wrapSafe(filename, content, cjsModuleInstance) {
9951001
});
9961002
}
9971003
let compiled;
998-
let filenameUrl = filename;
999-
try {
1000-
filenameUrl = normalizeReferrerURL(filename);
1001-
} catch {}
10021004
try {
10031005
compiled = compileFunction(
10041006
content,

lib/internal/source_map/prepare_stack_trace.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
8181
);
8282
}
8383
// Show both original and transpiled stack trace information:
84-
const originalSourceNoScheme =
85-
StringPrototypeStartsWith(originalSource, 'file://') ?
86-
fileURLToPath(originalSource) : originalSource;
87-
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
84+
str += `\n -> ${originalSource}:${originalLine + 1}:` +
8885
`${originalColumn + 1}`;
8986
}
9087
}
@@ -137,7 +134,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
137134
prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'.
138135

139136
exceptionLine =
140-
`${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`;
137+
`${originalSource}:${firstLine}\n${line}\n${prefix}^\n\n`;
141138
return exceptionLine;
142139
}
143140

test/sequential/test-cli-syntax-require.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const common = require('../common');
44
const assert = require('assert');
55
const { exec } = require('child_process');
66
const fixtures = require('../common/fixtures');
7+
const { pathToFileURL } = require('url');
78

89
const node = process.execPath;
910

@@ -29,8 +30,9 @@ const syntaxErrorRE = /^SyntaxError: \b/m;
2930
// stderr should have a syntax error message
3031
assert(syntaxErrorRE.test(stderr), `${syntaxErrorRE} === ${stderr}`);
3132

32-
// stderr should include the filename
33-
assert(stderr.startsWith(file), `${stderr} starts with ${file}`);
33+
// stderr should include the file URL
34+
const fileUrl = pathToFileURL(file);
35+
assert(stderr.startsWith(fileUrl), `${stderr} starts with ${file}`);
3436
}));
3537
});
3638
});

0 commit comments

Comments
 (0)