diff --git a/CHANGELOG.md b/CHANGELOG.md index 287f5272f8e..808ae64373a 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index 0134ae806c3..594adae1bc0 100644 --- a/internal/bundler/linker.go +++ b/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 diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 933c24d843e..c00b3575b2e 100644 --- a/scripts/end-to-end-tests.js +++ b/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