Skip to content
Open
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
3 changes: 2 additions & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1830,7 +1830,8 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
*/
Module.prototype._compile = function(content, filename, format) {
if (format === 'commonjs-typescript' || format === 'module-typescript' || format === 'typescript') {
content = stripTypeScriptModuleTypes(content, filename);
this[kURL] ??= convertCJSFilenameToURL(filename);
content = stripTypeScriptModuleTypes(content, filename, this[kURL]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of adding a new parameter to this function, we should pass it as filename

switch (format) {
case 'commonjs-typescript': {
format = 'commonjs';
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/modules/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@ function stripTypeScriptTypesForCoverage(code) {
* It is used by internal loaders.
* @param {string} source TypeScript code to parse.
* @param {string} filename The filename of the source code.
* @param {string} [sourceURL] The source URL of the source code. If not specified, use filename.
* @returns {TransformOutput} The stripped TypeScript code.
*/
function stripTypeScriptModuleTypes(source, filename) {
function stripTypeScriptModuleTypes(source, filename, sourceURL) {
assert(typeof source === 'string');
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
Expand All @@ -178,7 +179,7 @@ function stripTypeScriptModuleTypes(source, filename) {
const options = {
mode: 'strip-only',
sourceMap,
filename,
filename: sourceURL ?? filename,
};

const transpiled = processTypeScriptCode(source, options);
Expand Down
97 changes: 78 additions & 19 deletions test/parallel/test-inspector-strip-types.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,61 @@
// Verifies that type-stripped TypeScript reports a file: URL as its script
// sourceURL to the inspector across all module loading paths, hinting that it
// is a generated source.
'use strict';

const common = require('../common');
const { describe, it } = require('node:test');
common.skipIfInspectorDisabled();
if (!process.config.variables.node_use_amaro) common.skip('Requires Amaro');

const { NodeInstance } = require('../common/inspector-helper.js');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const { pathToFileURL } = require('url');

const scriptPath = fixtures.path('typescript/ts/test-typescript.ts');
const scriptURL = pathToFileURL(scriptPath);

async function runTest() {
const child = new NodeInstance(
['--inspect-brk=0'],
undefined,
scriptPath);
const scenarios = [
{
// CommonJS: a .ts entry point.
entry: 'typescript/ts/test-typescript.ts',
expected: [
fixtures.fileURL('typescript/ts/test-typescript.ts').href,
],
},
{
// CommonJS: a .ts entry that require()s a .cts dependency.
entry: 'typescript/ts/test-require-cts.ts',
expected: [
fixtures.fileURL('typescript/ts/test-require-cts.ts').href,
fixtures.fileURL('typescript/cts/test-cts-export-foo.cts').href,
],
},
{
// CommonJS: a .ts entry that require()s a .mts dependency.
entry: 'typescript/ts/test-require-mts.ts',
expected: [
fixtures.fileURL('typescript/ts/test-require-mts.ts').href,
fixtures.fileURL('typescript/mts/test-mts-export-foo.mts').href,
],
},
{
// ESM: a .mts entry that imports a .ts dependency.
entry: 'typescript/mts/test-import-ts-file.mts',
expected: [
fixtures.fileURL('typescript/mts/test-import-ts-file.mts').href,
fixtures.fileURL('typescript/mts/test-module-export.ts').href,
],
},
{
// ESM: a .mts entry that imports a .cts dependency.
entry: 'typescript/mts/test-import-commonjs.mts',
expected: [
fixtures.fileURL('typescript/mts/test-import-commonjs.mts').href,
fixtures.fileURL('typescript/cts/test-cts-export-foo.cts').href,
],
},
];

async function collectParsedScripts(scriptPath) {
const child = new NodeInstance(['--inspect-brk=0'], undefined, scriptPath);
const session = await child.connectInspectorSession();

await session.send({ method: 'NodeRuntime.enable' });
Expand All @@ -29,18 +67,39 @@ async function runTest() {
]);
await session.send({ method: 'NodeRuntime.disable' });

const scriptParsed = await session.waitForNotification((notification) => {
if (notification.method !== 'Debugger.scriptParsed') return false;

return notification.params.url === scriptPath || notification.params.url === scriptURL.href;
// Collect every parsed script while resuming through the break on start and
// any later pauses, until the main execution context is destroyed.
const scripts = new Map();
await session.waitForNotification((notification) => {
if (notification.method === 'Debugger.scriptParsed') {
const { url } = notification.params;
if (!url.startsWith('node:')) {
scripts.set(url, notification.params);
}
}
if (notification.method === 'Debugger.paused') {
session.send({ method: 'Debugger.resume' });
}
return notification.method === 'Runtime.executionContextDestroyed' &&
notification.params.executionContextId === 1;
});
// Verify that the script has a sourceURL, hinting that it is a generated source.
assert(scriptParsed.params.hasSourceURL || common.isInsideDirWithUnusualChars);

await session.waitForPauseOnStart();
await session.runToCompletion();
await session.waitForDisconnect();

assert.strictEqual((await child.expectShutdown()).exitCode, 0);
return scripts;
}

runTest().then(common.mustCall());
describe('type stripping source URLs', { concurrency: !process.env.TEST_PARALLEL }, () => {
for (const { entry, expected } of scenarios) {
it(entry, async () => {
const scripts = await collectParsedScripts(fixtures.path(entry));
for (const href of expected) {
const params = scripts.get(href);
assert(params,
`expected ${entry} to report ${href} to the inspector, ` +
`got:\n- ${[...scripts.keys()].join('\n- ')}`);
assert(params.hasSourceURL);
}
});
}
});
Loading