Skip to content

Commit

Permalink
fix #2793: bundling edge case with dynamic import
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 7, 2023
1 parent 6398f81 commit dad3e64
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 8 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@

This information is helpful when trying to reduce the size of your bundle. Using the ESM variant of a dependency instead of the CommonJS variant always results in a faster and smaller bundle because it omits CommonJS wrappers, and also may result in better tree-shaking as it allows esbuild to perform tree-shaking at the statement level instead of the module level.

* Fix a bundling edge case with dynamic import ([#2793](https://github.com/evanw/esbuild/issues/2793))

This release fixes a bug where esbuild's bundler could produce incorrect output. The problematic edge case involves the entry point importing itself using a dynamic `import()` expression in an imported file, like this:

```js
// src/a.js
export const A = 42;

// src/b.js
export const B = async () => (await import(".")).A

// src/index.js
export * from "./a"
export * from "./b"
```

## 0.16.14

* Preserve some comments in expressions ([#2721](https://github.com/evanw/esbuild/issues/2721))
Expand Down
47 changes: 47 additions & 0 deletions internal/bundler_tests/bundler_splitting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,50 @@ func TestSplittingChunkPathDirPlaceholderImplicitOutbase(t *testing.T) {
},
})
}

func TestEdgeCaseIssue2793WithSplitting(t *testing.T) {
splitting_suite.expectBundled(t, bundled{
files: map[string]string{
"/src/a.js": `
export const A = 42;
`,
"/src/b.js": `
export const B = async () => (await import(".")).A
`,
"/src/index.js": `
export * from "./a"
export * from "./b"
`,
},
entryPaths: []string{"/src/index.js"},
options: config.Options{
Mode: config.ModeBundle,
OutputFormat: config.FormatESModule,
CodeSplitting: true,
AbsOutputDir: "/out",
},
})
}

func TestEdgeCaseIssue2793WithoutSplitting(t *testing.T) {
splitting_suite.expectBundled(t, bundled{
files: map[string]string{
"/src/a.js": `
export const A = 42;
`,
"/src/b.js": `
export const B = async () => (await import(".")).A
`,
"/src/index.js": `
export * from "./a"
export * from "./b"
`,
},
entryPaths: []string{"/src/index.js"},
options: config.Options{
Mode: config.ModeBundle,
OutputFormat: config.FormatESModule,
AbsOutputDir: "/out",
},
})
}
50 changes: 50 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_splitting.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,53 @@
TestEdgeCaseIssue2793WithSplitting
---------- /out/index.js ----------
// src/a.js
var A = 42;

// src/b.js
var B = async () => (await import("./index.js")).A;
export {
A,
B
};

================================================================================
TestEdgeCaseIssue2793WithoutSplitting
---------- /out/index.js ----------
// src/a.js
var A;
var init_a = __esm({
"src/a.js"() {
A = 42;
}
});

// src/b.js
var B;
var init_b = __esm({
"src/b.js"() {
B = async () => (await Promise.resolve().then(() => (init_src(), src_exports))).A;
}
});

// src/index.js
var src_exports = {};
__export(src_exports, {
A: () => A,
B: () => B
});
var init_src = __esm({
"src/index.js"() {
init_a();
init_b();
}
});
init_src();
export {
A,
B
};

================================================================================
TestSplittingAssignToLocal
---------- /out/a.js ----------
import {
Expand Down
13 changes: 5 additions & 8 deletions internal/linker/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,13 +780,7 @@ func (c *linkerContext) computeCrossChunkDependencies() {
c.timer.Begin("Compute cross-chunk dependencies")
defer c.timer.End("Compute cross-chunk dependencies")

jsChunks := 0
for _, chunk := range c.chunks {
if _, ok := chunk.chunkRepr.(*chunkReprJS); ok {
jsChunks++
}
}
if jsChunks < 2 {
if !c.options.CodeSplitting {
// No need to compute cross-chunk dependencies if there can't be any
return
}
Expand Down Expand Up @@ -2866,7 +2860,10 @@ func (c *linkerContext) markFileLiveForTreeShaking(sourceIndex uint32) {
}

func (c *linkerContext) isExternalDynamicImport(record *ast.ImportRecord, sourceIndex uint32) bool {
return record.Kind == ast.ImportDynamic && c.graph.Files[record.SourceIndex.GetIndex()].IsEntryPoint() && record.SourceIndex.GetIndex() != sourceIndex
return c.options.CodeSplitting &&
record.Kind == ast.ImportDynamic &&
c.graph.Files[record.SourceIndex.GetIndex()].IsEntryPoint() &&
record.SourceIndex.GetIndex() != sourceIndex
}

func (c *linkerContext) markPartLiveForTreeShaking(sourceIndex uint32, partIndex uint32) {
Expand Down
30 changes: 30 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,36 @@ for (const minify of [[], ['--minify']]) {
'node2.ts': `throw [foo.bar, require('./node2.ts')] // Force this file to be lazily initialized so foo.js is lazily initialized`,
'foo.js': `export let foo = {bar: 123}`,
}),

// https://github.com/evanw/esbuild/issues/2793
test(['--bundle', 'src/index.js', '--outfile=node.js', '--format=esm'].concat(minify), {
'src/a.js': `
export const A = 42;
`,
'src/b.js': `
export const B = async () => (await import(".")).A
`,
'src/index.js': `
export * from "./a"
export * from "./b"
import { B } from '.'
export let async = async () => { if (42 !== await B()) throw 'fail' }
`,
}, { async: true }),
test(['--bundle', 'src/node.js', '--outdir=.', '--format=esm', '--splitting'].concat(minify), {
'src/a.js': `
export const A = 42;
`,
'src/b.js': `
export const B = async () => (await import("./node")).A
`,
'src/node.js': `
export * from "./a"
export * from "./b"
import { B } from './node'
export let async = async () => { if (42 !== await B()) throw 'fail' }
`,
}, { async: true }),
)
}

Expand Down

0 comments on commit dad3e64

Please sign in to comment.