Skip to content

Commit d5bf3db

Browse files
legendecasaduh95
legendecas
authored and
aduh95
committed
lib: allow skipping source maps in node_modules
Files in `node_modules` are not authored by the user directly and the original sources are less relevant to the user. Skipping source maps in `node_modules` improves the general performance. Add `module.setSourceMapsSupport(enabled, options)` to skip source maps in `node_modules` if it is needed. This moves all source maps related API to `node:module` and this a step to promote the source maps API to stable. PR-URL: #56639 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent c78e110 commit d5bf3db

24 files changed

+492
-48
lines changed

‎benchmark/es/error-stack.js

+22-4
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@
22

33
const common = require('../common.js');
44
const modPath = require.resolve('../fixtures/simple-error-stack.js');
5+
const nodeModulePath = require.resolve('../fixtures/node_modules/error-stack/simple-error-stack.js');
6+
const Module = require('node:module');
57

68
const bench = common.createBenchmark(main, {
7-
method: ['without-sourcemap', 'sourcemap'],
9+
method: [
10+
'without-sourcemap',
11+
'sourcemap',
12+
'node-modules-without-sourcemap',
13+
'node-module-sourcemap'],
814
n: [1e5],
915
});
1016

11-
function runN(n) {
17+
function runN(n, modPath) {
1218
delete require.cache[modPath];
1319
const mod = require(modPath);
1420
bench.start();
@@ -22,11 +28,23 @@ function main({ n, method }) {
2228
switch (method) {
2329
case 'without-sourcemap':
2430
process.setSourceMapsEnabled(false);
25-
runN(n);
31+
runN(n, modPath);
2632
break;
2733
case 'sourcemap':
2834
process.setSourceMapsEnabled(true);
29-
runN(n);
35+
runN(n, modPath);
36+
break;
37+
case 'node-modules-without-sourcemap':
38+
Module.setSourceMapsSupport(true, {
39+
nodeModules: false,
40+
});
41+
runN(n, nodeModulePath);
42+
break;
43+
case 'node-modules-sourcemap':
44+
Module.setSourceMapsSupport(true, {
45+
nodeModules: true,
46+
});
47+
runN(n, nodeModulePath);
3048
break;
3149
default:
3250
throw new Error(`Unexpected method "${method}"`);

‎benchmark/fixtures/node_modules/error-stack/simple-error-stack.js

+16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎benchmark/fixtures/node_modules/error-stack/simple-error-stack.ts

+19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎doc/api/module.md

+40
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,20 @@ import { findSourceMap, SourceMap } from 'node:module';
14461446
const { findSourceMap, SourceMap } = require('node:module');
14471447
```
14481448
1449+
### `module.getSourceMapsSupport()`
1450+
1451+
<!-- YAML
1452+
added: REPLACEME
1453+
-->
1454+
1455+
* Returns: {Object}
1456+
* `enabled` {boolean} If the source maps support is enabled
1457+
* `nodeModules` {boolean} If the support is enabled for files in `node_modules`.
1458+
* `generatedCode` {boolean} If the support is enabled for generated code from `eval` or `new Function`.
1459+
1460+
This method returns whether the [Source Map v3][Source Map] support for stack
1461+
traces is enabled.
1462+
14491463
<!-- Anchors to make sure old links find a target -->
14501464
14511465
<a id="module_module_findsourcemap_path_error"></a>
@@ -1465,6 +1479,31 @@ added:
14651479
`path` is the resolved path for the file for which a corresponding source map
14661480
should be fetched.
14671481
1482+
### `module.setSourceMapsSupport(enabled[, options])`
1483+
1484+
<!-- YAML
1485+
added: REPLACEME
1486+
-->
1487+
1488+
* `enabled` {boolean} Enable the source map support.
1489+
* `options` {Object} Optional
1490+
* `nodeModules` {boolean} If enabling the support for files in
1491+
`node_modules`. **Default:** `false`.
1492+
* `generatedCode` {boolean} If enabling the support for generated code from
1493+
`eval` or `new Function`. **Default:** `false`.
1494+
1495+
This function enables or disables the [Source Map v3][Source Map] support for
1496+
stack traces.
1497+
1498+
It provides same features as launching Node.js process with commandline options
1499+
`--enable-source-maps`, with additional options to alter the support for files
1500+
in `node_modules` or generated codes.
1501+
1502+
Only source maps in JavaScript files that are loaded after source maps has been
1503+
enabled will be parsed and loaded. Preferably, use the commandline options
1504+
`--enable-source-maps` to avoid losing track of source maps of modules loaded
1505+
before this API call.
1506+
14681507
### Class: `module.SourceMap`
14691508
14701509
<!-- YAML
@@ -1565,6 +1604,7 @@ returned object contains the following keys:
15651604
[Customization hooks]: #customization-hooks
15661605
[ES Modules]: esm.md
15671606
[Permission Model]: permissions.md#permission-model
1607+
[Source Map]: https://sourcemaps.info/spec.html
15681608
[Source map v3 format]: https://sourcemaps.info/spec.html#h.mofvlxcwqzej
15691609
[V8 JavaScript code coverage]: https://v8project.blogspot.com/2017/12/javascript-code-coverage.html
15701610
[V8 code cache]: https://v8.dev/blog/code-caching-for-devs

‎doc/api/process.md

+7-2
Original file line numberDiff line numberDiff line change
@@ -3974,7 +3974,7 @@ added:
39743974
- v14.18.0
39753975
-->
39763976
3977-
> Stability: 1 - Experimental
3977+
> Stability: 1 - Experimental: Use [`module.setSourceMapsSupport()`][] instead.
39783978
39793979
* `val` {boolean}
39803980
@@ -3987,6 +3987,9 @@ It provides same features as launching Node.js process with commandline options
39873987
Only source maps in JavaScript files that are loaded after source maps has been
39883988
enabled will be parsed and loaded.
39893989
3990+
This implies calling `module.setSourceMapsSupport()` with an option
3991+
`{ nodeModules: true, generatedCode: true }`.
3992+
39903993
## `process.setUncaughtExceptionCaptureCallback(fn)`
39913994
39923995
<!-- YAML
@@ -4021,7 +4024,7 @@ added:
40214024
- v18.19.0
40224025
-->
40234026
4024-
> Stability: 1 - Experimental
4027+
> Stability: 1 - Experimental: Use [`module.getSourceMapsSupport()`][] instead.
40254028
40264029
* {boolean}
40274030
@@ -4488,7 +4491,9 @@ cases:
44884491
[`console.error()`]: console.md#consoleerrordata-args
44894492
[`console.log()`]: console.md#consolelogdata-args
44904493
[`domain`]: domain.md
4494+
[`module.getSourceMapsSupport()`]: module.md#modulegetsourcemapssupport
44914495
[`module.isBuiltin(id)`]: module.md#moduleisbuiltinmodulename
4496+
[`module.setSourceMapsSupport()`]: module.md#modulesetsourcemapssupportenabled-options
44924497
[`net.Server`]: net.md#class-netserver
44934498
[`net.Socket`]: net.md#class-netsocket
44944499
[`os.constants.dlopen`]: os.md#dlopen-constants

‎lib/internal/bootstrap/node.js

+13-4
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,8 @@ internalBinding('process_methods').setEmitWarningSync(emitWarningSync);
377377

378378
{
379379
const {
380-
getSourceMapsEnabled,
381-
setSourceMapsEnabled,
380+
getSourceMapsSupport,
381+
setSourceMapsSupport,
382382
maybeCacheGeneratedSourceMap,
383383
} = require('internal/source_map/source_map_cache');
384384
const {
@@ -390,10 +390,19 @@ internalBinding('process_methods').setEmitWarningSync(emitWarningSync);
390390
enumerable: true,
391391
configurable: true,
392392
get() {
393-
return getSourceMapsEnabled();
393+
return getSourceMapsSupport().enabled;
394394
},
395395
});
396-
process.setSourceMapsEnabled = setSourceMapsEnabled;
396+
process.setSourceMapsEnabled = function setSourceMapsEnabled(val) {
397+
setSourceMapsSupport(val, {
398+
__proto__: null,
399+
// TODO(legendecas): In order to smoothly improve the source map support,
400+
// skip source maps in node_modules and generated code with
401+
// `process.setSourceMapsEnabled(true)` in a semver major version.
402+
nodeModules: val,
403+
generatedCode: val,
404+
});
405+
};
397406
// The C++ land calls back to maybeCacheGeneratedSourceMap()
398407
// when code is generated by user with eval() or new Function()
399408
// to cache the source maps from the evaluated code, if any.

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const {
3030
} = internalBinding('util');
3131
const { decorateErrorStack, kEmptyObject } = require('internal/util');
3232
const {
33-
getSourceMapsEnabled,
33+
getSourceMapsSupport,
3434
} = require('internal/source_map/source_map_cache');
3535
const assert = require('internal/assert');
3636
const resolvedPromise = PromiseResolve();
@@ -185,7 +185,7 @@ class ModuleJob extends ModuleJobBase {
185185
// of missing named export. This is currently not possible because
186186
// stack trace originates in module_job, not the file itself. A hidden
187187
// symbol with filename could be set in node_errors.cc to facilitate this.
188-
if (!getSourceMapsEnabled() &&
188+
if (!getSourceMapsSupport().enabled &&
189189
StringPrototypeIncludes(e.message,
190190
' does not provide an export named')) {
191191
const splitStack = StringPrototypeSplit(e.stack, '\n');

‎lib/internal/process/pre_execution.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -698,9 +698,17 @@ function initializeESMLoader(forceDefaultLoader) {
698698

699699
function initializeSourceMapsHandlers() {
700700
const {
701-
setSourceMapsEnabled,
701+
setSourceMapsSupport,
702702
} = require('internal/source_map/source_map_cache');
703-
setSourceMapsEnabled(getOptionValue('--enable-source-maps'));
703+
const enabled = getOptionValue('--enable-source-maps');
704+
setSourceMapsSupport(enabled, {
705+
__proto__: null,
706+
// TODO(legendecas): In order to smoothly improve the source map support,
707+
// skip source maps in node_modules and generated code with
708+
// `--enable-source-maps` in a semver major version.
709+
nodeModules: enabled,
710+
generatedCode: enabled,
711+
});
704712
}
705713

706714
function initializeFrozenIntrinsics() {

‎lib/internal/source_map/source_map_cache.js

+46-19
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const {
44
ArrayPrototypePush,
55
JSONParse,
6+
ObjectFreeze,
67
RegExpPrototypeExec,
78
SafeMap,
89
StringPrototypeCodePointAt,
@@ -15,15 +16,15 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
1516
debug = fn;
1617
});
1718

18-
const { validateBoolean } = require('internal/validators');
19+
const { validateBoolean, validateObject } = require('internal/validators');
1920
const {
2021
setSourceMapsEnabled: setSourceMapsNative,
2122
} = internalBinding('errors');
2223
const {
2324
defaultPrepareStackTrace,
2425
setInternalPrepareStackTrace,
2526
} = require('internal/errors');
26-
const { getLazy } = require('internal/util');
27+
const { getLazy, isUnderNodeModules, kEmptyObject } = require('internal/util');
2728

2829
const getModuleSourceMapCache = getLazy(() => {
2930
const { SourceMapCacheMap } = require('internal/source_map/source_map_cache_map');
@@ -45,30 +46,48 @@ const { fileURLToPath, pathToFileURL, URL, URLParse } = require('internal/url');
4546
let SourceMap;
4647

4748
// This is configured with --enable-source-maps during pre-execution.
48-
let sourceMapsEnabled = false;
49-
function getSourceMapsEnabled() {
50-
return sourceMapsEnabled;
49+
let sourceMapsSupport = ObjectFreeze({
50+
__proto__: null,
51+
enabled: false,
52+
nodeModules: false,
53+
generatedCode: false,
54+
});
55+
function getSourceMapsSupport() {
56+
// Return a read-only object.
57+
return sourceMapsSupport;
5158
}
5259

5360
/**
5461
* Enables or disables source maps programmatically.
55-
* @param {boolean} val
62+
* @param {boolean} enabled
63+
* @param {object} options
64+
* @param {boolean} [options.nodeModules]
65+
* @param {boolean} [options.generatedCode]
5666
*/
57-
function setSourceMapsEnabled(val) {
58-
validateBoolean(val, 'val');
67+
function setSourceMapsSupport(enabled, options = kEmptyObject) {
68+
validateBoolean(enabled, 'enabled');
69+
validateObject(options, 'options');
70+
71+
const { nodeModules = false, generatedCode = false } = options;
72+
validateBoolean(nodeModules, 'options.nodeModules');
73+
validateBoolean(generatedCode, 'options.generatedCode');
5974

60-
setSourceMapsNative(val);
61-
if (val) {
75+
setSourceMapsNative(enabled);
76+
if (enabled) {
6277
const {
6378
prepareStackTraceWithSourceMaps,
6479
} = require('internal/source_map/prepare_stack_trace');
6580
setInternalPrepareStackTrace(prepareStackTraceWithSourceMaps);
66-
} else if (sourceMapsEnabled !== undefined) {
67-
// Reset prepare stack trace callback only when disabling source maps.
81+
} else {
6882
setInternalPrepareStackTrace(defaultPrepareStackTrace);
6983
}
7084

71-
sourceMapsEnabled = val;
85+
sourceMapsSupport = ObjectFreeze({
86+
__proto__: null,
87+
enabled,
88+
nodeModules: nodeModules,
89+
generatedCode: generatedCode,
90+
});
7291
}
7392

7493
/**
@@ -130,14 +149,18 @@ function extractSourceMapURLMagicComment(content) {
130149
* @param {string | undefined} sourceMapURL - the source map url
131150
*/
132151
function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
133-
const sourceMapsEnabled = getSourceMapsEnabled();
134-
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
152+
const support = getSourceMapsSupport();
153+
if (!(process.env.NODE_V8_COVERAGE || support.enabled)) return;
135154
const { normalizeReferrerURL } = require('internal/modules/helpers');
136155
filename = normalizeReferrerURL(filename);
137156
if (filename === undefined) {
138157
// This is most likely an invalid filename in sourceURL of [eval]-wrapper.
139158
return;
140159
}
160+
if (!support.nodeModules && isUnderNodeModules(filename)) {
161+
// Skip file under node_modules if not enabled.
162+
return;
163+
}
141164

142165
if (sourceMapURL === undefined) {
143166
sourceMapURL = extractSourceMapURLMagicComment(content);
@@ -185,8 +208,8 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc
185208
* @param {string} content - the eval'd source code
186209
*/
187210
function maybeCacheGeneratedSourceMap(content) {
188-
const sourceMapsEnabled = getSourceMapsEnabled();
189-
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
211+
const support = getSourceMapsSupport();
212+
if (!(process.env.NODE_V8_COVERAGE || support.enabled || support.generated)) return;
190213

191214
const sourceURL = extractSourceURLMagicComment(content);
192215
if (sourceURL === null) {
@@ -352,6 +375,10 @@ function findSourceMap(sourceURL) {
352375
return undefined;
353376
}
354377

378+
if (!getSourceMapsSupport().nodeModules && isUnderNodeModules(sourceURL)) {
379+
return undefined;
380+
}
381+
355382
SourceMap ??= require('internal/source_map/source_map').SourceMap;
356383
try {
357384
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
@@ -377,8 +404,8 @@ function findSourceMap(sourceURL) {
377404

378405
module.exports = {
379406
findSourceMap,
380-
getSourceMapsEnabled,
381-
setSourceMapsEnabled,
407+
getSourceMapsSupport,
408+
setSourceMapsSupport,
382409
maybeCacheSourceMap,
383410
maybeCacheGeneratedSourceMap,
384411
sourceMapCacheToObject,

0 commit comments

Comments
 (0)