fix #1957: include "file" loader paths in hashes

Author: evanw

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fix bug with filename hashes and the `file` loader ([#1957](https://github.com/evanw/esbuild/issues/1957))
+
+    This release fixes a bug where if a file name template has the `[hash]` placeholder (either `--entry-names=` or `--chunk-names=`), the hash that esbuild generates didn't include the content of the string generated by the `file` loader. Importing a file with the `file` loader causes the imported file to be copied to the output directory and causes the imported value to be the relative path from the output JS file to that copied file. This bug meant that if the `--asset-names=` setting also contained `[hash]` and the file loaded with the `file` loader was changed, the hash in the copied file name would change but the hash of the JS file would not change, which could potentially result in a stale JS file being loaded. Now the hash of the JS file will be changed too which fixes the reload issue.
+
 * Prefer the `import` condition for entry points ([#1956](https://github.com/evanw/esbuild/issues/1956))
 
     The `exports` field in `package.json` maps package subpaths to file paths. The mapping can be conditional, which lets it vary in different situations. For example, you can have an `import` condition that applies when the subpath originated from a JS import statement, and a `require` condition that applies when the subpath originated from a JS require call. These are supposed to be mutually exclusive according to the specification: https://nodejs.org/api/packages.html#conditional-exports.

internal/bundler/linker.go

@@ -374,7 +374,7 @@ func (c *linkerContext) generateChunksInParallel(chunks []chunkInfo) []graph.Out
 		if config.HasPlaceholder(chunk.finalTemplate, config.HashPlaceholder) {
 			// Compute the final hash using the isolated hashes of the dependencies
 			hash := xxhash.New()
-			appendIsolatedHashesForImportedChunks(hash, chunks, uint32(chunkIndex), visited, ^uint32(chunkIndex))
+			c.appendIsolatedHashesForImportedChunks(hash, chunks, uint32(chunkIndex), visited, ^uint32(chunkIndex))
 			finalBytes = hash.Sum(finalBytes[:0])
 			finalString := hashForFileName(finalBytes)
 			hashSubstitution = &finalString
@@ -5200,7 +5200,7 @@ func maybeAppendLegalComments(
 	}
 }
 
-func appendIsolatedHashesForImportedChunks(
+func (c *linkerContext) appendIsolatedHashesForImportedChunks(
 	hash hash.Hash,
 	chunks []chunkInfo,
 	chunkIndex uint32,
@@ -5219,7 +5219,24 @@ func appendIsolatedHashesForImportedChunks(
 
 	// Visit the other chunks that this chunk imports before visiting this chunk
 	for _, chunkImport := range chunk.crossChunkImports {
-		appendIsolatedHashesForImportedChunks(hash, chunks, chunkImport.chunkIndex, visited, visitedKey)
+		c.appendIsolatedHashesForImportedChunks(hash, chunks, chunkImport.chunkIndex, visited, visitedKey)
+	}
+
+	// Mix in hashes for referenced asset paths (i.e. the "file" loader)
+	for _, piece := range chunk.intermediateOutput.pieces {
+		if piece.kind == 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, "\\", "/")
+
+			// Mix in the hash for the relative path, which ends up as a JS string
+			hashWriteLengthPrefixed(hash, []byte(relPath))
+		}
 	}
 
 	// Mix in the hash for this chunk

scripts/js-api-tests.js

@@ -583,6 +583,77 @@ let buildTests = {
     assert.strictEqual(result.__esModule, true)
   },
 
+  async fileLoaderEntryHash({ esbuild, testDir }) {
+    const input = path.join(testDir, 'index.js')
+    const data = path.join(testDir, 'data.bin')
+    const outdir = path.join(testDir, 'out')
+    await writeFileAsync(input, `export {default as value} from ${JSON.stringify(data)}`)
+    await writeFileAsync(data, `stuff`)
+    const result1 = await esbuild.build({
+      entryPoints: [input],
+      bundle: true,
+      outdir,
+      format: 'cjs',
+      loader: { '.bin': 'file' },
+      entryNames: '[name]-[hash]',
+      write: false,
+    })
+    await writeFileAsync(data, `more stuff`)
+    const result2 = await esbuild.build({
+      entryPoints: [input],
+      bundle: true,
+      outdir,
+      format: 'cjs',
+      loader: { '.bin': 'file' },
+      entryNames: '[name]-[hash]',
+      write: false,
+    })
+    assert.strictEqual(result1.outputFiles.length, 2)
+    assert.strictEqual(result2.outputFiles.length, 2)
+
+    // Make sure each path is unique. This tests for a bug where the hash in
+    // the output filename corresponding to the "index.js" entry point wasn't
+    // including the filename for the "file" loader.
+    assert.strictEqual(new Set(result1.outputFiles.concat(result2.outputFiles).map(x => x.path)).size, 4)
+  },
+
+  async fileLoaderEntryHashNoChange({ esbuild, testDir }) {
+    const input = path.join(testDir, 'index.js')
+    const data = path.join(testDir, 'data.bin')
+    const outdir = path.join(testDir, 'out')
+    await writeFileAsync(input, `export {default as value} from ${JSON.stringify(data)}`)
+    await writeFileAsync(data, `stuff`)
+    const result1 = await esbuild.build({
+      entryPoints: [input],
+      bundle: true,
+      outdir,
+      format: 'cjs',
+      loader: { '.bin': 'file' },
+      entryNames: '[name]-[hash]',
+      assetNames: '[name]',
+      write: false,
+    })
+    await writeFileAsync(data, `more stuff`)
+    const result2 = await esbuild.build({
+      entryPoints: [input],
+      bundle: true,
+      outdir,
+      format: 'cjs',
+      loader: { '.bin': 'file' },
+      entryNames: '[name]-[hash]',
+      assetNames: '[name]',
+      write: false,
+    })
+    assert.strictEqual(result1.outputFiles.length, 2)
+    assert.strictEqual(result2.outputFiles.length, 2)
+
+    // The paths should be the same. The hash augmentation from the previous
+    // test should only be checking for a file name difference, not a difference
+    // in content, because the JS output file only contains the file name for
+    // something using the "file" loader.
+    assert.strictEqual(new Set(result1.outputFiles.concat(result2.outputFiles).map(x => x.path)).size, 2)
+  },
+
   async splittingPublicPath({ esbuild, testDir }) {
     const input1 = path.join(testDir, 'a', 'in1.js')
     const input2 = path.join(testDir, 'b', 'in2.js')