Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src,lib: retrieve parsed source map url from v8 #44798

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,41 @@ console.log(globalVar);
// 1000
```

### `script.sourceMapURL`

<!-- YAML
added: REPLACEME
-->

* {string|undefined}

When the script is compiled from a source that contains a source map magic
comment, this property will be set to the URL of the source map.

```mjs
import vm from 'node:vm';

const script = new vm.Script(`
function myFunc() {}
//# sourceMappingURL=sourcemap.json
`);

console.log(script.sourceMapURL);
// Prints: sourcemap.json
```

```cjs
const vm = require('node:vm');
legendecas marked this conversation as resolved.
Show resolved Hide resolved

const script = new vm.Script(`
function myFunc() {}
//# sourceMappingURL=sourcemap.json
`);

console.log(script.sourceMapURL);
// Prints: sourcemap.json
```

## Class: `vm.Module`

<!-- YAML
Expand Down
26 changes: 21 additions & 5 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ const {
filterOwnProperties,
setOwnProperty,
} = require('internal/util');
const vm = require('vm');
const { Script } = require('vm');
const { internalCompileFunction } = require('internal/vm');
const assert = require('internal/assert');
const fs = require('fs');
const internalFS = require('internal/fs/utils');
Expand Down Expand Up @@ -1073,19 +1074,28 @@ let hasPausedEntry = false;
function wrapSafe(filename, content, cjsModuleInstance) {
if (patched) {
const wrapper = Module.wrap(content);
return vm.runInThisContext(wrapper, {
const script = new Script(wrapper, {
filename,
lineOffset: 0,
displayErrors: true,
importModuleDynamically: async (specifier, _, importAssertions) => {
const loader = asyncESM.esmLoader;
return loader.import(specifier, normalizeReferrerURL(filename),
importAssertions);
},
});

// Cache the source map for the module if present.
if (script.sourceMapURL) {
maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL);
}

return script.runInThisContext({
displayErrors: true,
});
}

try {
return vm.compileFunction(content, [
const result = internalCompileFunction(content, [
'exports',
'require',
'module',
Expand All @@ -1099,6 +1109,13 @@ function wrapSafe(filename, content, cjsModuleInstance) {
importAssertions);
},
});

// Cache the source map for the module if present.
if (result.sourceMapURL) {
maybeCacheSourceMap(filename, content, this, false, undefined, result.sourceMapURL);
}

return result.function;
} catch (err) {
if (process.mainModule === cjsModuleInstance)
enrichCJSError(err, content);
Expand All @@ -1119,7 +1136,6 @@ Module.prototype._compile = function(content, filename) {
policy.manifest.assertIntegrity(moduleURL, content);
}

maybeCacheSourceMap(filename, content, this);
const compiledWrapper = wrapSafe(filename, content, this);

let inspectorWrapper = null;
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,9 +579,11 @@ function initializeESMLoader() {
}

function initializeSourceMapsHandlers() {
const { setSourceMapsEnabled } =
const { setSourceMapsEnabled, getSourceMapsEnabled } =
require('internal/source_map/source_map_cache');
process.setSourceMapsEnabled = setSourceMapsEnabled;
// Initialize the environment flag of source maps.
getSourceMapsEnabled();
}

function initializeFrozenIntrinsics() {
Expand Down
98 changes: 56 additions & 42 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,21 @@ function extractSourceURLMagicComment(content) {
return sourceURL;
}

function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL) {
function extractSourceMapURLMagicComment(content) {
let match;
let lastMatch;
// A while loop is used here to get the last occurrence of sourceMappingURL.
// This is needed so that we don't match sourceMappingURL in string literals.
while ((match = RegExpPrototypeExec(kSourceMappingURLMagicComment, content))) {
lastMatch = match;
}
if (lastMatch == null) {
return null;
}
return lastMatch.groups.sourceMappingURL;
}

function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
const sourceMapsEnabled = getSourceMapsEnabled();
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
try {
Expand All @@ -108,52 +122,52 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
return;
}

let match;
let lastMatch;
// A while loop is used here to get the last occurrence of sourceMappingURL.
// This is needed so that we don't match sourceMappingURL in string literals.
while ((match = RegExpPrototypeExec(kSourceMappingURLMagicComment, content))) {
lastMatch = match;
if (sourceMapURL === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it by design that we still do the regex parse when V8 told us that there is no source map? If so, should we add a test case, e.g.:

checkSourceMapUrl(`
function myFunc() {}
`
//# sourceMappingURL=sourcemap.json
`;
`, 'sourcemap.json');

Copy link
Member

@joyeecheung joyeecheung Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fallback is kept here to handle the ES modules?

Copy link
Member Author

@legendecas legendecas Oct 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkrems @joyeecheung thanks for the suggestion. We don't need to apply the regex again when the parse result of the script indicates that no source mapping URL is available. I've updated the code to eliminate the duplicated scans.

It is true that the fallback is left here to handle the ES modules.

sourceMapURL = extractSourceMapURLMagicComment(content);
}

// Bail out when there is no source map url.
if (typeof sourceMapURL !== 'string') {
return;
}

if (sourceURL === undefined) {
sourceURL = extractSourceURLMagicComment(content);
}
if (lastMatch) {
const data = dataFromUrl(filename, lastMatch.groups.sourceMappingURL);
const url = data ? null : lastMatch.groups.sourceMappingURL;
if (cjsModuleInstance) {
cjsSourceMapCache.set(cjsModuleInstance, {
filename,
lineLengths: lineLengths(content),
data,
url,
sourceURL,
});
} else if (isGeneratedSource) {
const entry = {
lineLengths: lineLengths(content),
data,
url,
sourceURL
};
generatedSourceMapCache.set(filename, entry);
if (sourceURL) {
generatedSourceMapCache.set(sourceURL, entry);
}
} else {
// If there is no cjsModuleInstance and is not generated source assume we are in a
// "modules/esm" context.
const entry = {
lineLengths: lineLengths(content),
data,
url,
sourceURL,
};
esmSourceMapCache.set(filename, entry);
if (sourceURL) {
esmSourceMapCache.set(sourceURL, entry);
}

const data = dataFromUrl(filename, sourceMapURL);
const url = data ? null : sourceMapURL;
if (cjsModuleInstance) {
cjsSourceMapCache.set(cjsModuleInstance, {
filename,
lineLengths: lineLengths(content),
data,
url,
sourceURL,
});
} else if (isGeneratedSource) {
const entry = {
lineLengths: lineLengths(content),
data,
url,
sourceURL
};
generatedSourceMapCache.set(filename, entry);
if (sourceURL) {
generatedSourceMapCache.set(sourceURL, entry);
}
} else {
// If there is no cjsModuleInstance and is not generated source assume we are in a
// "modules/esm" context.
const entry = {
lineLengths: lineLengths(content),
data,
url,
sourceURL,
};
esmSourceMapCache.set(filename, entry);
if (sourceURL) {
esmSourceMapCache.set(sourceURL, entry);
}
}
}
Expand Down
113 changes: 113 additions & 0 deletions lib/internal/vm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
'use strict';

const {
ArrayPrototypeForEach,
} = primordials;

const {
compileFunction,
isContext: _isContext,
} = internalBinding('contextify');
const {
validateArray,
validateBoolean,
validateBuffer,
validateFunction,
validateObject,
validateString,
validateUint32,
} = require('internal/validators');
const {
ERR_INVALID_ARG_TYPE,
} = require('internal/errors').codes;

function isContext(object) {
validateObject(object, 'object', { __proto__: null, allowArray: true });

return _isContext(object);
}

function internalCompileFunction(code, params, options) {
validateString(code, 'code');
if (params !== undefined) {
validateArray(params, 'params');
ArrayPrototypeForEach(params,
(param, i) => validateString(param, `params[${i}]`));
}

const {
filename = '',
columnOffset = 0,
lineOffset = 0,
cachedData = undefined,
produceCachedData = false,
parsingContext = undefined,
contextExtensions = [],
importModuleDynamically,
} = options;

validateString(filename, 'options.filename');
validateUint32(columnOffset, 'options.columnOffset');
validateUint32(lineOffset, 'options.lineOffset');
if (cachedData !== undefined)
validateBuffer(cachedData, 'options.cachedData');
validateBoolean(produceCachedData, 'options.produceCachedData');
if (parsingContext !== undefined) {
if (
typeof parsingContext !== 'object' ||
parsingContext === null ||
!isContext(parsingContext)
) {
throw new ERR_INVALID_ARG_TYPE(
'options.parsingContext',
'Context',
parsingContext
);
}
}
validateArray(contextExtensions, 'options.contextExtensions');
ArrayPrototypeForEach(contextExtensions, (extension, i) => {
const name = `options.contextExtensions[${i}]`;
validateObject(extension, name, { __proto__: null, nullable: true });
});

const result = compileFunction(
code,
filename,
lineOffset,
columnOffset,
cachedData,
produceCachedData,
parsingContext,
contextExtensions,
params
);

if (produceCachedData) {
result.function.cachedDataProduced = result.cachedDataProduced;
}

if (result.cachedData) {
result.function.cachedData = result.cachedData;
}

if (importModuleDynamically !== undefined) {
validateFunction(importModuleDynamically,
'options.importModuleDynamically');
const { importModuleDynamicallyWrap } =
require('internal/vm/module');
legendecas marked this conversation as resolved.
Show resolved Hide resolved
const { callbackMap } = internalBinding('module_wrap');
const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
const func = result.function;
callbackMap.set(result.cacheKey, {
importModuleDynamically: (s, _k, i) => wrapped(s, func, i),
});
}

return result;
}

module.exports = {
internalCompileFunction,
isContext,
};
Loading