From 88821b7e7d46737f633120f91c65f662eace0bcf Mon Sep 17 00:00:00 2001
From: Evan Wallace <evan.exe@gmail.com>
Date: Wed, 5 May 2021 20:16:33 -0700
Subject: [PATCH] fix #1252: code splitting cjs re-export edge case

---
 CHANGELOG.md                |  4 ++++
 internal/bundler/linker.go  |  8 ++++++++
 scripts/end-to-end-tests.js | 16 ++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 947067228a2..2ec7733c6e2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,10 @@
 
     With this release, you can now continue to use esbuild after calling `stop()`. This will restart esbuild's API and means that you will need to call `stop()` again for Deno to be able to exit. This feature was contributed by [@lucacasonato](https://github.com/lucacasonato).
 
+* Fix code splitting edge case ([#1252](https://github.com/evanw/esbuild/issues/1252))
+
+    This release fixes an edge case where bundling with code splitting enabled generated incorrect code if multiple ESM entry points re-exported the same re-exported symbol from a CommonJS file. In this case the cross-chunk symbol dependency should be the variable that holds the return value from the `require()` call instead of the original ESM named `import` clause item. When this bug occurred, the generated ESM code contained an export and import for a symbol that didn't exist, which caused a module initialization error. This case should now work correctly.
+
 * Fix code generation with `declare` class fields ([#1242](https://github.com/evanw/esbuild/issues/1242))
 
     This fixes a bug with TypeScript code that uses `declare` on a class field and your `tsconfig.json` file has `"useDefineForClassFields": true`. Fields marked as `declare` should not be defined in the generated code, but they were incorrectly being declared as `undefined`. These fields are now correctly omitted from the generated code.
diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go
index f220387eaca..7765ac6432f 100644
--- a/internal/bundler/linker.go
+++ b/internal/bundler/linker.go
@@ -782,6 +782,14 @@ func (c *linkerContext) computeCrossChunkDependencies(chunks []chunkInfo) {
 								targetRef = importData.Ref
 							}
 
+							// If this is an ES6 import from a CommonJS file, it will become a
+							// property access off the namespace symbol instead of a bare
+							// identifier. In that case we want to pull in the namespace symbol
+							// instead. The namespace symbol stores the result of "require()".
+							if symbol := c.graph.Symbols.Get(targetRef); symbol.NamespaceAlias != nil {
+								targetRef = symbol.NamespaceAlias.NamespaceRef
+							}
+
 							imports[targetRef] = true
 						}
 					}
diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js
index 1517065082d..7e17d19b009 100644
--- a/scripts/end-to-end-tests.js
+++ b/scripts/end-to-end-tests.js
@@ -3907,6 +3907,22 @@
         if (a.test !== 1 || b.test !== 2 || c.test !== 1 || d.test !== 2) throw 'fail'
       `,
     }),
+
+    // https://github.com/evanw/esbuild/issues/1252
+    test(['client.js', 'utilities.js', '--splitting', '--bundle', '--format=esm', '--outdir=out'], {
+      'client.js': `export { Observable } from './utilities'`,
+      'utilities.js': `export { Observable } from './observable'`,
+      'observable.js': `
+        import Observable from './zen-observable'
+        export { Observable }
+      `,
+      'zen-observable.js': `module.exports = 123`,
+      'node.js': `
+        import {Observable as x} from './out/client.js'
+        import {Observable as y} from './out/utilities.js'
+        if (x !== 123 || y !== 123) throw 'fail'
+      `,
+    })
   )
 
   // Test the binary loader