Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various bugs with class expressions and JavaScript decorators (code run twice, internal compiler crash) #58436

Open
evanw opened this issue May 4, 2024 · 1 comment Β· May be fixed by #58554
Open
Assignees
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Domain: Decorators The issue relates to the decorator syntax Fix Available A PR has been opened for this issue

Comments

@evanw
Copy link
Contributor

evanw commented May 4, 2024

πŸ”Ž Search Terms

javascript decorators proposal class expressions crash debug failure error

πŸ•— Version & Regression Information

  • This is a crash

⏯ Playground Link

No response

πŸ’» Code

Case 1: (playground link)

const log: string[] = []
const dec = (x: number, y: number, z: number, t: number) => {
  log.push('x' + x)
  return (_: any, ctx: DecoratorContext): any => {
    log.push('y' + y)
    ctx.addInitializer(() => {
      log.push('z' + z)
    })
    if (ctx.kind === 'field') return () => { log.push('f' + t) }
    if (ctx.kind === 'accessor') return { init: () => { log.push('a' + t) } }
  }
}
const Foo = @dec(0, 4, 2, -1) class {
  @dec(1, 3, 3, 1) field: undefined
  @dec(2, 2, 0, 0) static field: undefined
  @dec(3, 1, 4, 1) accessor accessor: undefined
  @dec(4, 0, 1, 0) static accessor accessor: undefined
}
new Foo
const expected = 'x0,x1,x2,x3,x4,y0,y1,y2,y3,y4,f0,z0,a0,z1,z2,f1,z3,a1,z4'
const success = log + '' === expected
console.log('success: ' + success)
if (!success) {
  console.log('observed: ' + log)
  console.log('expected: ' + expected)
}

Case 2 (playground link):

This is case 1 with the decorator on the class expression removed.

const dec = (x: number, y: number, z: number, t: number) => {
  return (_: any, ctx: DecoratorContext): any => {
  }
}
const Foo = class {
  @dec(1, 3, 3, 1) field: undefined
  @dec(2, 2, 0, 0) static field: undefined
  @dec(3, 1, 4, 1) accessor accessor: undefined
  @dec(4, 0, 1, 0) static accessor accessor: undefined
}

Case 3 (playground link):

This is case 1 with the two instance elements removed.

const dec = (x: number, y: number, z: number, t: number) => {
  return (_: any, ctx: DecoratorContext): any => {
  }
}
const Foo = @dec(0, 4, 2, -1) class {
  @dec(2, 2, 0, 0) static field: undefined
  @dec(4, 0, 1, 0) static accessor accessor: undefined
}

πŸ™ Actual behavior

Case 1:

When run, this prints the following:

[LOG]: "success: false" 
[LOG]: "observed: x0,x1,x2,x3,x4,y0,y1,y2,y3,y4,f0,z0,a0,z1,z2,f0,z0,a0,f1,z3,a1,z4" 
[LOG]: "expected: x0,x1,x2,x3,x4,y0,y1,y2,y3,y4,f0,z0,a0,z1,z2,f1,z3,a1,z4" 

That means:

  1. TypeScript's behavior for decorators on class expressions diverges from TypeScript's behavior for class statements. The expected output was taken from TypeScript's transformation of the same code but as a class statement instead of a class expression, and appears to be correct.

  2. The code that TypeScript generates evaluates several initializers multiple times. Specifically, static field and static accessor initializers are run twice, which corresponds to duplicated calls to __runInitializers(_classThis, _static_field_extraInitializers) and __runInitializers(_classThis, _static_accessor_initializers) in the generated code.

Case 2:

This crashes the playground compiler with Error: Debug Failure. and a minified stack trace.

Case 3:

This crashes the playground compiler with Error: Debug Failure. False expression: Undeclared private name for property declaration. and a minified stack trace.

πŸ™‚ Expected behavior

TypeScript should transform decorators into code that behaves the same whether or not the original class was an expression or a statement. The compiler should also not crash with debug assertion failures.

For context, I discovered these issues while expanding the coverage of my decorator test suite: https://github.com/evanw/decorator-tests. I intend to use these test cases when adding support for transforming decorators to esbuild.

Additional information about the issue

No response

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: Decorators The issue relates to the decorator syntax Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output labels May 6, 2024
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.5.1 milestone May 6, 2024
@rbuckton
Copy link
Member

rbuckton commented May 8, 2024

For Case 1, it looks like the static field is being emit twice, which is triggering initializers twice.

For Case 2 and Case 3, it looks like a combination of target: esnext and useDefineForClassFields: false playing poorly with decorators.

@typescript-bot typescript-bot added Fix Available A PR has been opened for this issue labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output Domain: Decorators The issue relates to the decorator syntax Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants