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

static variable causes typescript lowering #1480

Closed
hyrious opened this issue Jul 30, 2021 · 4 comments
Closed

static variable causes typescript lowering #1480

hyrious opened this issue Jul 30, 2021 · 4 comments

Comments

@hyrious
Copy link

@hyrious hyrious commented Jul 30, 2021

I've noticed lowering always occurs in --target=esnext when there is static variable in class and the loader is ts or tsx (js and jsx are not affected). For example, this code:

class A {
    static C = 1;
    #a;
    f() { this.#a ??= A.C }
}

will always be lowered to something include __privateGet. while this code is not lowered when the loader is js.

Is this an intentional behavior?

Related issue: #1418 (comment)

Live example

@evanw
Copy link
Owner

@evanw evanw commented Sep 2, 2021

I believe in this case the issue is that the official TypeScript compiler defaults to useDefineForClassFields: false so esbuild does too. When you set it to true such as with this:

--loader=ts --target=esnext --tsconfig-raw={"compilerOptions":{"useDefineForClassFields":true}}

then that code is not lowered. Specifically when useDefineForClassFields is false the public class field initializer static C = 1; must be transformed to A.C = 1; to change from "define" to "assign" semantics. When that happens, the private class field initializer #a is lowered as well in case the initializer for C references it. This is necessary because the initializer is moved outside of the class body and syntactically the symbol #a is only valid inside of the class body. For example, if the variable was initialized to static C = new A().#a; instead, it would be a syntax error to convert that to A.C = new A().#a; outside of the class body. This check is somewhat conservative but it's done for the correctness of the resulting code. If you want this exact pass-through behavior, you'll have to configure TypeScript such that class fields behave like JavaScript since it's not the default behavior.

@hyrious
Copy link
Author

@hyrious hyrious commented Sep 2, 2021

Thank you for so detailed answer.
Maybe we can make useDefineForClassFields default to true when using transform with --target=esnext?

As of TypeScript default behavior, it is not even a bundler ——

 cat a.ts
class A {
  static C = 1;
  #a;
  f() { this.#a ??= A.C }
}

 tsc a.ts --target esnext --outFile /dev/stdout
class A {
    static C = 1;
    #a;
    f() { this.#a ??= A.C; }
}

@hyrious
Copy link
Author

@hyrious hyrious commented Nov 2, 2021

Update: I found typescript has a simple transform api, while its default behavior is targetting commonjs and es5.

const ts = require('typescript')

let start = performance.now()
let result = ts.transpileModule(`
  class A { a = 1 }
`, {
  compilerOptions: {
    target: 99 // esnext, if not set this, it will output es5
  }
})
let elapsed = performance.now() - start

console.log(result.outputText)
console.log(`[Finished in ${elapsed.toFixed(2)}ms]`)

Output:

class A {
    a = 1;
}

[Finished in 36.37ms]

@hyrious
Copy link
Author

@hyrious hyrious commented Dec 8, 2021

Since it was resolved in 0.14.0 (you can use --target=esnext to enable useDefineForClassFields in transform mode), closing it.

@hyrious hyrious closed this Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants