Disable tree shaking when code splitting is active

CHANGELOG.md

@@ -40,6 +40,10 @@
 
     2. The hashing algorithm has been changed from SHA1 to [xxHash](https://github.com/Cyan4973/xxHash) (specifically [this Go implementation](https://github.com/cespare/xxhash)) which means the hashing step is around 6x faster than before. Thanks to [@Jarred-Sumner](https://github.com/Jarred-Sumner) for the suggestion.
 
+* Disable tree shaking annotations when code splitting is active ([#1070](https://github.com/evanw/esbuild/issues/1070), [#1081](https://github.com/evanw/esbuild/issues/1081))
+
+    Support for [Webpack's `"sideEffects": false` annotation](https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free) in `package.json` is now disabled when code splitting is enabled and there is more than one entry point. This avoids a bug that could cause generated chunks to reference each other in some cases. Now all chunks generated by code splitting should be acyclic.
+
 ## 0.11.4
 
 * Avoid name collisions with TypeScript helper functions ([#1102](https://github.com/evanw/esbuild/issues/1102))

internal/bundler/linker.go

@@ -594,6 +594,37 @@ func (c *linkerContext) link() []OutputFile {
 	return c.generateChunksInParallel(chunks)
 }
 
+// Currently the automatic chunk generation algorithm should by construction
+// never generate chunks that import each other since files are allocated to
+// chunks based on which entry points they are reachable from.
+//
+// This will change in the future when we allow manual chunk labels. But before
+// we allow manual chunk labels, we'll need to rework module initialization to
+// allow code splitting chunks to be lazily-initialized.
+//
+// Since that work hasn't been finished yet, cycles in the chunk import graph
+// can cause initialization bugs. So let's forbid these cycles for now to guard
+// against code splitting bugs that could cause us to generate buggy chunks.
+func (c *linkerContext) enforceNoCyclicChunkImports(chunks []chunkInfo) {
+	var validate func(int, []int)
+	validate = func(chunkIndex int, path []int) {
+		for _, otherChunkIndex := range path {
+			if chunkIndex == otherChunkIndex {
+				c.log.AddError(nil, logger.Loc{}, "Internal error: generated chunks contain a circular import")
+				return
+			}
+		}
+		path = append(path, chunkIndex)
+		for _, otherChunkIndex := range chunks[chunkIndex].crossChunkImports {
+			validate(int(otherChunkIndex), path)
+		}
+	}
+	path := make([]int, 0, len(chunks))
+	for i := range chunks {
+		validate(i, path)
+	}
+}
+
 func (c *linkerContext) generateChunksInParallel(chunks []chunkInfo) []OutputFile {
 	// Generate each chunk on a separate goroutine
 	generateWaitGroup := sync.WaitGroup{}
@@ -606,6 +637,7 @@ func (c *linkerContext) generateChunksInParallel(chunks []chunkInfo) []OutputFil
 			go c.generateChunkCSS(chunks, chunkIndex, &generateWaitGroup)
 		}
 	}
+	c.enforceNoCyclicChunkImports(chunks)
 	generateWaitGroup.Wait()
 
 	// Compute the final hashes of each chunk. This can technically be done in
@@ -2537,7 +2569,11 @@ func (c *linkerContext) includeFile(sourceIndex uint32, entryPointBit uint, dist
 					// Don't include this module for its side effects if it can be
 					// considered to have no side effects
 					if otherFile := &c.files[otherSourceIndex]; otherFile.ignoreIfUnused && !c.options.IgnoreDCEAnnotations {
-						continue
+						// This is currently unsafe when code splitting is enabled, so
+						// disable it in that case
+						if len(c.entryPoints) < 2 {
+							continue
+						}
 					}
 
 					// Otherwise, include this module for its side effects

scripts/end-to-end-tests.js

@@ -3380,6 +3380,43 @@
         if (a === b || ca === cb || a !== ca || b !== cb) throw 'fail'
       `,
     }),
+
+    // "sideEffects": false
+    // https://github.com/evanw/esbuild/issues/1081
+    test(['entry.js', '--outdir=out', '--splitting', '--format=esm', '--bundle', '--chunk-names=[name]'], {
+      'entry.js': `import('./a'); import('./b')`,
+      'a.js': `import { bar } from './shared'; bar()`,
+      'b.js': `import './shared'`,
+      'shared.js': `import { foo } from './foo'; export let bar = foo`,
+      'foo/index.js': `export let foo = () => {}`,
+      'foo/package.json': `{ "sideEffects": false }`,
+      'node.js': `
+        import path from 'path'
+        import url from 'url'
+        const __dirname = path.dirname(url.fileURLToPath(import.meta.url))
+
+        // Read the output files
+        import fs from 'fs'
+        const a = fs.readFileSync(path.join(__dirname, 'out', 'a.js'), 'utf8')
+        const chunk = fs.readFileSync(path.join(__dirname, 'out', 'chunk.js'), 'utf8')
+
+        // Make sure the two output files don't import each other
+        import assert from 'assert'
+        assert.notStrictEqual(chunk.includes('a.js'), a.includes('chunk.js'), 'chunks must not import each other')
+      `,
+    }),
+    test(['entry.js', '--outdir=out', '--splitting', '--format=esm', '--bundle'], {
+      'entry.js': `await import('./a'); await import('./b')`,
+      'a.js': `import { bar } from './shared'; bar()`,
+      'b.js': `import './shared'`,
+      'shared.js': `import { foo } from './foo'; export let bar = foo`,
+      'foo/index.js': `export let foo = () => {}`,
+      'foo/package.json': `{ "sideEffects": false }`,
+      'node.js': `
+        // This must not crash
+        import './out/entry.js'
+      `,
+    }),
   )
 
   // Test the binary loader