Skip to content

Commit

Permalink
fix #1623: ignore class fields marked "abstract"
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 21, 2021
1 parent e03e743 commit 3249e05
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 1 deletion.
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@

## Unreleased

* Fix compilation of abstract class fields in TypeScript ([#1623](https://github.com/evanw/esbuild/issues/1623))

This release fixes a bug where esbuild could incorrectly include a TypeScript abstract class field in the compiled JavaScript output. This is incorrect because the official TypeScript compiler never does this. Note that this only happened in scenarios where TypeScript's `useDefineForClassFields` setting was set to `true` (or equivalently where TypeScript's `target` setting was set to `ESNext`). Here is the difference:

```js
// Original code
abstract class Foo {
abstract foo: any;
}

// Old output
class Foo {
foo;
}

// New output
class Foo {
}
```

* Proxy from the `__require` shim to `require` ([#1614](https://github.com/evanw/esbuild/issues/1614))

Some background: esbuild's bundler emulates a CommonJS environment. The bundling process replaces the literal syntax `require(<string>)` with the referenced module at compile-time. However, other uses of `require` such as `require(someFunction())` are not bundled since the value of `someFunction()` depends on code evaluation, and esbuild does not evaluate code at compile-time. So it's possible for some references to `require` to remain after bundling.
Expand Down
50 changes: 50 additions & 0 deletions internal/bundler/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1194,3 +1194,53 @@ func TestTSComputedClassFieldUseDefineTrueLower(t *testing.T) {
},
})
}

func TestTSAbstractClassFieldUseAssign(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
const keepThis = Symbol('keepThis')
declare const AND_REMOVE_THIS: unique symbol
abstract class Foo {
REMOVE_THIS: any
[keepThis]: any
abstract REMOVE_THIS_TOO: any
abstract [AND_REMOVE_THIS]: any
abstract [(x => y => x + y)('nested')('scopes')]: any
}
(() => new Foo())()
`,
},
entryPaths: []string{"/entry.ts"},
options: config.Options{
Mode: config.ModePassThrough,
AbsOutputFile: "/out.js",
UseDefineForClassFields: config.False,
},
})
}

func TestTSAbstractClassFieldUseDefine(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
const keepThisToo = Symbol('keepThisToo')
declare const REMOVE_THIS_TOO: unique symbol
abstract class Foo {
keepThis: any
[keepThisToo]: any
abstract REMOVE_THIS: any
abstract [REMOVE_THIS_TOO]: any
abstract [(x => y => x + y)('nested')('scopes')]: any
}
(() => new Foo())()
`,
},
entryPaths: []string{"/entry.ts"},
options: config.Options{
Mode: config.ModePassThrough,
AbsOutputFile: "/out.js",
UseDefineForClassFields: config.True,
},
})
}
19 changes: 19 additions & 0 deletions internal/bundler/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2579,6 +2579,25 @@ switch (bar) {
let a;
}

================================================================================
TestTSAbstractClassFieldUseAssign
---------- /out.js ----------
const keepThis = Symbol("keepThis");
class Foo {
}
keepThis;
(() => new Foo())();

================================================================================
TestTSAbstractClassFieldUseDefine
---------- /out.js ----------
const keepThisToo = Symbol("keepThisToo");
class Foo {
keepThis;
[keepThisToo];
}
(() => new Foo())();

================================================================================
TestTSComputedClassFieldUseDefineFalse
---------- /out.js ----------
Expand Down
12 changes: 11 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -1809,6 +1809,7 @@ type propertyOpts struct {
// Class-related options
isStatic bool
isTSDeclare bool
isTSAbstract bool
isClass bool
classHasExtends bool
allowTSDecorators bool
Expand Down Expand Up @@ -1936,7 +1937,16 @@ func (p *parser) parseProperty(kind js_ast.PropertyKind, opts propertyOpts, erro
return js_ast.Property{}, false
}

case "private", "protected", "public", "readonly", "abstract", "override":
case "abstract":
if opts.isClass && p.options.ts.Parse && !opts.isTSAbstract && raw == name {
opts.isTSAbstract = true
scopeIndex := len(p.scopesInOrder)
p.parseProperty(kind, opts, nil)
p.discardScopesUpTo(scopeIndex)
return js_ast.Property{}, false
}

case "private", "protected", "public", "readonly", "override":
// Skip over TypeScript keywords
if opts.isClass && p.options.ts.Parse && raw == name {
return p.parseProperty(kind, opts, nil)
Expand Down

0 comments on commit 3249e05

Please sign in to comment.