allow ambiguous star imports (#723)

CHANGELOG.md

@@ -4,11 +4,13 @@
 
 * Improve ambiguous import handling ([#723](https://github.com/evanw/esbuild/issues/723))
 
-    It is an error to try to import a name from a file where there are multiple matching exports due to multiple `export * from` statements from files which export that name. This release contains a few improvements to ambiguous import handling.
+    It is an error to try to import a name from a file where there are multiple matching exports due to multiple `export * from` statements from files which export that name. This release contains a few improvements to ambiguous import handling:
 
-    This release fixes a bug where named export shadowing didn't work correctly with multiple levels of re-exports. A named export closer in the re-export chain is supposed to hide a named export deeper in the re-export chain without causing an ambiguous import. The bug caused this case to be incorrectly flagged as an error even though it should have been allowed. This case is now allowed without an error.
+    1. This release fixes a bug where named export shadowing didn't work correctly with multiple levels of re-exports. A named export closer in the re-export chain is supposed to hide a named export deeper in the re-export chain without causing an ambiguous import. The bug caused this case to be incorrectly flagged as an error even though it should have been allowed. This case is now allowed without an error.
 
-    Previously the error message just said that there was an ambiguous import but didn't have any additional information. With this release, the error message also points out where the two different exports that have collided are in their original source files. Hopefully this should make it quicker to diagnose these types of issues.
+    2. Previously the error message just said that there was an ambiguous import but didn't have any additional information. With this release, the error message also points out where the two different exports that have collided are in their original source files. Hopefully this should make it quicker to diagnose these types of issues.
+
+    3. Real JavaScript environments only treat ambiguous imports as an error if they are explicitly a named import. Using the `import * as` syntax and then accessing the ambiguous import with a property access results in `undefined` instead of an error. Previously esbuild also treated this case as an error because it automatically rewrites star-import syntax to named-import syntax to improve tree shaking. With this release, this case is now treated as a warning instead of an error and the import will be automatically replaced with an `undefined` literal in the bundled code.
 
 ## 0.8.36
 

internal/bundler/bundler_css_test.go

@@ -145,7 +145,7 @@ func TestCSSFromJSMissingStarImport(t *testing.T) {
 			Mode:         config.ModeBundle,
 			AbsOutputDir: "/out",
 		},
-		expectedCompileLog: `entry.js: warning: No matching export for import "missing"
+		expectedCompileLog: `entry.js: warning: Import "missing" will always be undefined because there is no matching export
 `,
 	})
 }

internal/bundler/bundler_importstar_test.go

@@ -519,6 +519,38 @@ bar.js: note: Another matching export is here
 	})
 }
 
+func TestImportExportStarAmbiguousWarning(t *testing.T) {
+	importstar_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import * as ns from './common'
+				console.log(ns.x, ns.y, ns.z)
+			`,
+			"/common.js": `
+				export * from './foo'
+				export * from './bar'
+			`,
+			"/foo.js": `
+				export const x = 1
+				export const y = 2
+			`,
+			"/bar.js": `
+				export const y = 3
+				export const z = 4
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+		expectedCompileLog: `entry.js: warning: Import "y" will always be undefined because there are multiple matching exports
+foo.js: note: One matching export is here
+bar.js: note: Another matching export is here
+`,
+	})
+}
+
 func TestReExportStarNameCollisionNotAmbiguousImport(t *testing.T) {
 	importstar_suite.expectBundled(t, bundled{
 		files: map[string]string{
@@ -1033,7 +1065,7 @@ func TestNamespaceImportMissingES6(t *testing.T) {
 			Mode:          config.ModeBundle,
 			AbsOutputFile: "/out.js",
 		},
-		expectedCompileLog: `entry.js: warning: No matching export for import "foo"
+		expectedCompileLog: `entry.js: warning: Import "foo" will always be undefined because there is no matching export
 `,
 	})
 }
@@ -1095,7 +1127,7 @@ func TestNamespaceImportUnusedMissingES6(t *testing.T) {
 			Mode:          config.ModeBundle,
 			AbsOutputFile: "/out.js",
 		},
-		expectedCompileLog: `entry.js: warning: No matching export for import "foo"
+		expectedCompileLog: `entry.js: warning: Import "foo" will always be undefined because there is no matching export
 `,
 	})
 }
@@ -1251,7 +1283,7 @@ func TestNamespaceImportReExportStarMissingES6(t *testing.T) {
 			Mode:          config.ModeBundle,
 			AbsOutputFile: "/out.js",
 		},
-		expectedCompileLog: `entry.js: warning: No matching export for import "foo"
+		expectedCompileLog: `entry.js: warning: Import "foo" will always be undefined because there is no matching export
 `,
 	})
 }
@@ -1275,7 +1307,7 @@ func TestNamespaceImportReExportStarUnusedMissingES6(t *testing.T) {
 			Mode:          config.ModeBundle,
 			AbsOutputFile: "/out.js",
 		},
