From a94f94ffa54e8b2ebcada883b14a375f28787da0 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Wed, 29 Jul 2020 00:19:18 -0700 Subject: [PATCH] chunk names now hash contents instead of path --- internal/bundler/bundler_splitting_test.go | 66 +++++------ internal/bundler/linker.go | 130 ++++++++++++++------- scripts/js-api-tests.js | 16 +-- 3 files changed, 127 insertions(+), 85 deletions(-) diff --git a/internal/bundler/bundler_splitting_test.go b/internal/bundler/bundler_splitting_test.go index 1def4b6e8a6..b42e16cca30 100644 --- a/internal/bundler/bundler_splitting_test.go +++ b/internal/bundler/bundler_splitting_test.go @@ -29,19 +29,19 @@ func TestSplittingSharedES6IntoES6(t *testing.T) { expected: map[string]string{ "/out/a.js": `import { foo -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.iJkFSV6U.js"; // /a.js console.log(foo); `, "/out/b.js": `import { foo -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.iJkFSV6U.js"; // /b.js console.log(foo); `, - "/out/chunk.xL6KqlYO.js": `// /shared.js + "/out/chunk.iJkFSV6U.js": `// /shared.js let foo = 123; export { @@ -75,7 +75,7 @@ func TestSplittingSharedCommonJSIntoES6(t *testing.T) { expected: map[string]string{ "/out/a.js": `import { require_shared -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.W46Rb0pk.js"; // /a.js const {foo} = require_shared(); @@ -83,13 +83,13 @@ console.log(foo); `, "/out/b.js": `import { require_shared -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.W46Rb0pk.js"; // /b.js const {foo: foo2} = require_shared(); console.log(foo2); `, - "/out/chunk.xL6KqlYO.js": `// /shared.js + "/out/chunk.W46Rb0pk.js": `// /shared.js var require_shared = __commonJS((exports) => { exports.foo = 123; }); @@ -185,21 +185,21 @@ func TestSplittingDynamicAndNotDynamicES6IntoES6(t *testing.T) { expected: map[string]string{ "/out/entry.js": `import { bar -} from "./chunk.-fk8OGuR.js"; +} from "./chunk.nvw10ONy.js"; // /entry.js import("./foo.js").then(({bar: b}) => console.log(bar, b)); `, "/out/foo.js": `import { bar -} from "./chunk.-fk8OGuR.js"; +} from "./chunk.nvw10ONy.js"; // /foo.js export { bar }; `, - "/out/chunk.-fk8OGuR.js": `// /foo.js + "/out/chunk.nvw10ONy.js": `// /foo.js let bar = 123; export { @@ -231,7 +231,7 @@ func TestSplittingDynamicAndNotDynamicCommonJSIntoES6(t *testing.T) { expected: map[string]string{ "/out/entry.js": `import { require_foo -} from "./chunk.-fk8OGuR.js"; +} from "./chunk.K9Rnzqz2.js"; // /entry.js const foo = __toModule(require_foo()); @@ -239,12 +239,12 @@ import("./foo.js").then(({default: {bar: b}}) => console.log(foo.bar, b)); `, "/out/foo.js": `import { require_foo -} from "./chunk.-fk8OGuR.js"; +} from "./chunk.K9Rnzqz2.js"; // /foo.js export default require_foo(); `, - "/out/chunk.-fk8OGuR.js": `// /foo.js + "/out/chunk.K9Rnzqz2.js": `// /foo.js var require_foo = __commonJS((exports) => { exports.bar = 123; }); @@ -287,7 +287,7 @@ func TestSplittingAssignToLocal(t *testing.T) { "/out/a.js": `import { foo, setFoo -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.8-s4RjmR.js"; // /a.js setFoo(123); @@ -295,12 +295,12 @@ console.log(foo); `, "/out/b.js": `import { foo -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.8-s4RjmR.js"; // /b.js console.log(foo); `, - "/out/chunk.xL6KqlYO.js": `// /shared.js + "/out/chunk.8-s4RjmR.js": `// /shared.js let foo; function setFoo(value) { foo = value; @@ -340,7 +340,7 @@ func TestSplittingSideEffectsWithoutDependencies(t *testing.T) { AbsOutputDir: "/out", }, expected: map[string]string{ - "/out/a.js": `import "./chunk.xL6KqlYO.js"; + "/out/a.js": `import "./chunk.Wqqj8fqN.js"; // /shared.js let a = 1; @@ -348,7 +348,7 @@ let a = 1; // /a.js console.log(a); `, - "/out/b.js": `import "./chunk.xL6KqlYO.js"; + "/out/b.js": `import "./chunk.Wqqj8fqN.js"; // /shared.js let b = 2; @@ -356,7 +356,7 @@ let b = 2; // /b.js console.log(b); `, - "/out/chunk.xL6KqlYO.js": `// /shared.js + "/out/chunk.Wqqj8fqN.js": `// /shared.js console.log("side effect"); `, }, @@ -391,19 +391,19 @@ func TestSplittingNestedDirectories(t *testing.T) { expected: map[string]string{ "/Users/user/project/out/pageA/page.js": `import { shared_default -} from "../chunk.UcWke4C2.js"; +} from "../chunk.Y98gPOMH.js"; // /Users/user/project/src/pages/pageA/page.js console.log(shared_default); `, "/Users/user/project/out/pageB/page.js": `import { shared_default -} from "../chunk.UcWke4C2.js"; +} from "../chunk.Y98gPOMH.js"; // /Users/user/project/src/pages/pageB/page.js console.log(-shared_default); `, - "/Users/user/project/out/chunk.UcWke4C2.js": `// /Users/user/project/src/pages/shared.js + "/Users/user/project/out/chunk.Y98gPOMH.js": `// /Users/user/project/src/pages/shared.js var shared_default = 123; export { @@ -437,7 +437,7 @@ func TestSplittingCircularReferenceIssue251(t *testing.T) { "/out/a.js": `import { p, q -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.4ySCMXof.js"; // /a.js export { @@ -448,7 +448,7 @@ export { "/out/b.js": `import { p, q -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.4ySCMXof.js"; // /b.js export { @@ -456,7 +456,7 @@ export { q }; `, - "/out/chunk.xL6KqlYO.js": `// /b.js + "/out/chunk.4ySCMXof.js": `// /b.js var q = 6; // /a.js @@ -500,7 +500,7 @@ func TestSplittingMissingLazyExport(t *testing.T) { AbsOutputDir: "/out", }, expected: map[string]string{ - "/out/a.js": `import "./chunk.xL6KqlYO.js"; + "/out/a.js": `import "./chunk.nsO65Sdr.js"; // /empty.js const empty_exports = {}; @@ -513,7 +513,7 @@ function foo() { // /a.js console.log(foo()); `, - "/out/b.js": `import "./chunk.xL6KqlYO.js"; + "/out/b.js": `import "./chunk.nsO65Sdr.js"; // /common.js function bar() { @@ -523,7 +523,7 @@ function bar() { // /b.js console.log(bar()); `, - "/out/chunk.xL6KqlYO.js": `// /empty.js + "/out/chunk.nsO65Sdr.js": `// /empty.js // /common.js `, @@ -551,7 +551,7 @@ func TestSplittingReExportIssue273(t *testing.T) { expected: map[string]string{ "/out/a.js": `import { a -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.Ldjbzhqa.js"; // /a.js export { @@ -560,14 +560,14 @@ export { `, "/out/b.js": `import { a -} from "./chunk.xL6KqlYO.js"; +} from "./chunk.Ldjbzhqa.js"; // /b.js export { a }; `, - "/out/chunk.xL6KqlYO.js": `// /a.js + "/out/chunk.Ldjbzhqa.js": `// /a.js const a = 1; export { @@ -679,16 +679,16 @@ func TestSplittingCrossChunkAssignmentDependencies(t *testing.T) { expected: map[string]string{ "/out/a.js": `import { setValue -} from "./chunk.xL6KqlYO.js"; +} from "./chunk._9Zb2gmb.js"; // /a.js setValue(123); `, - "/out/b.js": `import "./chunk.xL6KqlYO.js"; + "/out/b.js": `import "./chunk._9Zb2gmb.js"; // /b.js `, - "/out/chunk.xL6KqlYO.js": `// /shared.js + "/out/chunk._9Zb2gmb.js": `// /shared.js var observer; var value; function getValue() { diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index d3dedfef6c0..31e6a6f250f 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -480,51 +480,76 @@ func (c *linkerContext) link() []OutputFile { } func (c *linkerContext) generateChunksInParallel(chunks []chunkMeta) []OutputFile { - // Fill in chunk names - for i, chunk := range chunks { - if chunk.baseNameOrEmpty != "" { - continue + // We want to process chunks with as much parallelism as possible. However, + // content hashing means chunks that import other chunks must be completed + // after the imported chunks are completed because the import paths contain + // the content hash. It's only safe to process a chunk when the dependency + // count reaches zero. + type ordering struct { + dependencies sync.WaitGroup + dependents []uint32 + } + chunkOrdering := make([]ordering, len(chunks)) + for chunkIndex, chunk := range chunks { + chunkOrdering[chunkIndex].dependencies.Add(len(chunk.crossChunkImports)) + for _, otherChunkIndex := range chunk.crossChunkImports { + dependents := &chunkOrdering[otherChunkIndex].dependents + *dependents = append(*dependents, uint32(chunkIndex)) } - dataForHash := "" - count := 0 - for i, entryPoint := range c.entryPoints { - if chunk.entryBits.hasBit(uint(i)) { - if count > 0 { - dataForHash += "\x00" - } - dataForHash += c.fileMeta[entryPoint].entryPointRelPath - count++ + } + + // Check for loops in the dependency graph since they cause a deadlock + var check func(int, []int) + check = func(chunkIndex int, path []int) { + for _, otherChunkIndex := range path { + if chunkIndex == otherChunkIndex { + panic("Internal error: Chunk import graph contains a cycle") } } - if count < 2 { - panic("Internal error") + path = append(path, chunkIndex) + for _, otherChunkIndex := range chunks[chunkIndex].crossChunkImports { + check(int(otherChunkIndex), path) } - bytes := []byte(lowerCaseAbsPathForWindows(dataForHash)) - hashBytes := sha1.Sum(bytes) - hash := base64.URLEncoding.EncodeToString(hashBytes[:])[:8] - chunks[i].baseNameOrEmpty = "chunk." + hash + c.options.OutputExtensionFor(".js") + } + for i := range chunks { + check(i, nil) } - // Generate chunks in parallel results := make([][]OutputFile, len(chunks)) - waitGroup := sync.WaitGroup{} - waitGroup.Add(len(chunks)) - for i, chunk := range chunks { - go func(i int, chunk chunkMeta) { + resultsWaitGroup := sync.WaitGroup{} + resultsWaitGroup.Add(len(chunks)) + + // Generate each chunk on a separate goroutine + for i := range chunks { + go func(i int) { + chunk := &chunks[i] + order := &chunkOrdering[i] + + // Wait for all dependencies to be resolved first + order.dependencies.Wait() + + // Fill in the cross-chunk import records now that the paths are known crossChunkImportRecords := make([]ast.ImportRecord, len(chunk.crossChunkImports)) for i, otherChunkIndex := range chunk.crossChunkImports { crossChunkImportRecords[i] = ast.ImportRecord{ Kind: ast.ImportStmt, - Path: ast.Path{Text: c.relativePathBetweenChunks(&chunk, chunks[otherChunkIndex].relPath())}, + Path: ast.Path{Text: c.relativePathBetweenChunks(chunk.relDir, chunks[otherChunkIndex].relPath())}, } } + + // Generate the chunk results[i] = c.generateChunk(chunk, crossChunkImportRecords) - waitGroup.Done() - }(i, chunk) + + // Wake up any dependents now that we're done + for _, chunkIndex := range order.dependents { + chunkOrdering[chunkIndex].dependencies.Done() + } + resultsWaitGroup.Done() + }(i) } - waitGroup.Wait() // Join the results in chunk order for determinism + resultsWaitGroup.Wait() var outputFiles []OutputFile for _, group := range results { outputFiles = append(outputFiles, group...) @@ -532,11 +557,11 @@ func (c *linkerContext) generateChunksInParallel(chunks []chunkMeta) []OutputFil return outputFiles } -func (c *linkerContext) relativePathBetweenChunks(fromChunk *chunkMeta, toRelPath string) string { - relPath, ok := c.fs.Rel(fromChunk.relDir, toRelPath) +func (c *linkerContext) relativePathBetweenChunks(fromRelDir string, toRelPath string) string { + relPath, ok := c.fs.Rel(fromRelDir, toRelPath) if !ok { c.log.AddError(nil, ast.Loc{}, - fmt.Sprintf("Cannot traverse from directory %q to chunk %q", fromChunk.relDir, toRelPath)) + fmt.Sprintf("Cannot traverse from directory %q to chunk %q", fromRelDir, toRelPath)) return "" } @@ -588,7 +613,7 @@ func (c *linkerContext) computeCrossChunkDependencies(chunks []chunkMeta) { for _, importRecordIndex := range part.ImportRecordIndices { record := &file.ast.ImportRecords[importRecordIndex] if record.SourceIndex != nil && c.isExternalDynamicImport(record) { - record.Path.Text = c.relativePathBetweenChunks(&chunk, c.fileMeta[*record.SourceIndex].entryPointRelPath) + record.Path.Text = c.relativePathBetweenChunks(chunk.relDir, c.fileMeta[*record.SourceIndex].entryPointRelPath) record.SourceIndex = nil } } @@ -2232,7 +2257,7 @@ func (a chunkOrderArray) Less(i int, j int) bool { return a[i].distance < a[j].distance || (a[i].distance == a[j].distance && a[i].path.ComesBeforeInSortedOrder(a[j].path)) } -func (c *linkerContext) chunkFileOrder(chunk chunkMeta) []uint32 { +func (c *linkerContext) chunkFileOrder(chunk *chunkMeta) []uint32 { sorted := make(chunkOrderArray, 0, len(chunk.filesWithPartsInChunk)) // Attach information to the files for use with sorting @@ -2735,7 +2760,7 @@ func (c *linkerContext) generateCodeForFileInChunk( waitGroup.Done() } -func (c *linkerContext) generateChunk(chunk chunkMeta, crossChunkImportRecords []ast.ImportRecord) (results []OutputFile) { +func (c *linkerContext) generateChunk(chunk *chunkMeta, crossChunkImportRecords []ast.ImportRecord) (results []OutputFile) { filesInChunkInOrder := c.chunkFileOrder(chunk) compileResults := make([]compileResult, 0, len(filesInChunkInOrder)) runtimeMembers := c.files[runtime.SourceIndex].ast.ModuleScope.Members @@ -2863,8 +2888,7 @@ func (c *linkerContext) generateChunk(chunk chunkMeta, crossChunkImportRecords [ } else { jMeta.AddString(",") } - chunkAbsPath := c.fs.Join(c.options.AbsOutputDir, chunk.relPath()) - importAbsPath := c.fs.Join(c.fs.Dir(chunkAbsPath), record.Path.Text) + importAbsPath := c.fs.Join(c.options.AbsOutputDir, chunk.relDir, record.Path.Text) jMeta.AddString(fmt.Sprintf("\n {\n \"path\": %s\n }", printer.QuoteForJSON(c.res.PrettyPath(importAbsPath)))) } @@ -2982,8 +3006,6 @@ func (c *linkerContext) generateChunk(chunk chunkMeta, crossChunkImportRecords [ j.AddString("\n") } - jsAbsPath := c.fs.Join(c.options.AbsOutputDir, chunk.relPath()) - if c.options.SourceMap != config.SourceMapNone { sourceMap := c.generateSourceMapForChunk(compileResultsForSourceMap) @@ -3002,23 +3024,41 @@ func (c *linkerContext) generateChunk(chunk chunkMeta, crossChunkImportRecords [ "{\n \"imports\": [],\n \"inputs\": {},\n \"bytes\": %d\n }", len(sourceMap))) } - results = append(results, OutputFile{ - AbsPath: jsAbsPath + ".map", - Contents: sourceMap, - jsonMetadataChunk: jsonMetadataChunk, - }) + // Figure out the base name for the source map which may include the content hash + var sourceMapBaseName string + if chunk.baseNameOrEmpty == "" { + hashBytes := sha1.Sum(sourceMap) + hash := base64.URLEncoding.EncodeToString(hashBytes[:])[:8] + sourceMapBaseName = "chunk." + hash + c.options.OutputExtensionFor(".js") + ".map" + } else { + sourceMapBaseName = chunk.baseNameOrEmpty + ".map" + } // Add a comment linking the source to its map if c.options.SourceMap == config.SourceMapLinkedWithComment { j.AddString("//# sourceMappingURL=") - j.AddString(c.fs.Base(jsAbsPath + ".map")) + j.AddString(sourceMapBaseName) j.AddString("\n") } + + results = append(results, OutputFile{ + AbsPath: c.fs.Join(c.options.AbsOutputDir, chunk.relDir, sourceMapBaseName), + Contents: sourceMap, + jsonMetadataChunk: jsonMetadataChunk, + }) } } + // The JavaScript contents are done now that the source map comment is in jsContents := j.Done() + // Figure out the base name for this chunk now that the content hash is known + if chunk.baseNameOrEmpty == "" { + hashBytes := sha1.Sum(jsContents) + hash := base64.URLEncoding.EncodeToString(hashBytes[:])[:8] + chunk.baseNameOrEmpty = "chunk." + hash + c.options.OutputExtensionFor(".js") + } + // End the metadata var jsonMetadataChunk []byte if c.options.AbsMetadataFile != "" { @@ -3030,7 +3070,7 @@ func (c *linkerContext) generateChunk(chunk chunkMeta, crossChunkImportRecords [ } results = append(results, OutputFile{ - AbsPath: jsAbsPath, + AbsPath: c.fs.Join(c.options.AbsOutputDir, chunk.relPath()), Contents: jsContents, jsonMetadataChunk: jsonMetadataChunk, }) diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index 865545e1768..065e7e38776 100644 --- a/scripts/js-api-tests.js +++ b/scripts/js-api-tests.js @@ -180,7 +180,7 @@ let buildTests = { const makePath = basename => path.relative(cwd, path.join(outdir, basename)).split(path.sep).join('/') // Check outputs - const chunk = 'chunk.3YfO7hrU.js'; + const chunk = 'chunk.zIqPdeGN.js'; assert.deepStrictEqual(json.outputs[makePath(path.basename(entry1))].imports, [{ path: makePath(chunk) }]) assert.deepStrictEqual(json.outputs[makePath(path.basename(entry2))].imports, [{ path: makePath(chunk) }]) assert.deepStrictEqual(json.outputs[makePath(chunk)].imports, []) @@ -232,16 +232,17 @@ let buildTests = { assert.strictEqual(value.outputFiles.length, 3) // These should all use forward slashes, even on Windows + const chunk = 'chunk.ELHpQs8z.js' assert.strictEqual(Buffer.from(value.outputFiles[0].contents).toString(), `import { common_default -} from "./chunk.xL6KqlYO.js"; +} from "./${chunk}"; // scripts/.js-api-tests/splittingRelativeSameDir/a.js console.log("a" + common_default); `) assert.strictEqual(Buffer.from(value.outputFiles[1].contents).toString(), `import { common_default -} from "./chunk.xL6KqlYO.js"; +} from "./${chunk}"; // scripts/.js-api-tests/splittingRelativeSameDir/b.js console.log("b" + common_default); @@ -256,7 +257,7 @@ export { assert.strictEqual(value.outputFiles[0].path, path.join(outdir, path.basename(inputA))) assert.strictEqual(value.outputFiles[1].path, path.join(outdir, path.basename(inputB))) - assert.strictEqual(value.outputFiles[2].path, path.join(outdir, 'chunk.xL6KqlYO.js')) + assert.strictEqual(value.outputFiles[2].path, path.join(outdir, chunk)) }, async splittingRelativeNestedDir({ esbuild, testDir }) { @@ -281,16 +282,17 @@ export { assert.strictEqual(value.outputFiles.length, 3) // These should all use forward slashes, even on Windows + const chunk = 'chunk.7QUqUGms.js' assert.strictEqual(Buffer.from(value.outputFiles[0].contents).toString(), `import { common_default -} from "../chunk.uWHCLtU_.js"; +} from "../${chunk}"; // scripts/.js-api-tests/splittingRelativeNestedDir/a/demo.js console.log("a" + common_default); `) assert.strictEqual(Buffer.from(value.outputFiles[1].contents).toString(), `import { common_default -} from "../chunk.uWHCLtU_.js"; +} from "../${chunk}"; // scripts/.js-api-tests/splittingRelativeNestedDir/b/demo.js console.log("b" + common_default); @@ -305,7 +307,7 @@ export { assert.strictEqual(value.outputFiles[0].path, path.join(outdir, path.relative(testDir, inputA))) assert.strictEqual(value.outputFiles[1].path, path.join(outdir, path.relative(testDir, inputB))) - assert.strictEqual(value.outputFiles[2].path, path.join(outdir, 'chunk.uWHCLtU_.js')) + assert.strictEqual(value.outputFiles[2].path, path.join(outdir, chunk)) }, async stdinStdoutBundle({ esbuild, testDir }) {