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

Proposal class static block support #43370

Merged
merged 45 commits into from
Jun 25, 2021

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented Mar 25, 2021

Closes #43308
Fixes #43012

* Add types factory and parser

* Add some case

* Make class static block as a container

* Update cases

* Add visitor

* Add emitter and more compile target

* Check boundary of break and continue
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 25, 2021
@Kingwl Kingwl marked this pull request as draft March 25, 2021 07:02
@Kingwl
Copy link
Contributor Author

Kingwl commented Mar 25, 2021

Let's try it again. @orta

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 25, 2021

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 3c0b7cf. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 25, 2021

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/99247/artifacts?artifactName=tgz&fileId=C99DA09C5ECBA3E66DC0D451539FCF2B7270F09A474D967333148F7F3AF9C95B02&fileName=/typescript-4.3.0-insiders.20210325.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.3.0-pr-43370-2".;

Copy link
Member

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still a Draft, but thought you might appreciate some early feedback.

We should have logic in the checker to check for invalid decorators or modifiers. We also should be checking for invalid uses of await as an identifier in the block.

src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/types.ts Outdated Show resolved Hide resolved
src/compiler/parser.ts Outdated Show resolved Hide resolved
src/compiler/parser.ts Outdated Show resolved Hide resolved
tests/baselines/reference/classStaticBlock1.symbols Outdated Show resolved Hide resolved
tests/baselines/reference/classStaticBlock1.types Outdated Show resolved Hide resolved
src/compiler/transformers/classFields.ts Outdated Show resolved Hide resolved
src/compiler/transformers/classFields.ts Outdated Show resolved Hide resolved
src/compiler/transformers/classFields.ts Show resolved Hide resolved
@Kingwl
Copy link
Contributor Author

Kingwl commented Mar 26, 2021

One of problem is we do not have something like IdentifierReferenceExpression.

If I'm correct. We should check many thinks:

  • name of property access: a.await Should be an error?
  • name of type parameter: type await = number. Should be an error?
  • name of type reference: const a: await Should be an error?
  • etc.

Seems it's very expensive? I would like to add a (flag? property?) into identifier if it's parsed by parsePrimaryExpression. That seems could help us check the reference by cheap way (And with check variable like & type reference )?

And we need many cases to check about the function boundary.

@rbuckton
Copy link
Member

rbuckton commented Mar 27, 2021

We could handle this via a special parser context, similar to NodeFlags.Await and NodeFlags.Yield, and then modify isIdentifier() in parser.ts.

For example, we don't allow await as a keyword in a type declaration in an async function today:

interface await {} // ok
async function f() {
    interface await {} // error, because we're in an Await context
    const a: await = 1; // not an error because parsing a type annotation exits the Await context
}

@rbuckton
Copy link
Member

Also a.await isn't an issue since that's parsed as a property name, which allows keywords.

@rbuckton
Copy link
Member

I would like to add a (flag? property?) into identifier if it's parsed by parsePrimaryExpression

This shouldn't be necessary.

@Kingwl
Copy link
Contributor Author

Kingwl commented Apr 16, 2021

Should we also deny the await identifiers inside type context?

@Kingwl
Copy link
Contributor Author

Kingwl commented Apr 16, 2021

Basically checked the function boundary. Computed property name is a known issue.
Please let me know if anything wrong.
Thanks!

@rbuckton
Copy link
Member

If it helps, I can work on the missing language service features and tests for static {} so that this can be ready for 4.4 beta.

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 24, 2021

that this can be ready for 4.4 beta.

Appreciate, if it need to be done before today end.

@rbuckton
Copy link
Member

I'd like to get this in before noon PDT on Friday. I've pushed up support for call hierarchy, references, and go to definition along with relevant tests.

@rbuckton
Copy link
Member

From a conversation with @RyanCavanaugh, it seems like we want to try to wrap 4.4 features today. I will do what I can to get this PR merged by EOD.

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 25, 2021

I'm not sure what time zone that you say but I'll stay late this night (6/25, your morning).

@rbuckton
Copy link
Member

I think this is good to merge except for the missing this support in static {}. Its possible we could merge this for the beta without this support (since referencing the class by name works), and add it once #43114 is merged. Either this PR or that one will have to deal with the merge conflicts.

Copy link
Member

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. We can address this support in static {} in #43114.

@rbuckton
Copy link
Member

@typescript-bot perf test
@typescript-bot run dt
@typescript-bot test this
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Heya @rbuckton, I've started to run the parallelized community code test suite on this PR at 6234640. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Heya @rbuckton, I've started to run the extended test suite on this PR at 6234640. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Heya @rbuckton, I've started to run the parallelized Definitely Typed test suite on this PR at 6234640. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Heya @rbuckton, I've started to run the perf test suite on this PR at 6234640. You can monitor the build here.

