Repository navigation
Malformed source URLs in source map cache on Windows #1769
Description
Activity
This is very likely to be caused by that
- Node.js uses
sourcesToAbsoluteto parse the paths in thesourcesfield for the source map cache, and as you can see in L181, it relies on theURLconstructor,return new URL(source, baseURL).href; // baseURL is guaranteed to be a file URL, file:///xxx
- Node.js current implementation of the constructor will parse the Windows disk letter, along with
:, as the protocol,> new URL('D:/Example').href 'd:/Example' > new URL('D:/Example', 'file:///Whatever').href 'd:/Example'
So I suggest we use either file URLs (PaperStrike@134bef2) or relative paths (PaperStrike@2a933ee).
- Node.js uses
Thanks for this. It'll probably take me a few days before I can properly review, merge, and publish this. The push to prepare and publish 10.8.0 ahead of TS 4.7 was time-consuming, so I've had to switch gears this week and play catch-up on some other stuff.
One thing that I want to make sure we get right:
Node's stack traces from ESM files use file URLs, but from CommonJS files, they use native paths. I remember writing some logic to ensure that we preserve this behavior even after we've source-mapped the stack frames.
Error at D:\this\is\a\commonjs\files.cjs:10:10 at file:///d/this/is/a/esm/file.mjs:10:10I want to be sure that, after we apply sourcemaps to a stack trace, the CJS files still use native paths, since that's the way vanilla node behaves.
Reacted by 华@cspotcode Thank you, I hadn't noticed the stack traces of different types of files, and hadn't considered the impact of the PRs on them.
So just now I wrote a test case locally, and luckily found that neither #1770 nor #1771 affects the mapped stack traces. They all start with:
Error: check the output traces at createTraces (D:\PR\ts-node-bug-repro\commonjs.cts:2:15) at file:///D:/PR/ts-node-bug-repro/esm.mts:3:1
Not posting code for this simple test so as not to waste everyone's time.
Let's check for other possible side effects another day.Thanks, I will also check our existing tests. They may already be checking for this in some way, and I see both of your pull requests have passing tests on CI.
I attempted the same test you did #1769 (comment)
and got the same result: it seems that paths/URLs in stack traces are correct with #1771.Since URLs in sourcemaps seems more correct/more sane/more robust than using basename, and since it doesn't seem to be breaking anything, I'm going to merge that one.
Reacted by 华- added a commit that references this issue
on Jun 17, 2022 @cspotcode Before we can test the impacts, could the file url/base name solution be made into an option? This problem is critical and easily reproducible when we use c8 with
--exclude-after-remapon Windows, where c8 excludes every file that was processed by ts-node and shows empty results, which is caused by that the mapped paths do not matches the including paths (the case of the disk letter is different). A new option wouldn't be breaking.
Search Terms
v8 coverage, coverage, source map windows
Expected Behavior
The
sourcesfield of the generated source map cache consists of file URLs (e.g.,file:///D:/Example/index.ts).Actual Behavior
Some generated
sourcesfield contain malformed file paths (e.g.,d:/Example/index.ts).I think it "malformed" because the disk letter, along with
:, is treated like a protocol. At least, it should be in upper case. It will be better if it follows thefile:///prefix like the others.Steps to reproduce the problem
NODE_V8_COVERAGE.tsfile withts-nodesourcesfields in each produced coverage output file with each other.Minimal reproduction
https://github.com/PaperStrike/ts-node-bug-repro
The workflow does basically the steps above, and prints the malformed URLs.
Specifications