Skip to content

Commit 1a56215

Browse files
committed
use built-in source map support in node v20
The @cspotcode/source-map-support module does not function properly on Node 20, resulting in incorrect stack traces. Fortunately, the built-in source map support in Node is now quite reliable. This does have the following (somewhat subtle) changes to error output: - When a call site is in a method defined within a constructor function, it picks up the function name *as well as* the type name and method name. So, in tests where a method is called and throws within the a function constructor, we see `Foo.Foo.bar` instead of `Foo.bar`. - Call sites displays show filenames instead of file URLs. - The call site display puts the `^` character under the `throw` rather than the construction point of the error object. This is closer to how normal un-transpiled JavaScript behaves, and thus somewhat preferrable, but isn't possible when all we have to go on is the Error stack property, so it is a change. I haven't been able to figure out why exactly, but the call sites appear to be somewhat different in the repl/eval contexts as a result of this change. It almost seems like the @cspotcode/source-map-support was applying source maps to the vm-evaluated scripts, but I don't see how that could be, and in fact, there's a comment in the code stating that that *isn't* the case. But the line number showing up in an Error.stack property is `1` prior to this change (matching the location in the TS source) and is `2` afterwards (matching the location in the compiled JS). An argument could be made that specific line numbers are a bit meaningless in a REPL anyway, and the best approach is to just make those tests accept either result. One possible approach to provide built-in source map support for the repl would be to refactor the `appendCompileAndEvalInput` to diff and append the *input* TS, and compile within the `runInContext` method. If the transpiled code was prepended with `process.setSourceMapsEnabled(true);`, then Error stacks and call sites would be properly source mapped by Node internally.
1 parent 2bdfea6 commit 1a56215

File tree

4 files changed

+47
-51
lines changed

4 files changed

+47
-51
lines changed

src/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,13 @@ export function createFromPreloadedConfig(foundConfigResult: ReturnType<typeof f
691691
}
692692

