Skip to content

Commit

Permalink
fix #2534: jsx name collision edge case
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 8, 2022
1 parent c64f7b4 commit 1641569
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 2 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,25 @@
# Changelog

## Unreleased

* Fix JSX name collision edge case ([#2534](https://github.com/evanw/esbuild/issues/2534))

Code generated by esbuild could have a name collision in the following edge case:

* The JSX transformation mode is set to `automatic`, which causes `import` statements to be inserted
* An element uses a `{...spread}` followed by a `key={...}`, which uses the legacy `createElement` fallback imported from `react`
* Another import uses a name that ends with `react` such as `@remix-run/react`
* The output format has been set to CommonJS so that `import` statements are converted into require calls

In this case, esbuild previously generated two variables with the same name `import_react`, like this:

```js
var import_react = require("react");
var import_react2 = require("@remix-run/react");
```

That bug is fixed in this release. The code generated by esbuild no longer contains a name collision.

## 0.15.7

* Add `--watch=forever` to allow esbuild to never terminate ([#1511](https://github.com/evanw/esbuild/issues/1511), [#1885](https://github.com/evanw/esbuild/issues/1885))
Expand Down
20 changes: 20 additions & 0 deletions internal/bundler/bundler_loader_test.go
Expand Up @@ -1161,3 +1161,23 @@ func TestLoaderCopyWithFormat(t *testing.T) {
},
})
}

func TestJSXAutomaticNoNameCollision(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.jsx": `
import { Link } from "@remix-run/react"
const x = <Link {...y} key={z} />
`,
},
entryPaths: []string{"/entry.jsx"},
options: config.Options{
Mode: config.ModeConvertFormat,
OutputFormat: config.FormatCommonJS,
AbsOutputFile: "/out.js",
JSX: config.JSXOptions{
AutomaticRuntime: true,
},
},
})
}
10 changes: 10 additions & 0 deletions internal/bundler/snapshots/snapshots_default.txt
Expand Up @@ -1462,6 +1462,16 @@ console.log(/* @__PURE__ */ jsx2("div", {
children: /* @__PURE__ */ jsx2(Fragment, {})
}));

================================================================================
TestJSXAutomaticNoNameCollision
---------- /out.js ----------
var import_react = require("react");
var import_react2 = require("@remix-run/react");
const x = /* @__PURE__ */ (0, import_react.createElement)(import_react2.Link, {
...y,
key: z
});

================================================================================
TestJSXConstantFragments
---------- /out.js ----------
Expand Down
5 changes: 3 additions & 2 deletions internal/js_parser/js_parser.go
Expand Up @@ -16364,17 +16364,18 @@ func (p *parser) generateImportStmt(

namespaceRef := p.newSymbol(js_ast.SymbolOther, "import_"+js_ast.GenerateNonUniqueNameFromPath(path))
p.moduleScope.Generated = append(p.moduleScope.Generated, namespaceRef)
declaredSymbols := make([]js_ast.DeclaredSymbol, len(imports))
declaredSymbols := make([]js_ast.DeclaredSymbol, 1+len(imports))
clauseItems := make([]js_ast.ClauseItem, len(imports))
importRecordIndex := p.addImportRecord(ast.ImportStmt, loc, path, nil)
if sourceIndex != nil {
p.importRecords[importRecordIndex].SourceIndex = ast.MakeIndex32(*sourceIndex)
}
declaredSymbols[0] = js_ast.DeclaredSymbol{Ref: namespaceRef, IsTopLevel: true}

// Create per-import information
for i, alias := range imports {
it := symbols[alias]
declaredSymbols[i] = js_ast.DeclaredSymbol{Ref: it.Ref, IsTopLevel: true}
declaredSymbols[i+1] = js_ast.DeclaredSymbol{Ref: it.Ref, IsTopLevel: true}
clauseItems[i] = js_ast.ClauseItem{
Alias: alias,
AliasLoc: it.Loc,
Expand Down

0 comments on commit 1641569

Please sign in to comment.