esbuild: Class renaming breaks certain patterns

It’s not uncommon for code to use the class name constructor for certain patterns i.e.

const registry: Record<string, WebComponent> = {}

class WebComponent {
  static register (registrationID = this.constructor.name) { register[registryID] = this }
  static load (registryID: string) { return register[registryID] }
}

class ComponentA extends WebComponent {}

ComponentA.register()

elsewhere

const ComponentA = '123'
WebComponent.load('ComponentA') // fails b/c it's "ComponentA2"

When the esbuild bundler hits a conflicting class name it appends a number, which breaks the pattern.

There are several potential ways to fix this, including (purely for illustration):

an optional check (e.g. --no-class-rename) that throws an error rather than adding a number to the class name. (For obvious reasons, this --no-class-rename would also ideally prevent class name mangling during minification).
giving priority to class names not being renamed (i.e. maintaining its original name) over other symbols when there’s a collision
including some source-code comment that specifies that the symbol not be renamed

This is blocking our project from using esbuild, and would also break one of the patterns in the TKO project that I maintain (a successor to/built on knockout.js).

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 2
Comments: 16 (4 by maintainers)

Most upvoted comments

Anyone coming here from Google, the option --keep-names has been added since this was closed I think. It seems to have solved the problem I had that led me here.

+10

davetron5000 on Feb 28, 2023

Yeah sorry about that. I discovered this myself yesterday. It’s already fixed and is going out with the next release: https://github.com/evanw/esbuild/blob/2001dff446c016612f4043c8ff8b7cb1bf69ed5e/CHANGELOG.md#unreleased. It’s actually not related to this issue. It’s related to issue #478 instead.

evanw on Dec 6, 2020

What makes the late-rename necessary? Not sure about TS, but in JS, let Hello2 = class Hello {}; and class Hello2 {} have semantic equivalence apart from what value ends up set as the function’s name.

Here’s the case I’m worried about. Imagine two files:

// a.js
import {Hello as Foo} from './b'
export class Hello { foo = new Foo }

// b.js
import {Hello as Bar} from './a'
export class Hello { bar = new Bar }

It would be bad if esbuild’s scope hoisting pass compiled that to something like this:

// a.js
let Hello = class Hello { foo = new Hello2 }

// b.js
let Hello2 = class Hello { bar = new Hello }

The second class stored in the variable Hello2 would accidentally set bar to an instance of the class from file b.js instead of the class from file a.js because the use of a class expression introduces a new symbol Hello that shadows the top-level symbol Hello. It might be possible to get this to work with enough renaming magic but it seems more complicated.

Another issue is that people are going to want this to work with functions too, and there the same approach doesn’t apply because function declarations are hoisted while variable declarations and function expressions aren’t. Hoisting can be mitigated to some extent by reordering statements but doing that across files starts to get complex.

Basically assigning to name is the simplest solution.

evanw on Nov 8, 2020