fix #605: re-export of side-effect free cjs

CHANGELOG.md

@@ -6,6 +6,10 @@
 
     Now calling `startService()` multiple times will share the underlying esbuild child process as long as the lifetimes of the service objects overlap (i.e. the time from `startService()` to `service.stop()`). This is just an internal change; there is no change to the public API. It should result in a faster implementation that uses less memory if your code calls `startService()` multiple times. Previously each call to `startService()` generated a separate esbuild child process.
 
+* Fix re-exports of a side-effect free CommonJS module ([#605](https://github.com/evanw/esbuild/issues/605))
+
+    This release fixes a regression introduced in version 0.8.19 in which an `import` of an `export {...} from` re-export of a CommonJS module does not include the CommonJS module if it has been marked as `"sideEffect": false` in its `package.json` file. This was the case with the [Ramda](https://ramdajs.com/) library, and was due to an unhandled case in the linker.
+
 ## 0.8.23
 
 * Fix non-string objects being passed to `transformSync` ([#596](https://github.com/evanw/esbuild/issues/596))

internal/bundler/bundler_dce_test.go

@@ -1158,3 +1158,32 @@ func TestRemoveTrailingReturn(t *testing.T) {
 		},
 	})
 }
+
+func TestImportReExportOfNamespaceImport(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/Users/user/project/entry.js": `
+				import * as ns from 'pkg'
+				console.log(ns.foo)
+			`,
+			"/Users/user/project/node_modules/pkg/index.js": `
+				export { default as foo } from './foo'
+				export { default as bar } from './bar'
+			`,
+			"/Users/user/project/node_modules/pkg/package.json": `
+				{ "sideEffects": false }
+			`,
+			"/Users/user/project/node_modules/pkg/foo.js": `
+				module.exports = 123
+			`,
+			"/Users/user/project/node_modules/pkg/bar.js": `
+				module.exports = 'abc'
+			`,
+		},
+		entryPaths: []string{"/Users/user/project/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}

internal/bundler/linker.go

@@ -1758,6 +1758,17 @@ func (c *linkerContext) matchImportsWithExportsForFile(sourceIndex uint32) {
 				Alias:        result.alias,
 			}
 
+		case matchImportNormalAndNamespace:
+			repr.meta.importsToBind[importRef] = importToBind{
+				sourceIndex: result.sourceIndex,
+				ref:         result.ref,
+			}
+
+			c.symbols.Get(importRef).NamespaceAlias = &js_ast.NamespaceAlias{
+				NamespaceRef: result.namespaceRef,
+				Alias:        result.alias,
+			}
+
 		case matchImportCycle:
 			namedImport := repr.ast.NamedImports[importRef]
 			c.addRangeError(file.source, js_lexer.RangeOfIdentifier(file.source, namedImport.AliasLoc),
@@ -1786,6 +1797,9 @@ const (
 	// "namespaceRef" and "alias" are in use
 	matchImportNamespace
 
+	// Both "matchImportNormal" and "matchImportNamespace"
+	matchImportNormalAndNamespace
+
 	// The import could not be evaluated due to a cycle
 	matchImportCycle
 
@@ -1844,10 +1858,16 @@ loop:
 			trackerFile := &c.files[tracker.sourceIndex]
 			namedImport := trackerFile.repr.(*reprJS).ast.NamedImports[tracker.importRef]
 			if namedImport.NamespaceRef != js_ast.InvalidRef {
-				result = matchImportResult{
-					kind:         matchImportNamespace,
-					namespaceRef: namedImport.NamespaceRef,
-					alias:        namedImport.Alias,
+				if result.kind == matchImportNormal {
+					result.kind = matchImportNormalAndNamespace
+					result.namespaceRef = namedImport.NamespaceRef
+					result.alias = namedImport.Alias
+				} else {
+					result = matchImportResult{
+						kind:         matchImportNamespace,
+						namespaceRef: namedImport.NamespaceRef,
+						alias:        namedImport.Alias,
+					}
 				}
 			}
 

internal/bundler/snapshots/snapshots_dce.txt

@@ -101,6 +101,26 @@ TestFileLoaderRemoveUnused
 // entry.js
 console.log("unused import");
 
+================================================================================
+TestImportReExportOfNamespaceImport
+---------- /out.js ----------
+// Users/user/project/node_modules/pkg/foo.js
+var require_foo = __commonJS((exports, module) => {
+  module.exports = 123;
+});
+
+// Users/user/project/node_modules/pkg/bar.js
+var require_bar = __commonJS((exports, module) => {
+  module.exports = "abc";
+});
+
+// Users/user/project/node_modules/pkg/index.js
+var foo = __toModule(require_foo());
+var bar = __toModule(require_bar());
+
+// Users/user/project/entry.js
+console.log(foo.default);
+
 ================================================================================
 TestJSONLoaderRemoveUnused
 ---------- /out.js ----------