-		expectedCompileLog: `entry.js: warning: No matching export for import "foo"
+		expectedCompileLog: `entry.js: warning: Import "foo" will always be undefined because there is no matching export
 `,
 	})
 }
@@ -1629,8 +1661,8 @@ func TestImportDefaultNamespaceComboIssue446(t *testing.T) {
 				},
 			},
 		},
-		expectedCompileLog: `internal-def.js: warning: No matching export for import "def"
-internal-ns-def.js: warning: No matching export for import "def"
+		expectedCompileLog: `internal-def.js: warning: Import "def" will always be undefined because there is no matching export
+internal-ns-def.js: warning: Import "def" will always be undefined because there is no matching export
 `,
 	})
 }
@@ -1662,12 +1694,12 @@ func TestImportDefaultNamespaceComboNoDefault(t *testing.T) {
 			},
 		},
 		expectedCompileLog: `entry-default-ns-prop.js: error: No matching export for import "default"
-entry-default-ns-prop.js: warning: No matching export for import "default"
+entry-default-ns-prop.js: warning: Import "default" will always be undefined because there is no matching export
 entry-default-ns.js: error: No matching export for import "default"
 entry-default-prop.js: error: No matching export for import "default"
-entry-default-prop.js: warning: No matching export for import "default"
+entry-default-prop.js: warning: Import "default" will always be undefined because there is no matching export
 entry-default.js: error: No matching export for import "default"
-entry-prop.js: warning: No matching export for import "default"
+entry-prop.js: warning: Import "default" will always be undefined because there is no matching export
 `,
 	})
 }

internal/bundler/bundler_splitting_test.go

@@ -272,7 +272,7 @@ func TestSplittingMissingLazyExport(t *testing.T) {
 			OutputFormat:  config.FormatESModule,
 			AbsOutputDir:  "/out",
 		},
-		expectedCompileLog: `common.js: warning: No matching export for import "missing"
+		expectedCompileLog: `common.js: warning: Import "missing" will always be undefined because there is no matching export
 `,
 	})
 }

internal/bundler/linker.go

@@ -1813,16 +1813,33 @@ func (c *linkerContext) matchImportsWithExportsForFile(sourceIndex uint32) {
 		case matchImportAmbiguous:
 			namedImport := repr.ast.NamedImports[importRef]
 			r := js_lexer.RangeOfIdentifier(file.source, namedImport.AliasLoc)
-			msg := fmt.Sprintf("Ambiguous import %q has multiple matching exports", namedImport.Alias)
+			var notes []logger.MsgData
+
+			// Provide the locations of both ambiguous exports if possible
 			if result.nameLoc.Start != 0 && result.otherNameLoc.Start != 0 {
 				a := c.files[result.sourceIndex].source
 				b := c.files[result.otherSourceIndex].source
-				c.addRangeErrorWithNotes(file.source, r, msg, []logger.MsgData{
+				notes = []logger.MsgData{
 					logger.RangeData(&a, js_lexer.RangeOfIdentifier(a, result.nameLoc), "One matching export is here"),
 					logger.RangeData(&b, js_lexer.RangeOfIdentifier(b, result.otherNameLoc), "Another matching export is here"),
-				})
+				}
+			}
+
+			symbol := c.symbols.Get(importRef)
+			if symbol.ImportItemStatus == js_ast.ImportItemGenerated {
+				// This is a warning instead of an error because although it appears
+				// to be a named import, it's actually an automatically-generated
+				// named import that was originally a property access on an import
+				// star namespace object. Normally this property access would just
+				// resolve to undefined at run-time instead of failing at binding-
+				// time, so we emit a warning and rewrite the value to the literal
+				// "undefined" instead of emitting an error.
+				symbol.ImportItemStatus = js_ast.ImportItemMissing
+				msg := fmt.Sprintf("Import %q will always be undefined because there are multiple matching exports", namedImport.Alias)
+				c.log.AddRangeWarningWithNotes(&file.source, r, msg, notes)
 			} else {
-				c.addRangeError(file.source, r, msg)
+				msg := fmt.Sprintf("Ambiguous import %q has multiple matching exports", namedImport.Alias)
+				c.addRangeErrorWithNotes(file.source, r, msg, notes)
 			}
 		}
 	}
@@ -1944,7 +1961,7 @@ loop:
 				// time, so we emit a warning and rewrite the value to the literal
 				// "undefined" instead of emitting an error.
 				symbol.ImportItemStatus = js_ast.ImportItemMissing
-				c.log.AddRangeWarning(&source, r, fmt.Sprintf("No matching export for import %q", namedImport.Alias))
+				c.log.AddRangeWarning(&source, r, fmt.Sprintf("Import %q will always be undefined because there is no matching export", namedImport.Alias))
 			} else {
 				c.addRangeError(source, r, fmt.Sprintf("No matching export for import %q", namedImport.Alias))
 			}

internal/bundler/snapshots/snapshots_importstar.txt

@@ -268,6 +268,18 @@ export {
   entry_exports as ns
 };
 
+================================================================================
+TestImportExportStarAmbiguousWarning
+---------- /out.js ----------
+// foo.js
+var x = 1;
+
+// bar.js
+var z = 4;
+
+// entry.js
+console.log(x, void 0, z);
+
 ================================================================================
 TestImportOfExportStar
 ---------- /out.js ----------