fix #2071: remap bytesInOutput for substitutions

evanw · evanw · commit a375b372f5a4 · 2022-12-14T14:34:27.000-05:00
closes #2091
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,10 @@
      }
     ```
 
+* Fix byte counts in metafile regarding references to other output files ([#2071](https://github.com/evanw/esbuild/issues/2071))
+
+    Previously files that contained references to other output files had slightly incorrect metadata for the byte counts of input files which contributed to that output file. So for example if `app.js` imports `image.png` using the file loader and esbuild generates `out.js` and `image-LSAMBFUD.png`, the metadata for how many bytes of `out.js` are from `app.js` was slightly off (the metadata for the byte count of `out.js` was still correct). The reason is because esbuild substitutes the final paths for references between output files toward the end of the build to handle cyclic references, and the byte counts needed to be adjusted as well during the path substitution. This release fixes these byte counts (specifically the `bytesInOutput` values).
+
 * The alias feature now strips a trailing slash ([#2730](https://github.com/evanw/esbuild/issues/2730))
 
     People sometimes add a trailing slash to the name of one of node's built-in modules to force node to import from the file system instead of importing the built-in module. For example, importing `util` imports node's built-in module called `util` but importing `util/` tries to find a package called `util` on the file system. Previously attempting to use esbuild's package alias feature to replace imports to `util` with a specific file would fail because the file path would also gain a trailing slash (e.g. mapping `util` to `./file.js` turned `util/` into `./file.js/`). With this release, esbuild will now omit the path suffix if it's a single trailing slash, which should now allow you to successfully apply aliases to these import paths.
diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go
@@ -614,7 +614,7 @@ func (c *linkerContext) generateChunksInParallel(additionalFiles []graph.OutputF
 			// Path substitution for the JSON metadata
 			var jsonMetadataChunk string
 			if c.options.NeedsMetafile {
-				jsonMetadataChunkPieces := c.breakOutputIntoPieces(chunk.jsonMetadataChunkCallback(len(outputContents)))
+				jsonMetadataChunkPieces := c.breakJoinerIntoPieces(chunk.jsonMetadataChunkCallback(len(outputContents)))
 				jsonMetadataChunkBytes, _ := c.substituteFinalPaths(jsonMetadataChunkPieces, func(finalRelPathForImport string) string {
 					return c.res.PrettyPath(logger.Path{Text: c.fs.Join(c.options.AbsOutputDir, finalRelPathForImport), Namespace: "file"})
 				})
@@ -704,6 +704,37 @@ func (c *linkerContext) substituteFinalPaths(
 	return
 }
 
+func (c *linkerContext) accurateFinalByteCount(output intermediateOutput, chunkFinalRelDir string) int {
+	count := 0
+
+	// Note: The paths generated here must match "substituteFinalPaths" above
+	for _, piece := range output.pieces {
+		count += len(piece.data)
+
+		switch piece.kind {
+		case outputPieceAssetIndex:
+			file := c.graph.Files[piece.index]
+			if len(file.InputFile.AdditionalFiles) != 1 {
+				panic("Internal error")
+			}
+			relPath, _ := c.fs.Rel(c.options.AbsOutputDir, file.InputFile.AdditionalFiles[0].AbsPath)
+
+			// Make sure to always use forward slashes, even on Windows
+			relPath = strings.ReplaceAll(relPath, "\\", "/")
+
+			importPath := c.pathBetweenChunks(chunkFinalRelDir, relPath)
+			count += len(importPath)
+
+		case outputPieceChunkIndex:
+			chunk := c.chunks[piece.index]
+			importPath := c.pathBetweenChunks(chunkFinalRelDir, chunk.finalRelPath)
+			count += len(importPath)
+		}
+	}
+
+	return count
+}
+
 func (c *linkerContext) pathBetweenChunks(fromRelDir string, toRelPath string) string {
 	// Join with the public path if it has been configured
 	if c.options.PublicPath != "" {
@@ -5036,12 +5067,12 @@ func (c *linkerContext) generateChunkJS(chunkIndex int, chunkWaitGroup *sync.Wai
 	var compileResultsForSourceMap []compileResultForSourceMap
 	var legalCommentList []string
 	var metaOrder []uint32
-	var metaByteCount map[string]int
+	var metaBytes map[uint32][][]byte
 	legalCommentSet := make(map[string]bool)
 	prevFileNameComment := uint32(0)
 	if c.options.NeedsMetafile {
 		metaOrder = make([]uint32, 0, len(compileResults))
-		metaByteCount = make(map[string]int, len(compileResults))
+		metaBytes = make(map[uint32][][]byte, len(compileResults))
 	}
 	for _, compileResult := range compileResults {
 		isRuntime := compileResult.sourceIndex == runtime.SourceIndex
@@ -5104,13 +5135,11 @@ func (c *linkerContext) generateChunkJS(chunkIndex int, chunkWaitGroup *sync.Wai
 			// Include this file in the metadata
 			if c.options.NeedsMetafile {
 				// Accumulate file sizes since a given file may be split into multiple parts
-				path := c.graph.Files[compileResult.sourceIndex].InputFile.Source.PrettyPath
-				if count, ok := metaByteCount[path]; ok {
-					metaByteCount[path] = count + len(compileResult.JS)
-				} else {
+				bytes, ok := metaBytes[compileResult.sourceIndex]
+				if !ok {
 					metaOrder = append(metaOrder, compileResult.sourceIndex)
-					metaByteCount[path] = len(compileResult.JS)
 				}
+				metaBytes[compileResult.sourceIndex] = append(bytes, compileResult.JS)
 			}
 		}
 
@@ -5148,7 +5177,7 @@ func (c *linkerContext) generateChunkJS(chunkIndex int, chunkWaitGroup *sync.Wai
 	}
 
 	// The JavaScript contents are done now that the source map comment is in
-	chunk.intermediateOutput = c.breakOutputIntoPieces(j)
+	chunk.intermediateOutput = c.breakJoinerIntoPieces(j)
 	timer.End("Join JavaScript files")
 
 	if c.options.SourceMap != config.SourceMapNone {
@@ -5161,20 +5190,30 @@ func (c *linkerContext) generateChunkJS(chunkIndex int, chunkWaitGroup *sync.Wai
 	// End the metadata lazily. The final output size is not known until the
 	// final import paths are substituted into the output pieces generated below.
 	if c.options.NeedsMetafile {
+		pieces := make([][]intermediateOutput, len(metaOrder))
+		for i, sourceIndex := range metaOrder {
+			slices := metaBytes[sourceIndex]
+			outputs := make([]intermediateOutput, len(slices))
+			for j, slice := range slices {
+				outputs[j] = c.breakOutputIntoPieces(slice)
+			}
+			pieces[i] = outputs
+		}
 		chunk.jsonMetadataChunkCallback = func(finalOutputSize int) helpers.Joiner {
-			isFirstMeta := true
-			for _, sourceIndex := range metaOrder {
-				if isFirstMeta {
-					isFirstMeta = false
-				} else {
+			finalRelDir := c.fs.Dir(chunk.finalRelPath)
+			for i, sourceIndex := range metaOrder {
+				if i > 0 {
 					jMeta.AddString(",")
 				}
-				path := c.graph.Files[sourceIndex].InputFile.Source.PrettyPath
-				extra := c.generateExtraDataForFileJS(sourceIndex)
-				jMeta.AddString(fmt.Sprintf("\n        %s: {\n          \"bytesInOutput\": %d\n        %s}",
-					helpers.QuoteForJSON(path, c.options.ASCIIOnly), metaByteCount[path], extra))
+				count := 0
+				for _, output := range pieces[i] {
+					count += c.accurateFinalByteCount(output, finalRelDir)
+				}
+				jMeta.AddString(fmt.Sprintf("\n        %s: {\n          \"bytesInOutput\": %d\n        }",
+					helpers.QuoteForJSON(c.graph.Files[sourceIndex].InputFile.Source.PrettyPath, c.options.ASCIIOnly),
+					count))
 			}
-			if !isFirstMeta {
+			if len(metaOrder) > 0 {
 				jMeta.AddString("\n      ")
 			}
 			jMeta.AddString(fmt.Sprintf("},\n      \"bytes\": %d\n    }", finalOutputSize))
@@ -5461,7 +5500,6 @@ func (c *linkerContext) generateChunkCSS(chunkIndex int, chunkWaitGroup *sync.Wa
 			jMeta.AddString("],\n      \"inputs\": {")
 		}
 	}
-	isFirstMeta := true
 
 	// Concatenate the generated CSS chunks together
 	var compileResultsForSourceMap []compileResultForSourceMap
@@ -5507,18 +5545,6 @@ func (c *linkerContext) generateChunkCSS(chunkIndex int, chunkWaitGroup *sync.Wa
 				})
 			}
 		}
-
-		// Include this file in the metadata
-		if c.options.NeedsMetafile {
-			if isFirstMeta {
-				isFirstMeta = false
-			} else {
-				jMeta.AddString(",")
-			}
-			jMeta.AddString(fmt.Sprintf("\n        %s: {\n          \"bytesInOutput\": %d\n        }",
-				helpers.QuoteForJSON(c.graph.Files[compileResult.sourceIndex].InputFile.Source.PrettyPath, c.options.ASCIIOnly),
-				len(compileResult.CSS)))
-		}
 	}
 
 	// Make sure the file ends with a newline
@@ -5531,7 +5557,7 @@ func (c *linkerContext) generateChunkCSS(chunkIndex int, chunkWaitGroup *sync.Wa
 	}
 
 	// The CSS contents are done now that the source map comment is in
-	chunk.intermediateOutput = c.breakOutputIntoPieces(j)
+	chunk.intermediateOutput = c.breakJoinerIntoPieces(j)
 	timer.End("Join CSS files")
 
 	if c.options.SourceMap != config.SourceMapNone {
@@ -5544,8 +5570,21 @@ func (c *linkerContext) generateChunkCSS(chunkIndex int, chunkWaitGroup *sync.Wa
 	// End the metadata lazily. The final output size is not known until the
 	// final import paths are substituted into the output pieces generated below.
 	if c.options.NeedsMetafile {
+		pieces := make([]intermediateOutput, len(compileResults))
+		for i, compileResult := range compileResults {
+			pieces[i] = c.breakOutputIntoPieces(compileResult.CSS)
+		}
 		chunk.jsonMetadataChunkCallback = func(finalOutputSize int) helpers.Joiner {
-			if !isFirstMeta {
+			finalRelDir := c.fs.Dir(chunk.finalRelPath)
+			for i, compileResult := range compileResults {
+				if i > 0 {
+					jMeta.AddString(",")
+				}
+				jMeta.AddString(fmt.Sprintf("\n        %s: {\n          \"bytesInOutput\": %d\n        }",
+					helpers.QuoteForJSON(c.graph.Files[compileResult.sourceIndex].InputFile.Source.PrettyPath, c.options.ASCIIOnly),
+					c.accurateFinalByteCount(pieces[i], finalRelDir)))
+			}
+			if len(compileResults) > 0 {
 				jMeta.AddString("\n      ")
 			}
 			jMeta.AddString(fmt.Sprintf("},\n      \"bytes\": %d\n    }", finalOutputSize))
@@ -5632,16 +5671,18 @@ func (c *linkerContext) appendIsolatedHashesForImportedChunks(
 	hash.Write(chunk.waitForIsolatedHash())
 }
 
-func (c *linkerContext) breakOutputIntoPieces(j helpers.Joiner) intermediateOutput {
+func (c *linkerContext) breakJoinerIntoPieces(j helpers.Joiner) intermediateOutput {
 	// Optimization: If there can be no substitutions, just reuse the initial
 	// joiner that was used when generating the intermediate chunk output
 	// instead of creating another one and copying the whole file into it.
 	if !j.Contains(c.uniqueKeyPrefix, c.uniqueKeyPrefixBytes) {
 		return intermediateOutput{joiner: j}
 	}
+	return c.breakOutputIntoPieces(j.Done())
+}
 
+func (c *linkerContext) breakOutputIntoPieces(output []byte) intermediateOutput {
 	var pieces []outputPiece
-	output := j.Done()
 	prefix := c.uniqueKeyPrefixBytes
 	for {
 		// Scan for the next piece boundary
diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt
@@ -3165,7 +3165,7 @@ d {
           "bytesInOutput": 101
         },
         "project/entry.js": {
-          "bytesInOutput": 242
+          "bytesInOutput": 233
         },
         "project/esm.js": {
           "bytesInOutput": 21
@@ -3174,7 +3174,7 @@ d {
           "bytesInOutput": 24
         },
         "project/file.file": {
-          "bytesInOutput": 48
+          "bytesInOutput": 43
         }
       },
       "bytes": 642
@@ -3234,7 +3234,7 @@ d {
       "entryPoint": "project/entry.css",
       "inputs": {
         "project/entry.css": {
-          "bytesInOutput": 193
+          "bytesInOutput": 183
         }
       },
       "bytes": 230
@@ -3351,7 +3351,7 @@ a {
       "entryPoint": "project/bytesInOutput should be at least 99 (1).js",
       "inputs": {
         "project/111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111.file": {
-          "bytesInOutput": 45
+          "bytesInOutput": 135
         },
         "project/bytesInOutput should be at least 99 (1).js": {
           "bytesInOutput": 24
@@ -3380,7 +3380,7 @@ a {
       "entryPoint": "project/bytesInOutput should be at least 99 (2).js",
       "inputs": {
         "project/bytesInOutput should be at least 99 (2).js": {
-          "bytesInOutput": 59
+          "bytesInOutput": 149
         }
       },
       "bytes": 203
@@ -3396,7 +3396,7 @@ a {
       "entryPoint": "project/bytesInOutput should be at least 99 (3).js",
       "inputs": {
         "project/bytesInOutput should be at least 99 (3).js": {
-          "bytesInOutput": 55
+          "bytesInOutput": 143
         }
       },
       "bytes": 197
@@ -3432,7 +3432,7 @@ a {
       "entryPoint": "project/bytesInOutput should be at least 99.css",
       "inputs": {
         "project/bytesInOutput should be at least 99.css": {
-          "bytesInOutput": 52
+          "bytesInOutput": 142
         }
       },
       "bytes": 196
diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js
@@ -1241,7 +1241,7 @@ body {
     assert.deepStrictEqual(json.outputs[outImport2].exports, [])
     assert.deepStrictEqual(json.outputs[outChunk].exports, [])
 
-    assert.deepStrictEqual(json.outputs[outEntry].inputs, { [inEntry]: { bytesInOutput: 74 } })
+    assert.deepStrictEqual(json.outputs[outEntry].inputs, { [inEntry]: { bytesInOutput: 66 } })
     assert.deepStrictEqual(json.outputs[outImport1].inputs, { [inImport1]: { bytesInOutput: 0 } })
     assert.deepStrictEqual(json.outputs[outImport2].inputs, { [inImport2]: { bytesInOutput: 0 } })
     assert.deepStrictEqual(json.outputs[outChunk].inputs, { [inShared]: { bytesInOutput: 28 } })

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,10 @@`
`26`	`26`	`}`
`27`	`27`	```
`28`	`28`
	`29`	`+* Fix byte counts in metafile regarding references to other output files ([#2071](https://github.com/evanw/esbuild/issues/2071))`
	`30`	`+`
	`31`	+ Previously files that contained references to other output files had slightly incorrect metadata for the byte counts of input files which contributed to that output file. So for example if `app.js` imports `image.png` using the file loader and esbuild generates `out.js` and `image-LSAMBFUD.png`, the metadata for how many bytes of `out.js` are from `app.js` was slightly off (the metadata for the byte count of `out.js` was still correct). The reason is because esbuild substitutes the final paths for references between output files toward the end of the build to handle cyclic references, and the byte counts needed to be adjusted as well during the path substitution. This release fixes these byte counts (specifically the `bytesInOutput` values).
	`32`	`+`
`29`	`33`	`* The alias feature now strips a trailing slash ([#2730](https://github.com/evanw/esbuild/issues/2730))`
`30`	`34`
`31`	`35`	People sometimes add a trailing slash to the name of one of node's built-in modules to force node to import from the file system instead of importing the built-in module. For example, importing `util` imports node's built-in module called `util` but importing `util/` tries to find a package called `util` on the file system. Previously attempting to use esbuild's package alias feature to replace imports to `util` with a specific file would fail because the file path would also gain a trailing slash (e.g. mapping `util` to `./file.js` turned `util/` into `./file.js/`). With this release, esbuild will now omit the path suffix if it's a single trailing slash, which should now allow you to successfully apply aliases to these import paths.