From d5000619c9e5e5f2eceabf71b5e108d2d6d144c1 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 16 Jun 2022 09:02:28 -0400 Subject: [PATCH] fix #2322: a log message for ambiguous re-exports --- CHANGELOG.md | 22 ++++++++++++++++ internal/bundler/bundler_default_test.go | 25 +++++++++++++++++++ internal/bundler/linker.go | 25 +++++++++++++++++-- .../bundler/snapshots/snapshots_loader.txt | 17 +++++++++++++ internal/logger/msg_ids.go | 5 ++++ 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c7eb3e12e4..b6330565a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## Unreleased + +* Add a log message for ambiguous re-exports ([#2322](https://github.com/evanw/esbuild/issues/2322)) + + In JavaScript, you can re-export symbols from another file using `export * from './another-file'`. When you do this from multiple files that export different symbols with the same name, this creates an ambiguous export which is causes that name to not be exported. This is harmless if you don't plan on using the ambiguous export name, so esbuild doesn't have a warning for this. But if you do want a warning for this (or if you want to make it an error), you can now opt-in to seeing this log message with `--log-override:ambiguous-import=warning` or `--log-override:ambiguous-import=error`. The log message looks like this: + + ``` + ▲ [WARNING] Re-export of "common" in "example.js" is ambiguous and has been removed [ambiguous-import] + + One definition of "common" comes from "a.js" here: + + a.js:2:11: + 2 │ export let common = 2 + ╵ ~~~~~~ + + Another definition of "common" comes from "b.js" here: + + b.js:3:14: + 3 │ export { b as common } + ╵ ~~~~~~ + ``` + ## 0.14.44 * Add a `copy` loader ([#2255](https://github.com/evanw/esbuild/issues/2255)) diff --git a/internal/bundler/bundler_default_test.go b/internal/bundler/bundler_default_test.go index 59b9dce7137..ba023294216 100644 --- a/internal/bundler/bundler_default_test.go +++ b/internal/bundler/bundler_default_test.go @@ -6397,3 +6397,28 @@ ident.js: DEBUG: Indirect calls to "require" will not be bundled `, }) } + +func TestAmbiguousReexportMsg(t *testing.T) { + loader_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + export * from './a' + export * from './b' + export * from './c' + `, + "/a.js": `export let a = 1, x = 2`, + "/b.js": `export let b = 3; export { b as x }`, + "/c.js": `export let c = 4, x = 5`, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + }, + debugLogs: true, + expectedCompileLog: `DEBUG: Re-export of "x" in "entry.js" is ambiguous and has been removed +a.js: NOTE: One definition of "x" comes from "a.js" here: +b.js: NOTE: Another definition of "x" comes from "b.js" here: +`, + }) +} diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index c00bfa3d293..525e3b3add9 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -1443,21 +1443,42 @@ func (c *linkerContext) scanImportsAndExports() { aliases := make([]string, 0, len(repr.Meta.ResolvedExports)) nextAlias: for alias, export := range repr.Meta.ResolvedExports { + otherFile := &c.graph.Files[export.SourceIndex].InputFile + otherRepr := otherFile.Repr.(*graph.JSRepr) + // Re-exporting multiple symbols with the same name causes an ambiguous // export. These names cannot be used and should not end up in generated code. - otherRepr := c.graph.Files[export.SourceIndex].InputFile.Repr.(*graph.JSRepr) if len(export.PotentiallyAmbiguousExportStarRefs) > 0 { mainRef := export.Ref + mainLoc := export.NameLoc if imported, ok := otherRepr.Meta.ImportsToBind[export.Ref]; ok { mainRef = imported.Ref + mainLoc = imported.NameLoc } + for _, ambiguousExport := range export.PotentiallyAmbiguousExportStarRefs { - ambiguousRepr := c.graph.Files[ambiguousExport.SourceIndex].InputFile.Repr.(*graph.JSRepr) + ambiguousFile := &c.graph.Files[ambiguousExport.SourceIndex].InputFile + ambiguousRepr := ambiguousFile.Repr.(*graph.JSRepr) ambiguousRef := ambiguousExport.Ref + ambiguousLoc := ambiguousExport.NameLoc if imported, ok := ambiguousRepr.Meta.ImportsToBind[ambiguousExport.Ref]; ok { ambiguousRef = imported.Ref + ambiguousLoc = imported.NameLoc } + if mainRef != ambiguousRef { + file := &c.graph.Files[sourceIndex].InputFile + otherTracker := logger.MakeLineColumnTracker(&otherFile.Source) + ambiguousTracker := logger.MakeLineColumnTracker(&ambiguousFile.Source) + c.log.AddIDWithNotes(logger.MsgID_Bundler_AmbiguousReexport, logger.Debug, nil, logger.Range{}, + fmt.Sprintf("Re-export of %q in %q is ambiguous and has been removed", alias, file.Source.PrettyPath), + []logger.MsgData{ + otherTracker.MsgData(js_lexer.RangeOfIdentifier(otherFile.Source, mainLoc), + fmt.Sprintf("One definition of %q comes from %q here:", alias, otherFile.Source.PrettyPath)), + ambiguousTracker.MsgData(js_lexer.RangeOfIdentifier(ambiguousFile.Source, ambiguousLoc), + fmt.Sprintf("Another definition of %q comes from %q here:", alias, ambiguousFile.Source.PrettyPath)), + }, + ) continue nextAlias } } diff --git a/internal/bundler/snapshots/snapshots_loader.txt b/internal/bundler/snapshots/snapshots_loader.txt index 0ad229920d5..d6f3e7b1b13 100644 --- a/internal/bundler/snapshots/snapshots_loader.txt +++ b/internal/bundler/snapshots/snapshots_loader.txt @@ -1,3 +1,20 @@ +TestAmbiguousReexportMsg +---------- /out/entry.js ---------- +// a.js +var a = 1; + +// b.js +var b = 3; + +// c.js +var c = 4; +export { + a, + b, + c +}; + +================================================================================ TestAutoDetectMimeTypeFromExtension ---------- /out.js ---------- // test.svg diff --git a/internal/logger/msg_ids.go b/internal/logger/msg_ids.go index 5f5de4772e7..671a7d0479a 100644 --- a/internal/logger/msg_ids.go +++ b/internal/logger/msg_ids.go @@ -49,6 +49,7 @@ const ( MsgID_CSS_UnsupportedCSSProperty // Bundler + MsgID_Bundler_AmbiguousReexport MsgID_Bundler_DifferentPathCase MsgID_Bundler_IgnoredBareImport MsgID_Bundler_IgnoredDynamicImport @@ -156,6 +157,8 @@ func StringToMsgIDs(str string, logLevel LogLevel, overrides map[MsgID]LogLevel) overrides[MsgID_CSS_UnsupportedCSSProperty] = logLevel // Bundler + case "ambiguous-reexport": + overrides[MsgID_Bundler_AmbiguousReexport] = logLevel case "different-path-case": overrides[MsgID_Bundler_DifferentPathCase] = logLevel case "ignored-bare-import": @@ -266,6 +269,8 @@ func MsgIDToString(id MsgID) string { return "unsupported-css-property" // Bundler + case MsgID_Bundler_AmbiguousReexport: + return "ambiguous-reexport" case MsgID_Bundler_DifferentPathCase: return "different-path-case" case MsgID_Bundler_IgnoredBareImport: