fix #1099: always hash all chunk contents

CHANGELOG.md

@@ -10,6 +10,12 @@
 
     To avoid these problems, esbuild will now use different names for its helper functions.
 
+* Fix a chunk hashing issue ([#1099](https://github.com/evanw/esbuild/issues/1099))
+
+    Previously the chunk hashing algorithm skipped hashing entry point chunks when the `--entry-names=` setting doesn't contain `[hash]`, since the hash wasn't used in the file name. However, this is no longer correct with the change in version 0.11.0 that made dynamic entry point chunks use `--chunk-names=` instead of `--entry-names=` since `--chunk-names=` can still contain `[hash]`.
+
+    With this release, chunk contents will now always be hashed regardless of the chunk type. This makes esbuild somewhat slower than before in the common case, but it fixes this correctness issue.
+
 ## 0.11.3
 
 * Auto-define `process.env.NODE_ENV` when platform is set to `browser`

internal/bundler/linker.go

@@ -4914,14 +4914,6 @@ func (c *linkerContext) breakOutputIntoPieces(output []byte, chunkCount uint32)
 func (c *linkerContext) generateIsolatedChunkHash(chunk *chunkInfo, pieces []outputPiece) {
 	chunk.outputPieces = pieces
 
-	// Attempt to determine when a hash is not needed, and then bail early without
-	// computing the hash. This is to improve performance in the common one-file case.
-	if chunk.isEntryPoint && !config.HasPlaceholder(c.options.EntryPathTemplate, config.HashPlaceholder) {
-		chunk.outputHash = sha1.New()
-		chunk.isolatedChunkHash = []byte{}
-		return
-	}
-
 	hash := sha1.New()
 
 	// Mix the file names and part ranges of all of the files in this chunk into

internal/bundler/snapshots/snapshots_default.txt

@@ -2650,23 +2650,23 @@ var import_es6_ns_export_abstract_class = __toModule(require_es6_ns_export_abstr
 TestTopLevelAwaitAllowedImportWithSplitting
 ---------- /out/entry.js ----------
 // entry.js
-import("./a-QJXNRG2U.js");
-import("./b-QJXNRG2U.js");
-import("./c-7Y4CCLAO.js");
+import("./a-CDAX2F64.js");
+import("./b-TU32ADII.js");
+import("./c-HCUABLOG.js");
 require_entry();
 await 0;
 
----------- /out/a-QJXNRG2U.js ----------
+---------- /out/a-CDAX2F64.js ----------
 import "./chunk-7VOEXAIV.js";
 import "./chunk-FLIOJ2XZ.js";
 
----------- /out/b-QJXNRG2U.js ----------
+---------- /out/b-TU32ADII.js ----------
 import "./chunk-7VOEXAIV.js";
 import "./chunk-FLIOJ2XZ.js";
 
 ---------- /out/chunk-7VOEXAIV.js ----------
 
----------- /out/c-7Y4CCLAO.js ----------
+---------- /out/c-HCUABLOG.js ----------
 import "./chunk-FLIOJ2XZ.js";
 
 ---------- /out/chunk-FLIOJ2XZ.js ----------

internal/bundler/snapshots/snapshots_splitting.txt

@@ -209,9 +209,9 @@ import {
 
 // entry.js
 var import_foo = __toModule(require_foo());
-import("./foo-42KSISCA.js").then(({default: {bar: b}}) => console.log(import_foo.bar, b));
+import("./foo-CARSV5UE.js").then(({default: {bar: b}}) => console.log(import_foo.bar, b));
 
----------- /out/foo-42KSISCA.js ----------
+---------- /out/foo-CARSV5UE.js ----------
 import {
   require_foo
 } from "./chunk-EIGPJKCZ.js";
@@ -236,9 +236,9 @@ import {
 } from "./chunk-3CWABKVA.js";
 
 // entry.js
-import("./foo-H5N7CGKD.js").then(({bar: b}) => console.log(bar, b));
+import("./foo-BS4VMZCM.js").then(({bar: b}) => console.log(bar, b));
 
----------- /out/foo-H5N7CGKD.js ----------
+---------- /out/foo-BS4VMZCM.js ----------
 import {
   bar
 } from "./chunk-3CWABKVA.js";
@@ -258,9 +258,9 @@ export {
 TestSplittingDynamicCommonJSIntoES6
 ---------- /out/entry.js ----------
 // entry.js
-import("./foo-3I42H3S6.js").then(({default: {bar}}) => console.log(bar));
+import("./foo-AADOILU2.js").then(({default: {bar}}) => console.log(bar));
 
----------- /out/foo-3I42H3S6.js ----------
+---------- /out/foo-AADOILU2.js ----------
 // foo.js
 var require_foo = __commonJS((exports) => {
   exports.bar = 123;
@@ -271,9 +271,9 @@ export default require_foo();
 TestSplittingDynamicES6IntoES6
 ---------- /out/entry.js ----------
 // entry.js
-import("./foo-3I42H3S6.js").then(({bar}) => console.log(bar));
+import("./foo-4LZZB43C.js").then(({bar}) => console.log(bar));
 
----------- /out/foo-3I42H3S6.js ----------
+---------- /out/foo-4LZZB43C.js ----------
 // foo.js
 var bar = 123;
 export {
@@ -297,13 +297,13 @@ export {
 TestSplittingDynamicImportOutsideSourceTreeIssue264
 ---------- /out/entry1.js ----------
 // Users/user/project/src/entry1.js
-import("./package-3I42H3S6.js");
+import("./package-OI36NPKJ.js");
 
 ---------- /out/entry2.js ----------
 // Users/user/project/src/entry2.js
-import("./package-3I42H3S6.js");
+import("./package-OI36NPKJ.js");
 
----------- /out/package-3I42H3S6.js ----------
+---------- /out/package-OI36NPKJ.js ----------
 // Users/user/project/node_modules/package/index.js
 console.log("imported");
 
@@ -442,9 +442,9 @@ export {
 TestSplittingPublicPathEntryName
 ---------- /out/a.js ----------
 // a.js
-import("/www/b-3I42H3S6.js");
+import("/www/b-OEFETMRQ.js");
 
----------- /out/b-3I42H3S6.js ----------
+---------- /out/b-OEFETMRQ.js ----------
 // b.js
 console.log("b");
 

scripts/js-api-tests.js

@@ -930,8 +930,8 @@ body {
     const inShared = makeInPath(shared);
     const chunk = 'chunk-HPFXYN2E.js';
     const outEntry = makeOutPath(path.relative(testDir, entry));
-    const outImport1 = makeOutPath('import1-5CIBHJEX.js');
-    const outImport2 = makeOutPath('import2-5CIBHJEX.js');
+    const outImport1 = makeOutPath('import1-7LFMXWCG.js');
+    const outImport2 = makeOutPath('import2-BCITUCVB.js');
     const outChunk = makeOutPath(chunk);
 
     assert.deepStrictEqual(json.inputs[inEntry], {
@@ -956,6 +956,13 @@ body {
     })
     assert.deepStrictEqual(json.inputs[inShared], { bytes: 38, imports: [] })
 
+    assert.deepStrictEqual(Object.keys(json.outputs), [
+      outEntry,
+      outImport1,
+      outImport2,
+      outChunk,
+    ])
+
     assert.deepStrictEqual(json.outputs[outEntry].imports, [{ path: makeOutPath(chunk), kind: 'import-statement' }])
     assert.deepStrictEqual(json.outputs[outImport1].imports, [{ path: makeOutPath(chunk), kind: 'import-statement' }])
     assert.deepStrictEqual(json.outputs[outImport2].imports, [{ path: makeOutPath(chunk), kind: 'import-statement' }])

scripts/plugin-tests.js

@@ -1863,6 +1863,40 @@ let pluginTests = {
     })
     assert.strictEqual(build.outputFiles[0].path, path.join(testDir, 'first.js'))
   },
+
+  async dynamicImportDuplicateChunkIssue1099({ esbuild, testDir }) {
+    const outdir = path.join(testDir, 'out')
+    await mkdirAsync(path.join(testDir, 'hi'), { recursive: true })
+    await writeFileAsync(path.join(testDir, 'index.js'), `import x from 'manifest'; console.log(x.name(), x.hi())`)
+    await writeFileAsync(path.join(testDir, 'name.js'), `import x from 'manifest'; console.log(x.index(), x.hi())`)
+    await writeFileAsync(path.join(testDir, 'hi', 'name.js'), `import x from 'manifest'; console.log(x.index(), x.name())`)
+    await esbuild.build({
+      entryPoints: [path.join(testDir, 'index.js')],
+      outdir,
+      bundle: true,
+      splitting: true,
+      format: 'esm',
+      plugins: [{
+        name: 'plugin',
+        setup(build) {
+          build.onResolve({ filter: /^manifest$/ }, () => {
+            return { path: 'manifest', namespace: 'Manifest' }
+          })
+          build.onLoad({ namespace: 'Manifest', filter: /.*/ }, () => {
+            return {
+              resolveDir: testDir,
+              contents: `
+                export const index = () => import('./index')
+                export const name = () => import('./name')
+                export const hi = () => import('./hi/name')
+                export default {index, name, hi}
+              `,
+            }
+          })
+        },
+      }],
+    })
+  },
 }
 
 // These tests have to run synchronously