Update: The results are in!

@rbuckton
Copy link
Member

After the bot checks all the things, I can merge this in the morning if there are no major breaks. Then we can work on getting this to work with #43114.

@typescript-bot
Copy link
Collaborator

@rbuckton
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..43370

Metric main 43370 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 344,436k (± 0.02%) 344,539k (± 0.02%) +102k (+ 0.03%) 344,280k 344,682k
Parse Time 1.80s (± 0.71%) 1.80s (± 0.46%) -0.00s (- 0.22%) 1.78s 1.82s
Bind Time 0.84s (± 0.74%) 0.84s (± 0.77%) -0.00s (- 0.12%) 0.82s 0.85s
Check Time 5.20s (± 0.60%) 5.21s (± 0.41%) +0.01s (+ 0.17%) 5.16s 5.26s
Emit Time 5.60s (± 0.67%) 5.63s (± 0.54%) +0.03s (+ 0.57%) 5.55s 5.70s
Total Time 13.44s (± 0.56%) 13.48s (± 0.35%) +0.04s (+ 0.28%) 13.34s 13.55s
Compiler-Unions - node (v10.16.3, x64)
Memory used 201,022k (± 0.03%) 201,123k (± 0.06%) +101k (+ 0.05%) 200,701k 201,321k
Parse Time 0.78s (± 0.63%) 0.79s (± 0.76%) +0.00s (+ 0.13%) 0.77s 0.80s
Bind Time 0.53s (± 1.40%) 0.53s (± 1.13%) -0.00s (- 0.75%) 0.52s 0.54s
Check Time 7.63s (± 0.60%) 7.55s (± 0.41%) -0.09s (- 1.13%) 7.48s 7.63s
Emit Time 2.30s (± 1.55%) 2.29s (± 1.40%) -0.01s (- 0.52%) 2.22s 2.38s
Total Time 11.25s (± 0.68%) 11.16s (± 0.44%) -0.10s (- 0.87%) 11.06s 11.25s
Monaco - node (v10.16.3, x64)
Memory used 340,546k (± 0.03%) 340,492k (± 0.02%) -54k (- 0.02%) 340,336k 340,637k
Parse Time 1.46s (± 0.77%) 1.45s (± 0.38%) -0.01s (- 0.62%) 1.44s 1.46s
Bind Time 0.74s (± 0.98%) 0.74s (± 0.87%) 0.00s ( 0.00%) 0.73s 0.76s
Check Time 5.34s (± 0.60%) 5.33s (± 0.30%) -0.01s (- 0.17%) 5.30s 5.38s
Emit Time 3.01s (± 0.95%) 3.00s (± 0.72%) -0.01s (- 0.23%) 2.95s 3.06s
Total Time 10.56s (± 0.41%) 10.54s (± 0.26%) -0.02s (- 0.23%) 10.46s 10.59s
TFS - node (v10.16.3, x64)
Memory used 303,760k (± 0.01%) 303,773k (± 0.02%) +13k (+ 0.00%) 303,599k 303,927k
Parse Time 1.18s (± 0.64%) 1.18s (± 0.38%) -0.01s (- 0.59%) 1.17s 1.19s
Bind Time 0.71s (± 1.05%) 0.71s (± 0.31%) -0.00s (- 0.14%) 0.70s 0.71s
Check Time 4.89s (± 0.46%) 4.87s (± 0.70%) -0.02s (- 0.37%) 4.80s 4.96s
Emit Time 3.14s (± 1.82%) 3.12s (± 1.20%) -0.02s (- 0.73%) 3.05s 3.20s
Total Time 9.93s (± 0.78%) 9.88s (± 0.50%) -0.05s (- 0.52%) 9.79s 10.01s
material-ui - node (v10.16.3, x64)
Memory used 471,410k (± 0.01%) 471,388k (± 0.01%) -22k (- 0.00%) 471,294k 471,473k
Parse Time 1.72s (± 0.38%) 1.72s (± 0.33%) -0.00s (- 0.12%) 1.71s 1.74s
Bind Time 0.67s (± 0.87%) 0.66s (± 0.67%) -0.01s (- 0.75%) 0.65s 0.67s
Check Time 14.12s (± 0.48%) 14.08s (± 0.43%) -0.05s (- 0.32%) 13.98s 14.24s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.51s (± 0.43%) 16.46s (± 0.39%) -0.05s (- 0.33%) 16.34s 16.62s
Angular - node (v12.1.0, x64)
Memory used 322,741k (± 0.03%) 322,570k (± 0.10%) -171k (- 0.05%) 321,642k 322,990k
Parse Time 1.78s (± 0.57%) 1.77s (± 0.71%) -0.01s (- 0.56%) 1.75s 1.80s
Bind Time 0.83s (± 0.74%) 0.83s (± 0.60%) -0.00s (- 0.36%) 0.81s 0.83s
Check Time 5.11s (± 0.52%) 5.14s (± 0.64%) +0.03s (+ 0.57%) 5.08s 5.23s
Emit Time 5.84s (± 1.04%) 5.88s (± 1.14%) +0.04s (+ 0.69%) 5.77s 6.08s
Total Time 13.56s (± 0.64%) 13.62s (± 0.63%) +0.05s (+ 0.38%) 13.50s 13.88s
Compiler-Unions - node (v12.1.0, x64)
Memory used 188,750k (± 0.29%) 188,545k (± 0.13%) -206k (- 0.11%) 187,632k 188,822k
Parse Time 0.77s (± 0.72%) 0.77s (± 0.84%) +0.00s (+ 0.52%) 0.76s 0.79s
Bind Time 0.53s (± 1.05%) 0.53s (± 0.98%) +0.00s (+ 0.19%) 0.52s 0.54s
Check Time 7.07s (± 0.33%) 7.07s (± 0.50%) +0.00s (+ 0.03%) 6.98s 7.15s
Emit Time 2.32s (± 1.08%) 2.32s (± 1.30%) -0.00s (- 0.13%) 2.25s 2.40s
Total Time 10.69s (± 0.20%) 10.69s (± 0.46%) +0.00s (+ 0.00%) 10.57s 10.81s
Monaco - node (v12.1.0, x64)
Memory used 323,548k (± 0.02%) 323,647k (± 0.02%) +99k (+ 0.03%) 323,472k 323,828k
Parse Time 1.41s (± 0.84%) 1.42s (± 0.66%) +0.01s (+ 0.57%) 1.40s 1.45s
Bind Time 0.71s (± 0.95%) 0.72s (± 0.69%) +0.00s (+ 0.28%) 0.71s 0.73s
Check Time 5.20s (± 0.66%) 5.20s (± 0.44%) +0.00s (+ 0.10%) 5.16s 5.26s
Emit Time 3.06s (± 0.87%) 3.05s (± 0.77%) -0.01s (- 0.23%) 3.02s 3.11s
Total Time 10.38s (± 0.61%) 10.39s (± 0.46%) +0.01s (+ 0.09%) 10.31s 10.51s
TFS - node (v12.1.0, x64)
Memory used 288,369k (± 0.01%) 288,453k (± 0.03%) +84k (+ 0.03%) 288,243k 288,674k
Parse Time 1.19s (± 0.84%) 1.19s (± 0.80%) -0.00s (- 0.34%) 1.17s 1.20s
Bind Time 0.70s (± 1.17%) 0.69s (± 0.83%) -0.01s (- 1.00%) 0.68s 0.70s
Check Time 4.78s (± 0.39%) 4.79s (± 0.64%) +0.00s (+ 0.04%) 4.73s 4.86s
Emit Time 3.18s (± 1.22%) 3.20s (± 0.78%) +0.02s (+ 0.69%) 3.15s 3.25s
Total Time 9.85s (± 0.49%) 9.86s (± 0.49%) +0.01s (+ 0.10%) 9.77s 10.00s
material-ui - node (v12.1.0, x64)
Memory used 450,057k (± 0.01%) 450,113k (± 0.02%) +56k (+ 0.01%) 449,923k 450,249k
Parse Time 1.71s (± 0.76%) 1.71s (± 0.38%) +0.00s (+ 0.12%) 1.70s 1.72s
Bind Time 0.64s (± 1.27%) 0.64s (± 0.70%) -0.01s (- 0.78%) 0.63s 0.65s
Check Time 12.66s (± 0.52%) 12.62s (± 0.33%) -0.04s (- 0.36%) 12.53s 12.73s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.01s (± 0.42%) 14.96s (± 0.26%) -0.05s (- 0.34%) 14.89s 15.07s
Angular - node (v14.15.1, x64)
Memory used 321,315k (± 0.01%) 321,366k (± 0.01%) +51k (+ 0.02%) 321,305k 321,423k
Parse Time 1.81s (± 0.54%) 1.79s (± 0.45%) -0.02s (- 0.88%) 1.78s 1.81s
Bind Time 0.87s (± 0.60%) 0.88s (± 1.64%) +0.01s (+ 1.38%) 0.87s 0.94s
Check Time 5.12s (± 0.54%) 5.11s (± 0.36%) -0.01s (- 0.21%) 5.06s 5.15s
Emit Time 5.91s (± 0.78%) 5.86s (± 0.42%) -0.05s (- 0.86%) 5.78s 5.89s
Total Time 13.70s (± 0.58%) 13.64s (± 0.27%) -0.06s (- 0.46%) 13.55s 13.72s
Compiler-Unions - node (v14.15.1, x64)
Memory used 189,144k (± 0.62%) 189,872k (± 0.50%) +727k (+ 0.38%) 187,272k 190,578k
Parse Time 0.81s (± 0.61%) 0.80s (± 0.74%) -0.00s (- 0.12%) 0.79s 0.82s
Bind Time 0.56s (± 0.59%) 0.56s (± 0.72%) -0.00s (- 0.18%) 0.55s 0.57s
Check Time 7.17s (± 0.33%) 7.13s (± 0.43%) -0.04s (- 0.53%) 7.08s 7.19s
Emit Time 2.29s (± 0.77%) 2.32s (± 2.22%) +0.02s (+ 0.96%) 2.27s 2.52s
Total Time 10.83s (± 0.33%) 10.81s (± 0.46%) -0.02s (- 0.18%) 10.72s 10.97s
Monaco - node (v14.15.1, x64)
Memory used 322,485k (± 0.01%) 322,530k (± 0.00%) +45k (+ 0.01%) 322,497k 322,556k
Parse Time 1.48s (± 0.91%) 1.48s (± 0.71%) +0.00s (+ 0.20%) 1.47s 1.51s
Bind Time 0.74s (± 0.80%) 0.75s (± 0.87%) +0.00s (+ 0.40%) 0.73s 0.76s
Check Time 5.19s (± 0.63%) 5.17s (± 0.64%) -0.02s (- 0.31%) 5.09s 5.22s
Emit Time 3.13s (± 0.85%) 3.11s (± 1.08%) -0.02s (- 0.54%) 3.06s 3.20s
Total Time 10.54s (± 0.39%) 10.51s (± 0.50%) -0.02s (- 0.23%) 10.41s 10.64s
TFS - node (v14.15.1, x64)
Memory used 287,397k (± 0.01%) 287,471k (± 0.01%) +74k (+ 0.03%) 287,428k 287,526k
Parse Time 1.27s (± 0.80%) 1.25s (± 0.53%) -0.02s (- 1.26%) 1.24s 1.27s
Bind Time 0.72s (± 0.81%) 0.72s (± 1.04%) +0.00s (+ 0.00%) 0.71s 0.74s
Check Time 4.81s (± 0.39%) 4.82s (± 0.50%) +0.01s (+ 0.12%) 4.78s 4.89s
Emit Time 3.29s (± 1.03%) 3.27s (± 0.61%) -0.02s (- 0.58%) 3.24s 3.33s
Total Time 10.09s (± 0.37%) 10.06s (± 0.25%) -0.03s (- 0.25%) 10.00s 10.12s
material-ui - node (v14.15.1, x64)
Memory used 448,218k (± 0.05%) 448,340k (± 0.04%) +121k (+ 0.03%) 447,631k 448,486k
Parse Time 1.75s (± 0.50%) 1.75s (± 0.63%) -0.00s (- 0.17%) 1.72s 1.77s
Bind Time 0.69s (± 0.32%) 0.69s (± 0.58%) +0.00s (+ 0.00%) 0.68s 0.70s
Check Time 12.84s (± 0.56%) 12.83s (± 0.47%) -0.01s (- 0.07%) 12.73s 12.94s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.28s (± 0.48%) 15.27s (± 0.41%) -0.01s (- 0.07%) 15.16s 15.37s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-206-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
Benchmark Name Iterations
Current 43370 10
Baseline main 10

Developer Information:

Download Benchmark

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 25, 2021

Some tests failed. But seems not my fault.

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 25, 2021

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 6234640. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 25, 2021

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/105559/artifacts?artifactName=tgz&fileId=BA047F25FA4C0D5193B0BA0847043F33E9205E79E02B5C9EADF397ADE79219B402&fileName=/typescript-4.4.0-insiders.20210625.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.4.0-pr-43370-43".;

@sedghi
Copy link

sedghi commented Aug 17, 2022

Is there any way to disable this with a flag? Previously in typescript 4.5.x my esm build was

image

But from 4.6.x I have a static block

image

Which apparently terser has problem, and also consuming apps don't recognize this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Proposal class static block
5 participants