Skip to content

Commit 1f04fa4

Browse files
committed
fix absolute windows paths in source maps
1 parent 9ca03f6 commit 1f04fa4

File tree

3 files changed

+72
-20
lines changed

3 files changed

+72
-20
lines changed

internal/bundler/bundler.go

+26-1
Original file line numberDiff line numberDiff line change
@@ -642,8 +642,33 @@ func parseFile(args parseArgs) {
642642
sourceMap.SourcesContent = slice
643643
}
644644

645-
// Attempt to fill in null entries using the file system
646645
for i, source := range sourceMap.Sources {
646+
// Convert absolute paths to "file://" URLs, which is especially important
647+
// for Windows where file paths don't look like URLs at all (they use "\"
648+
// as a path separator and start with a "C:\" volume label instead of "/").
649+
//
650+
// The new source map specification (https://tc39.es/ecma426/) says that
651+
// each source is "a string that is a (potentially relative) URL". So we
652+
// should technically not be finding absolute paths here in the first place.
653+
//
654+
// However, for a long time source maps was poorly-specified. The old source
655+
// map specification (https://sourcemaps.info/spec.html) only says "sources"
656+
// is "a list of original sources used by the mappings entry" which could
657+
// be anything, really.
658+
//
659+
// So it makes sense that software which predates the formal specification
660+
// of source maps might fill in the sources array with absolute file paths
661+
// instead of URLs. Here are some cases where that happened:
662+
//
663+
// - https://github.com/mozilla/source-map/issues/355
664+
// - https://github.com/webpack/webpack/issues/8226
665+
//
666+
if path.Namespace == "file" && args.fs.IsAbs(source) {
667+
source = helpers.FileURLFromFilePath(source).String()
668+
sourceMap.Sources[i] = source
669+
}
670+
671+
// Attempt to fill in null entries using the file system
647672
if sourceMap.SourcesContent[i].Value == nil {
648673
if sourceURL, err := url.Parse(source); err == nil && helpers.IsFileURL(sourceURL) {
649674
if contents, err, _ := args.caches.FSCache.ReadFile(args.fs, helpers.FilePathFromFileURL(args.fs, sourceURL)); err == nil {

internal/fs/fs_mock.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (fs *mockFS) ModKey(path string) (ModKey, error) {
116116
}
117117

118118
func win2unix(p string) string {
119-
if strings.HasPrefix(p, "C:\\") {
119+
if strings.HasPrefix(p, "C:\\") || strings.HasPrefix(p, "c:\\") {
120120
p = p[2:]
121121
}
122122
p = strings.ReplaceAll(p, "\\", "/")

scripts/verify-source-map.js

+45-18
Original file line numberDiff line numberDiff line change
@@ -573,41 +573,68 @@ const toSearchNestedFoldersIssue4070 = {
573573
'bar': 'src/app/app.config.js',
574574
}
575575

576+
// This test checks what happens when you use absolute paths in inlined source
577+
// maps. This is done two ways, first using a "file://" URL and second using
578+
// an actual absolute path.
579+
//
580+
// Only the first way is supposed to be valid, at least according to the formal
581+
// specification (https://tc39.es/ecma426/) which says that each source is "a
582+
// string that is a (potentially relative) URL".
583+
//
584+
// However, for a long time source maps was poorly-specified. The old source map
585+
// specification (https://sourcemaps.info/spec.html) only says "sources" is "a
586+
// list of original sources used by the mappings entry".
587+
//
588+
// So it makes sense that software which predates the formal specification of
589+
// source maps might fill in the sources array with absolute file paths instead
590+
// of URLs. So we test for that here to make sure esbuild works either way.
591+
//
592+
// Windows paths make this complicated. Here are all five possible combinations
593+
// of absolute paths for the file "folder/file.js":
594+
//
595+
// - Unix URL: "file:///folder/file.js"
596+
// - Unix path: "/folder/file.js"
597+
// - Windows URL: "file:///C:/folder/file.js"
598+
// - Windows path v1: "C:/folder/file.js" (not covered here)
599+
// - Windows path v2: "C:\folder\file.js"
600+
//
601+
const rootDir = path.dirname(process.cwd().split(path.sep).slice(0, 2).join(path.sep))
602+
const pathIssue4075 = path.join(rootDir, 'out', 'src', 'styles')
603+
const urlIssue4075 = url.pathToFileURL(pathIssue4075)
604+
const urlIssue4075Encoded = encodeURIComponent(JSON.stringify(urlIssue4075 + '1.scss'))
605+
const pathIssue4075Encoded = encodeURIComponent(JSON.stringify(pathIssue4075 + '2.scss'))
576606
const testCaseAbsolutePathIssue4075 = {
577607
'entry.css': `
578-
@import "./styles.css";
608+
@import "./styles1.css";
579609
@import "./styles2.css";
580610
`,
581-
'styles.css': `/* You can add global styles to this file, and also import other style files */
611+
'styles1.css': `/* You can add global styles to this file, and also import other style files */
582612
* {
583613
content: "foo";
584614
}
585615
586616
/*# sourceMappingURL=data:application/json;charset=utf-8,%7B%22version%22:3,` +
587-
`%22sourceRoot%22:%22%22,%22sources%22:%5B%22file:///out/src/styles.scss` +
588-
`%22%5D,%22names%22:%5B%5D,%22mappings%22:%22AAAA;AACA;EACE,SAAS%22,%22f` +
589-
`ile%22:%22out%22,%22sourcesContent%22:%5B%22/*%20You%20can%20add%20glob` +
590-
`al%20styles%20to%20this%20file,%20and%20also%20import%20other%20style%2` +
591-
`0files%20%2A/%5Cn*%20%7B%5Cn%20%20content:%20%5C%22foo%5C%22%5Cn%7D%5Cn` +
592-
`%22%5D%7D */`,
617+
`%22sourceRoot%22:%22%22,%22sources%22:%5B${urlIssue4075Encoded}%5D,%22n` +
618+
`ames%22:%5B%5D,%22mappings%22:%22AAAA;AACA;EACE,SAAS%22,%22file%22:%22o` +
619+
`ut%22,%22sourcesContent%22:%5B%22/*%20You%20can%20add%20global%20styles` +
620+
`%20to%20this%20file,%20and%20also%20import%20other%20style%20files%20%2` +
621+
`A/%5Cn*%20%7B%5Cn%20%20content:%20%5C%22foo%5C%22%5Cn%7D%5Cn%22%5D%7D */`,
593622
'styles2.css': `/* You can add global styles to this file, and also import other style files */
594623
* {
595624
content: "bar";
596625
}
597626
598627
/*# sourceMappingURL=data:application/json;charset=utf-8,%7B%22version%22:3,` +
599-
`%22sourceRoot%22:%22%22,%22sources%22:%5B%22/out/src/styles2.scss%22%5D` +
600-
`,%22names%22:%5B%5D,%22mappings%22:%22AAAA;AACA;EACE,SAAS%22,%22file%22` +
601-
`:%22out%22,%22sourcesContent%22:%5B%22/*%20You%20can%20add%20global%20s` +
602-
`tyles%20to%20this%20file,%20and%20also%20import%20other%20style%20files` +
603-
`%20%2A/%5Cn*%20%7B%5Cn%20%20content:%20%5C%22bar%5C%22%5Cn%7D%5Cn%22%5D` +
604-
`%7D */
605-
`,
628+
`%22sourceRoot%22:%22%22,%22sources%22:%5B${pathIssue4075Encoded}%5D,%22` +
629+
`names%22:%5B%5D,%22mappings%22:%22AAAA;AACA;EACE,SAAS%22,%22file%22:%22` +
630+
`out%22,%22sourcesContent%22:%5B%22/*%20You%20can%20add%20global%20style` +
631+
`s%20to%20this%20file,%20and%20also%20import%20other%20style%20files%20%` +
632+
`2A/%5Cn*%20%7B%5Cn%20%20content:%20%5C%22bar%5C%22%5Cn%7D%5Cn%22%5D%7D */`,
606633
}
607634

608635
const toSearchAbsolutePathIssue4075 = {
609-
foo: path.relative(path.join(testDir, '(this test)'), '/out/src/styles.scss'),
610-
bar: path.relative(path.join(testDir, '(this test)'), '/out/src/styles2.scss'),
636+
foo: path.relative(path.join(testDir, '(this test)'), pathIssue4075 + '1.scss').replaceAll('\\', '/'),
637+
bar: path.relative(path.join(testDir, '(this test)'), pathIssue4075 + '2.scss').replaceAll('\\', '/'),
611638
}
612639

613640
const testCaseMissingSourcesIssue4104 = {
@@ -1190,7 +1217,7 @@ async function main() {
11901217
entryPoints: ['entry1.ts', 'entry2.ts'],
11911218
crlf,
11921219
checkFirstChunk: true,
1193-
})
1220+
}),
11941221
)
11951222
}
11961223
}

0 commit comments

Comments
 (0)