From 516ca317a40185e69265cb50a146130b2e6a7fe6 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Fri, 24 May 2024 16:58:06 -0400 Subject: [PATCH] fix #3343: allow bundle-internal string aliases --- CHANGELOG.md | 4 + .../bundler_tests/bundler_default_test.go | 201 ++++++++++++++++++ .../snapshots/snapshots_default.txt | 46 ++++ internal/js_parser/js_parser.go | 2 - internal/js_parser/js_parser_lower.go | 5 - internal/js_parser/js_parser_test.go | 10 - internal/linker/linker.go | 38 ++++ 7 files changed, 289 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e5f28bed8..345facf225e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,10 @@ Chrome shipped this new CSS at-rule in version 125 as part of the [CSS anchor positioning API](https://developer.chrome.com/blog/anchor-positioning-api). With this release, esbuild now knows to expect a declaration list inside of the `@position-try` body block and will format it appropriately. +* Always allow internal string import and export aliases ([#3343](https://github.com/evanw/esbuild/issues/3343)) + + Import and export names can be string literals in ES2022+. Previously esbuild forbid any usage of these aliases when the target was below ES2022. Starting with this release, esbuild will only forbid such usage when the alias would otherwise end up in output as a string literal. String literal aliases that are only used internally in the bundle and are "compiled away" are no longer errors. This makes it possible to use string literal aliases with esbuild's `inject` feature even when the target is earlier than ES2022. + ## 0.21.3 * Implement the decorator metadata proposal ([#3760](https://github.com/evanw/esbuild/issues/3760)) diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go index 5b574ac38f4..dd8d6b8fbad 100644 --- a/internal/bundler_tests/bundler_default_test.go +++ b/internal/bundler_tests/bundler_default_test.go @@ -8894,3 +8894,204 @@ func TestObjectLiteralProtoSetterEdgeCasesMinifySyntax(t *testing.T) { }, }) } + +func TestForbidStringImportNamesNoBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import { "an import" as anImport } from "./foo" + export { "another import" as "an export" } from "./foo" + anImport() + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModePassThrough, + AbsOutputFile: "/out.js", + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + expectedCompileLog: `entry.js: ERROR: Using the string "an import" as an import name is not supported in the configured target environment +entry.js: ERROR: Using the string "another import" as an import name is not supported in the configured target environment +entry.js: ERROR: Using the string "an export" as an export name is not supported in the configured target environment +`, + }) +} + +func TestForbidStringExportNamesNoBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + let ok = true + export { ok as "ok", ok as "not ok" } + export { "same name" } from "./foo" + export { "name 1" as "name 2" } from "./foo" + export * as "name space" from "./foo" + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModePassThrough, + AbsOutputFile: "/out.js", + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + expectedCompileLog: `entry.js: ERROR: Using the string "not ok" as an export name is not supported in the configured target environment +entry.js: ERROR: Using the string "same name" as an export name is not supported in the configured target environment +entry.js: ERROR: Using the string "name 1" as an import name is not supported in the configured target environment +entry.js: ERROR: Using the string "name 2" as an export name is not supported in the configured target environment +entry.js: ERROR: Using the string "name space" as an export name is not supported in the configured target environment +`, + }) +} + +func TestForbidStringImportNamesBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import { "nest ed" as nested } from "./nested.js" + export { nested } + `, + "/nested.js": ` + import { "some import" as nested } from "external" + export { nested as "nest ed" } + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + ExternalSettings: config.ExternalSettings{ + PreResolve: config.ExternalMatchers{Exact: map[string]bool{ + "external": true, + }}, + }, + }, + expectedCompileLog: `nested.js: ERROR: Using the string "some import" as an import name is not supported in the configured target environment +`, + }) +} + +func TestForbidStringExportNamesBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import { "o.k." as ok } from "./internal.js" + export { ok as "ok", ok as "not ok" } + export * from "./nested.js" + export * as "name space" from "./nested.js" + `, + "/internal.js": ` + let ok = true + export { ok as "o.k." } + `, + "/nested.js": ` + export * from "./very-nested.js" + let nested = 1 + export { nested as "nested name" } + `, + "/very-nested.js": ` + let nested = 2 + export { nested as "very nested name" } + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + expectedCompileLog: `entry.js: ERROR: Using the string "not ok" as an export name is not supported in the configured target environment +entry.js: ERROR: Using the string "name space" as an export name is not supported in the configured target environment +nested.js: ERROR: Using the string "nested name" as an export name is not supported in the configured target environment +very-nested.js: ERROR: Using the string "very nested name" as an export name is not supported in the configured target environment +`, + }) +} + +func TestInjectWithStringExportNameNoBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + console.log(test) + `, + "/inject.js": ` + const old = console.log + const fn = (...args) => old.apply(console, ['log:'].concat(args)) + export { fn as "console.log" } + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModePassThrough, + AbsOutputFile: "/out.js", + InjectPaths: []string{"/inject.js"}, + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + }) +} + +func TestInjectWithStringExportNameBundle(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + console.log(test) + console.info(test) + console.warn(test) + `, + "/inject.js": ` + const old = console.log + const fn = (...args) => old.apply(console, ['log:'].concat(args)) + export { fn as "console.log" } + export { "console.log" as "console.info" } from "./inject.js" + import { "console.info" as info } from "./inject.js" + export { info as "console.warn" } + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + InjectPaths: []string{"/inject.js"}, + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + }) +} + +func TestStringExportNamesCommonJS(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import { "some import" as someImport } from "./foo" + export { someImport as "some export" } + export * as "all the stuff" from "./foo" + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeConvertFormat, + AbsOutputFile: "/out.js", + OutputFormat: config.FormatCommonJS, + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + }, + }) +} + +func TestStringExportNamesIIFE(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import { "some import" as someImport } from "./foo" + export { someImport as "some export" } + export * as "all the stuff" from "./foo" + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeConvertFormat, + AbsOutputFile: "/out.js", + OutputFormat: config.FormatIIFE, + UnsupportedJSFeatures: compat.ArbitraryModuleNamespaceNames, + GlobalName: []string{"global", "name"}, + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index fb8c4ad0d17..9465f0e104e 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -2197,6 +2197,25 @@ console.log( second2 === "success (dot name)" ); +================================================================================ +TestInjectWithStringExportNameBundle +---------- /out.js ---------- +// inject.js +var old = console.log; +var fn = (...args) => old.apply(console, ["log:"].concat(args)); + +// entry.js +fn(test); +fn(test); +fn(test); + +================================================================================ +TestInjectWithStringExportNameNoBundle +---------- /out.js ---------- +var old = console.log; +var fn = (...args) => old.apply(console, ["log:"].concat(args)); +fn(test); + ================================================================================ TestJSXAutomaticImportsCommonJS ---------- /out.js ---------- @@ -6171,6 +6190,33 @@ export function outer() { } __name(outer, "outer"), outer(); +================================================================================ +TestStringExportNamesCommonJS +---------- /out.js ---------- +var entry_exports = {}; +__export(entry_exports, { + "all the stuff": () => all_the_stuff, + "some export": () => import_foo["some import"] +}); +module.exports = __toCommonJS(entry_exports); +var import_foo = require("./foo"); +var all_the_stuff = __toESM(require("./foo")); + +================================================================================ +TestStringExportNamesIIFE +---------- /out.js ---------- +var global; +(global ||= {}).name = (() => { + var entry_exports = {}; + __export(entry_exports, { + "all the stuff": () => all_the_stuff, + "some export": () => import_foo["some import"] + }); + var import_foo = require("./foo"); + var all_the_stuff = __toESM(require("./foo")); + return __toCommonJS(entry_exports); +})(); + ================================================================================ TestSwitchScopeNoBundle ---------- /out.js ---------- diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 1d293ab0b31..d1319c40966 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -5484,8 +5484,6 @@ func (p *parser) parseClauseAlias(kind string) js_lexer.MaybeSubstring { if !ok { p.log.AddError(&p.tracker, r, fmt.Sprintf("This %s alias is invalid because it contains the unpaired Unicode surrogate U+%X", kind, problem)) - } else { - p.markSyntaxFeature(compat.ArbitraryModuleNamespaceNames, r) } return js_lexer.MaybeSubstring{String: alias} } diff --git a/internal/js_parser/js_parser_lower.go b/internal/js_parser/js_parser_lower.go index f9f9958a781..3991058e178 100644 --- a/internal/js_parser/js_parser_lower.go +++ b/internal/js_parser/js_parser_lower.go @@ -88,11 +88,6 @@ func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (di "Top-level await is not available in %s", where)) return - case compat.ArbitraryModuleNamespaceNames: - p.log.AddError(&p.tracker, r, fmt.Sprintf( - "Using a string as a module namespace identifier name is not supported in %s", where)) - return - case compat.Bigint: // Transforming these will never be supported p.log.AddError(&p.tracker, r, fmt.Sprintf( diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index bf1f2b9c1f7..98c3df73adc 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -3029,8 +3029,6 @@ func TestImport(t *testing.T) { ": ERROR: This import alias is invalid because it contains the unpaired Unicode surrogate U+D800\n") expectParseError(t, "import {'\\uDC00' as x} from 'foo'", ": ERROR: This import alias is invalid because it contains the unpaired Unicode surrogate U+DC00\n") - expectParseErrorTarget(t, 2020, "import {'' as x} from 'foo'", - ": ERROR: Using a string as a module namespace identifier name is not supported in the configured target environment\n") // String import alias with "import * as" expectParseError(t, "import * as '' from 'foo'", ": ERROR: Expected identifier but found \"''\"\n") @@ -3083,8 +3081,6 @@ func TestExport(t *testing.T) { ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+D800\n") expectParseError(t, "let x; export {x as '\\uDC00'}", ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+DC00\n") - expectParseErrorTarget(t, 2020, "let x; export {x as ''}", - ": ERROR: Using a string as a module namespace identifier name is not supported in the configured target environment\n") // String import alias with "export {} from" expectPrinted(t, "export {'' as x} from 'foo'", "export { \"\" as x } from \"foo\";\n") @@ -3095,8 +3091,6 @@ func TestExport(t *testing.T) { ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+D800\n") expectParseError(t, "export {'\\uDC00' as x} from 'foo'", ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+DC00\n") - expectParseErrorTarget(t, 2020, "export {'' as x} from 'foo'", - ": ERROR: Using a string as a module namespace identifier name is not supported in the configured target environment\n") // String export alias with "export {} from" expectPrinted(t, "export {x as ''} from 'foo'", "export { x as \"\" } from \"foo\";\n") @@ -3107,8 +3101,6 @@ func TestExport(t *testing.T) { ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+D800\n") expectParseError(t, "export {x as '\\uDC00'} from 'foo'", ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+DC00\n") - expectParseErrorTarget(t, 2020, "export {x as ''} from 'foo'", - ": ERROR: Using a string as a module namespace identifier name is not supported in the configured target environment\n") // String import and export alias with "export {} from" expectPrinted(t, "export {'x'} from 'foo'", "export { x } from \"foo\";\n") @@ -3125,8 +3117,6 @@ func TestExport(t *testing.T) { ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+D800\n") expectParseError(t, "export * as '\\uDC00' from 'foo'", ": ERROR: This export alias is invalid because it contains the unpaired Unicode surrogate U+DC00\n") - expectParseErrorTarget(t, 2020, "export * as '' from 'foo'", - ": ERROR: Using a string as a module namespace identifier name is not supported in the configured target environment\n") } func TestExportDuplicates(t *testing.T) { diff --git a/internal/linker/linker.go b/internal/linker/linker.go index 8985541da16..219fe8e614b 100644 --- a/internal/linker/linker.go +++ b/internal/linker/linker.go @@ -1623,6 +1623,10 @@ func (c *linkerContext) scanImportsAndExports() { continue } + if c.options.OutputFormat == config.FormatESModule && c.options.UnsupportedJSFeatures.Has(compat.ArbitraryModuleNamespaceNames) && c.graph.Files[sourceIndex].IsEntryPoint() { + c.maybeForbidArbitraryModuleNamespaceIdentifier("export", export.SourceIndex, export.NameLoc, alias) + } + aliases = append(aliases, alias) } sort.Strings(aliases) @@ -2837,6 +2841,15 @@ loop: return } +func (c *linkerContext) maybeForbidArbitraryModuleNamespaceIdentifier(kind string, sourceIndex uint32, loc logger.Loc, alias string) { + if !js_ast.IsIdentifier(alias) { + file := &c.graph.Files[sourceIndex] + where := config.PrettyPrintTargetEnvironment(c.options.OriginalTargetEnv, c.options.UnsupportedJSFeatureOverridesMask) + c.log.AddError(file.LineColumnTracker(), file.InputFile.Source.RangeOfString(loc), fmt.Sprintf( + "Using the string %q as an %s name is not supported in %s", alias, kind, where)) + } +} + // Attempt to correct an import name with a typo func (c *linkerContext) maybeCorrectObviousTypo(repr *graph.JSRepr, name string, msg *logger.Msg) { if repr.Meta.ResolvedExportTypos == nil { @@ -4307,6 +4320,12 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL continue } + if c.options.UnsupportedJSFeatures.Has(compat.ArbitraryModuleNamespaceNames) && s.Items != nil { + for _, item := range *s.Items { + c.maybeForbidArbitraryModuleNamespaceIdentifier("import", sourceIndex, item.AliasLoc, item.Alias) + } + } + // Make sure these don't end up in the wrapper closure if shouldExtractESMStmtsForWrap { stmtList.outsideWrapperPrefix = append(stmtList.outsideWrapperPrefix, stmt) @@ -4320,6 +4339,10 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL continue } + if c.options.UnsupportedJSFeatures.Has(compat.ArbitraryModuleNamespaceNames) { + c.maybeForbidArbitraryModuleNamespaceIdentifier("export", sourceIndex, s.Alias.Loc, s.Alias.OriginalName) + } + if shouldStripExports { // Turn this statement into "import * as ns from 'path'" stmt.Data = &js_ast.SImport{ @@ -4426,6 +4449,15 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL continue } + if c.options.UnsupportedJSFeatures.Has(compat.ArbitraryModuleNamespaceNames) { + for _, item := range s.Items { + c.maybeForbidArbitraryModuleNamespaceIdentifier("export", sourceIndex, item.AliasLoc, item.Alias) + if item.AliasLoc != item.Name.Loc { + c.maybeForbidArbitraryModuleNamespaceIdentifier("import", sourceIndex, item.Name.Loc, item.OriginalName) + } + } + } + if shouldStripExports { // Turn this statement into "import {foo} from 'path'" for i, item := range s.Items { @@ -4451,6 +4483,12 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL continue } + if c.options.UnsupportedJSFeatures.Has(compat.ArbitraryModuleNamespaceNames) { + for _, item := range s.Items { + c.maybeForbidArbitraryModuleNamespaceIdentifier("export", sourceIndex, item.AliasLoc, item.Alias) + } + } + // Make sure these don't end up in the wrapper closure if shouldExtractESMStmtsForWrap { stmtList.outsideWrapperPrefix = append(stmtList.outsideWrapperPrefix, stmt)