693693
// Install source map support and read from memory cache.
694+
const useBuiltInSourceMaps = versionGteLt(process.versions.node, '20.0.0');
694695
function installSourceMapSupport() {
696+
if (useBuiltInSourceMaps) {
697+
//@ts-ignore added to node somewhat recently, not yet in DT.
698+
process.setSourceMapsEnabled(true);
699+
return;
700+
}
695701
const sourceMapSupport = require('@cspotcode/source-map-support') as typeof _sourceMapSupport;
696702
sourceMapSupport.install({
697703
environment: 'node',

src/test/esm-loader.spec.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { join, resolve } from 'path';
2525
import * as expect from 'expect';
2626
import type { NodeLoaderHooksAPI2 } from '../';
2727
import { pathToFileURL } from 'url';
28+
import { versionGteLt } from '../util';
2829

2930
const test = context(ctxTsNode);
3031

@@ -48,16 +49,18 @@ test.suite('esm', (test) => {
4849
cwd: join(TEST_DIR, './esm'),
4950
});
5051
expect(r.err).not.toBe(null);
51-
const expectedModuleUrl = pathToFileURL(join(TEST_DIR, './esm/throw error.ts')).toString();
52+
// on node 20, this will be a path. prior versions, a file: url
53+
const expectedModulePath = join(TEST_DIR, './esm/throw error.ts');
54+
const expectedModuleUrl = pathToFileURL(expectedModulePath).toString();
55+
const expectedModulePrint = versionGteLt(process.versions.node, '20.0.0') ? expectedModulePath : expectedModuleUrl;
5256
expect(r.err!.message).toMatch(
53-
[
54-
`${expectedModuleUrl}:100`,
55-
" bar() { throw new Error('this is a demo'); }",
56-
' ^',
57-
'Error: this is a demo',
58-
` at Foo.bar (${expectedModuleUrl}:100:17)`,
59-
].join('\n')
57+
[`${expectedModulePrint}:100`, " bar() { throw new Error('this is a demo'); }"].join('\n')
6058
);
59+
// the ^ and number of line-breaks is platform-specific
60+
// also, node 20 puts the type in here when source mapping it, so it
61+
// shows as Foo.Foo.bar
62+
expect(r.err!.message).toMatch(/^ at (Foo\.){1,2}bar \(/m);
63+
expect(r.err!.message).toMatch(`Foo.bar (${expectedModulePrint}:100:17)`);
6164
});
6265

6366
test.suite('supports experimental-specifier-resolution=node', (test) => {
@@ -95,7 +98,7 @@ test.suite('esm', (test) => {
9598
});
9699
expect(r.err).not.toBe(null);
97100
// expect error from node's default resolver
98-
expect(r.stderr).toMatch(/Error \[ERR_UNSUPPORTED_ESM_URL_SCHEME\]:.*(?:\n.*){0,2}\n *at default(Load|Resolve)/);
101+
expect(r.stderr).toMatch(/Error \[ERR_UNSUPPORTED_ESM_URL_SCHEME\]:.*(?:\n.*){0,10}\n *at default(Load|Resolve)/);
99102
});
100103

101104
test('should bypass import cache when changing search params', async () => {

src/test/index.spec.ts

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,7 @@ test.suite('ts-node', (test) => {
182182
}
183183

184184
expect(r.err.message).toMatch(
185-
[
186-
`${join(TEST_DIR, 'throw error.ts')}:100`,
187-
" bar() { throw new Error('this is a demo'); }",
188-
' ^',
189-
'Error: this is a demo',
190-
].join('\n')
185+
[`${join(TEST_DIR, 'throw error.ts')}:100`, " bar() { throw new Error('this is a demo'); }"].join('\n')
191186
);
192187
});
193188

@@ -198,12 +193,7 @@ test.suite('ts-node', (test) => {
198193
}
199194

200195
expect(r.err.message).toMatch(
201-
[
202-
`${join(TEST_DIR, 'throw error.ts')}:100`,
203-
" bar() { throw new Error('this is a demo'); }",
204-
' ^',
205-
'Error: this is a demo',
206-
].join('\n')
196+
[`${join(TEST_DIR, 'throw error.ts')}:100`, " bar() { throw new Error('this is a demo'); }"].join('\n')
207197
);
208198
});
209199

@@ -214,11 +204,7 @@ test.suite('ts-node', (test) => {
214204
}
215205

216206
expect(r.err.message).toMatch(
217-
[
218-
`${join(TEST_DIR, 'throw error.ts')}:100`,
219-
" bar() { throw new Error('this is a demo'); }",
220-
' ^',
221-
].join('\n')
207+
[`${join(TEST_DIR, 'throw error.ts')}:100`, " bar() { throw new Error('this is a demo'); }"].join('\n')
222208
);
223209
});
224210

@@ -309,25 +295,19 @@ test.suite('ts-node', (test) => {
309295
const r = await exec(`${CMD_TS_NODE_WITH_PROJECT_FLAG} "throw error react tsx.tsx"`);
310296
expect(r.err).not.toBe(null);
311297
expect(r.err!.message).toMatch(
312-
[
313-
`${join(TEST_DIR, './throw error react tsx.tsx')}:100`,
314-
" bar() { throw new Error('this is a demo'); }",
315-
' ^',
316-
'Error: this is a demo',
317-
].join('\n')
298+
[`${join(TEST_DIR, './throw error react tsx.tsx')}:100`, " bar() { throw new Error('this is a demo'); }"].join(
299+
'\n'
300+
)
318301
);
319302
});
320303

321304
test('should use source maps with react tsx in --transpile-only mode', async () => {
322305
const r = await exec(`${CMD_TS_NODE_WITH_PROJECT_FLAG} --transpile-only "throw error react tsx.tsx"`);
323306
expect(r.err).not.toBe(null);
324307
expect(r.err!.message).toMatch(
325-
[
326-
`${join(TEST_DIR, './throw error react tsx.tsx')}:100`,
327-
" bar() { throw new Error('this is a demo'); }",
328-
' ^',
329-
'Error: this is a demo',
330-
].join('\n')
308+
[`${join(TEST_DIR, './throw error react tsx.tsx')}:100`, " bar() { throw new Error('this is a demo'); }"].join(
309+
'\n'
310+
)
331311
);
332312
});
333313

src/test/sourcemaps.spec.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as expect from 'expect';
2+
import { versionGteLt } from '../util';
23
import { createExec, createExecTester, CMD_TS_NODE_WITH_PROJECT_FLAG, ctxTsNode, TEST_DIR } from './helpers';
34
import { context } from './testlib';
45
const test = context(ctxTsNode);
@@ -10,17 +11,23 @@ const exec = createExecTester({
1011
}),
1112
});
1213

13-
test('Redirects source-map-support to @cspotcode/source-map-support so that third-party libraries get correct source-mapped locations', async () => {
14-
const r = await exec({
15-
flags: `./legacy-source-map-support-interop/index.ts`,
14+
const useBuiltInSourceMaps = versionGteLt(process.versions.node, '20.0.0');
15+
16+
if (useBuiltInSourceMaps) {
17+
test.skip('Skip source-map-support redirection on node 20', () => {});
18+
} else {
19+
test('Redirects source-map-support to @cspotcode/source-map-support so that third-party libraries get correct source-mapped locations', async () => {
20+
const r = await exec({
21+
flags: `./legacy-source-map-support-interop/index.ts`,
22+
});
23+
expect(r.err).toBeNull();
24+
expect(r.stdout.split('\n')).toMatchObject([
25+
expect.stringContaining('.ts:2 '),
26+
'true',
27+
'true',
28+
expect.stringContaining('.ts:100:'),
29+
expect.stringContaining('.ts:101 '),
30+
'',
31+
]);
1632
});
17-
expect(r.err).toBeNull();
18-
expect(r.stdout.split('\n')).toMatchObject([
19-
expect.stringContaining('.ts:2 '),
20-
'true',
21-
'true',
22-
expect.stringContaining('.ts:100:'),
23-
expect.stringContaining('.ts:101 '),
24-
'',
25-
]);
26-
});
33+
}

0 commit comments

Comments
 (0)