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

Add deprecated related feature #38523

Merged
merged 45 commits into from
Jun 19, 2020
Merged

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented May 13, 2020

Fixes #390
Fixes #33092
Fixes #33093

After read the codebase, I found below things need todo:

  • completions with symbol tag

  • workspace symbol with symbol tag

  • document symbol with symbol tag

  • call hierarchy with symbol tag

  • DiagnosticTags

current state:
222

TIM截图20200514183407
TIM截图20200514183420
TIM截图20200514184807
TIM截图20200518150229

image

Others need to do:

  • protocol
  • reports deprecated diag
  • confirm where's the line
  • confirm the place where report suggestion is correct
  • tests

@typescript-bot
Copy link
Collaborator

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

@Kingwl
Copy link
Contributor Author

Kingwl commented May 13, 2020

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 13, 2020

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

@Kingwl Kingwl changed the title Add deprecated related feature [Expirement] [WIP] Add deprecated related feature May 13, 2020
@Kingwl Kingwl mentioned this pull request May 13, 2020
@Kingwl
Copy link
Contributor Author

Kingwl commented May 13, 2020

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 13, 2020

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 13, 2020

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/73795/artifacts?artifactName=tgz&fileId=744B1D71C0DD25EAC199979B809F997229EB4AB80010D5B25D42E3374ABD21EE02&fileName=/typescript-4.0.0-insiders.20200513.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@Kingwl
Copy link
Contributor Author

Kingwl commented May 14, 2020

Ping, need some help. Please take a look
@sheetalkamat, @amcasey, @mjbvz, @minestarks, @DanielRosenwasser

@Kingwl
Copy link
Contributor Author

Kingwl commented May 14, 2020

Another problem is, where's the line:

a.b.c.d // c is deprecated

What is the expected behavior?

  1. [a.b.c].d
  2. a.b.[c].d
  3. a.b.[c.d]

And if:

a.b.c.d // c and d is deprecated

What is the expected behavior

  1. [a.b.c.d]
  2. a.b.[c].[d]
  3. a.b.[c.d]

@amcasey
Copy link
Member

amcasey commented May 14, 2020

@Kingwl, sorry, I'm not sure I understand your question. Are you asking about the design of the features? If so, you probably want @sandersn.

@sandersn sandersn self-assigned this May 14, 2020
@Kingwl
Copy link
Contributor Author

Kingwl commented May 15, 2020

Does the protocol looks good?I'm not sure🤷‍♂️

@spahnke
Copy link

spahnke commented May 17, 2020

I believe this would also fix #33093

@Kingwl
Copy link
Contributor Author

Kingwl commented May 18, 2020

Finally, It passed tests. I'll add more tests soon.
@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 18, 2020

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 18, 2020

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/74212/artifacts?artifactName=tgz&fileId=2056AC579024F64F4D733979778E0700E6D317EE140EA5F9D238E4B88C38090F02&fileName=/typescript-4.0.0-insiders.20200518.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@@ -533,6 +534,7 @@ namespace ts {
}
}

symbol.isDeprecated = symbol.isDeprecated || isDeprecated;
Copy link
Member

Choose a reason for hiding this comment

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

SHould this be SymbolFlags instead?

Copy link
Member

Choose a reason for hiding this comment

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

I agree -- if there's space there. I think the limit is 29 so there's 1 left.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, making this a ModifierFlag would go well with public/@public et al. It would also go better with getNodeModifiers in services.

@sandersn sandersn added this to Not started in PR Backlog May 19, 2020
@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 18, 2020

@typescript-bot perf test.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 18, 2020

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..38523

Metric master 38523 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 340,211k (± 0.02%) 339,838k (± 0.02%) -373k (- 0.11%) 339,673k 339,959k
Parse Time 1.98s (± 0.59%) 1.98s (± 0.70%) +0.00s (+ 0.00%) 1.94s 2.01s
Bind Time 0.81s (± 1.32%) 0.80s (± 0.59%) -0.00s (- 0.37%) 0.79s 0.81s
Check Time 4.68s (± 0.56%) 4.71s (± 0.57%) +0.02s (+ 0.51%) 4.65s 4.79s
Emit Time 5.21s (± 0.79%) 5.18s (± 0.41%) -0.03s (- 0.52%) 5.13s 5.22s
Total Time 12.67s (± 0.48%) 12.66s (± 0.38%) -0.01s (- 0.05%) 12.57s 12.79s
Monaco - node (v10.16.3, x64)
Memory used 338,724k (± 0.01%) 338,774k (± 0.02%) +50k (+ 0.01%) 338,667k 338,949k
Parse Time 1.55s (± 0.69%) 1.55s (± 0.62%) -0.00s (- 0.19%) 1.52s 1.57s
Bind Time 0.69s (± 0.58%) 0.69s (± 0.69%) -0.00s (- 0.29%) 0.68s 0.70s
Check Time 4.85s (± 0.56%) 4.84s (± 0.49%) -0.01s (- 0.12%) 4.78s 4.88s
Emit Time 2.75s (± 0.49%) 2.73s (± 0.55%) -0.01s (- 0.55%) 2.70s 2.76s
Total Time 9.83s (± 0.38%) 9.80s (± 0.23%) -0.03s (- 0.25%) 9.73s 9.85s
TFS - node (v10.16.3, x64)
Memory used 301,536k (± 0.02%) 301,848k (± 0.03%) +312k (+ 0.10%) 301,663k 302,032k
Parse Time 1.21s (± 1.09%) 1.21s (± 0.91%) -0.00s (- 0.08%) 1.18s 1.22s
Bind Time 0.64s (± 1.47%) 0.65s (± 1.39%) +0.01s (+ 0.78%) 0.62s 0.66s
Check Time 4.35s (± 0.66%) 4.35s (± 0.44%) +0.00s (+ 0.07%) 4.32s 4.40s
Emit Time 2.85s (± 0.97%) 2.87s (± 1.34%) +0.02s (+ 0.56%) 2.77s 2.94s
Total Time 9.04s (± 0.58%) 9.07s (± 0.53%) +0.03s (+ 0.29%) 8.95s 9.21s
material-ui - node (v10.16.3, x64)
Memory used 459,155k (± 0.02%) 459,128k (± 0.01%) -26k (- 0.01%) 459,009k 459,276k
Parse Time 2.02s (± 0.52%) 2.03s (± 0.52%) +0.00s (+ 0.15%) 2.01s 2.05s
Bind Time 0.64s (± 1.32%) 0.65s (± 1.35%) +0.01s (+ 2.19%) 0.64s 0.67s
Check Time 12.80s (± 0.71%) 12.85s (± 0.57%) +0.05s (+ 0.43%) 12.69s 13.00s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.46s (± 0.63%) 15.53s (± 0.54%) +0.07s (+ 0.47%) 15.38s 15.72s
Angular - node (v12.1.0, x64)
Memory used 317,588k (± 0.03%) 317,116k (± 0.09%) -472k (- 0.15%) 316,034k 317,351k
Parse Time 1.95s (± 0.39%) 1.93s (± 0.75%) -0.02s (- 1.28%) 1.90s 1.96s
Bind Time 0.78s (± 0.57%) 0.78s (± 0.67%) -0.00s (- 0.13%) 0.77s 0.79s
Check Time 4.57s (± 0.58%) 4.59s (± 0.63%) +0.02s (+ 0.35%) 4.53s 4.65s
Emit Time 5.32s (± 0.53%) 5.31s (± 1.00%) -0.01s (- 0.23%) 5.21s 5.46s
Total Time 12.63s (± 0.37%) 12.61s (± 0.68%) -0.03s (- 0.20%) 12.45s 12.84s
Monaco - node (v12.1.0, x64)
Memory used 321,236k (± 0.02%) 321,314k (± 0.02%) +79k (+ 0.02%) 321,184k 321,497k
Parse Time 1.52s (± 0.60%) 1.51s (± 1.00%) -0.01s (- 0.66%) 1.48s 1.54s
Bind Time 0.67s (± 0.60%) 0.68s (± 1.01%) +0.00s (+ 0.60%) 0.66s 0.69s
Check Time 4.65s (± 0.44%) 4.63s (± 0.37%) -0.02s (- 0.37%) 4.60s 4.66s
Emit Time 2.81s (± 0.44%) 2.80s (± 0.58%) -0.02s (- 0.53%) 2.76s 2.83s
Total Time 9.65s (± 0.28%) 9.61s (± 0.18%) -0.04s (- 0.38%) 9.57s 9.65s
TFS - node (v12.1.0, x64)
Memory used 285,983k (± 0.02%) 286,294k (± 0.02%) +311k (+ 0.11%) 286,197k 286,394k
Parse Time 1.22s (± 0.79%) 1.23s (± 1.02%) +0.00s (+ 0.33%) 1.20s 1.26s
Bind Time 0.61s (± 0.97%) 0.62s (± 1.33%) +0.01s (+ 1.63%) 0.60s 0.64s
Check Time 4.25s (± 0.54%) 4.25s (± 0.39%) +0.00s (+ 0.12%) 4.22s 4.30s
Emit Time 2.90s (± 0.69%) 2.91s (± 1.02%) +0.00s (+ 0.07%) 2.83s 2.97s
Total Time 8.99s (± 0.43%) 9.00s (± 0.44%) +0.02s (+ 0.20%) 8.90s 9.09s
material-ui - node (v12.1.0, x64)
Memory used 437,736k (± 0.02%) 437,626k (± 0.01%) -110k (- 0.03%) 437,520k 437,782k
Parse Time 1.99s (± 0.58%) 2.01s (± 0.76%) +0.02s (+ 0.95%) 1.99s 2.05s
Bind Time 0.63s (± 0.98%) 0.63s (± 1.05%) -0.00s (- 0.16%) 0.61s 0.64s
Check Time 11.45s (± 0.52%) 11.45s (± 0.56%) +0.00s (+ 0.04%) 11.34s 11.66s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.07s (± 0.41%) 14.09s (± 0.42%) +0.02s (+ 0.16%) 14.01s 14.28s
Angular - node (v8.9.0, x64)
Memory used 336,631k (± 0.02%) 336,369k (± 0.02%) -262k (- 0.08%) 336,259k 336,489k
Parse Time 2.49s (± 0.57%) 2.49s (± 0.73%) +0.00s (+ 0.12%) 2.46s 2.54s
Bind Time 0.83s (± 0.60%) 0.83s (± 0.70%) +0.01s (+ 0.97%) 0.82s 0.84s
Check Time 5.31s (± 0.50%) 5.36s (± 0.60%) +0.05s (+ 0.88%) 5.27s 5.44s
Emit Time 5.90s (± 1.04%) 5.97s (± 1.06%) +0.07s (+ 1.20%) 5.83s 6.10s
Total Time 14.53s (± 0.46%) 14.65s (± 0.62%) +0.13s (+ 0.87%) 14.49s 14.85s
Monaco - node (v8.9.0, x64)
Memory used 340,066k (± 0.01%) 340,145k (± 0.01%) +79k (+ 0.02%) 340,063k 340,233k
Parse Time 1.85s (± 0.57%) 1.85s (± 0.48%) +0.00s (+ 0.22%) 1.84s 1.88s
Bind Time 0.86s (± 0.46%) 0.86s (± 0.52%) +0.00s (+ 0.12%) 0.85s 0.87s
Check Time 5.33s (± 0.38%) 5.33s (± 0.53%) +0.00s (+ 0.08%) 5.29s 5.39s
Emit Time 3.21s (± 0.53%) 3.20s (± 0.54%) -0.01s (- 0.19%) 3.17s 3.24s
Total Time 11.25s (± 0.24%) 11.25s (± 0.42%) +0.00s (+ 0.03%) 11.15s 11.36s
TFS - node (v8.9.0, x64)
Memory used 303,226k (± 0.01%) 303,498k (± 0.01%) +272k (+ 0.09%) 303,431k 303,576k
Parse Time 1.54s (± 0.26%) 1.54s (± 0.50%) +0.00s (+ 0.00%) 1.53s 1.56s
Bind Time 0.65s (± 0.76%) 0.65s (± 0.53%) +0.00s (+ 0.15%) 0.64s 0.65s
Check Time 4.96s (± 1.74%) 4.93s (± 1.80%) -0.03s (- 0.54%) 4.81s 5.13s
Emit Time 3.05s (± 3.38%) 3.06s (± 3.18%) +0.01s (+ 0.46%) 2.81s 3.24s
Total Time 10.19s (± 0.61%) 10.19s (± 0.53%) -0.01s (- 0.06%) 10.09s 10.33s
material-ui - node (v8.9.0, x64)
Memory used 463,424k (± 0.01%) 463,232k (± 0.01%) -191k (- 0.04%) 463,123k 463,335k
Parse Time 2.35s (± 0.51%) 2.37s (± 0.49%) +0.01s (+ 0.55%) 2.34s 2.39s
Bind Time 0.76s (± 0.81%) 0.77s (± 0.87%) +0.01s (+ 0.79%) 0.76s 0.78s
Check Time 16.97s (± 0.50%) 16.94s (± 0.88%) -0.03s (- 0.17%) 16.61s 17.28s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 20.08s (± 0.42%) 20.07s (± 0.71%) -0.01s (- 0.05%) 19.77s 20.40s
Angular - node (v8.9.0, x86)
Memory used 193,160k (± 0.01%) 193,040k (± 0.02%) -120k (- 0.06%) 192,942k 193,128k
Parse Time 2.42s (± 0.70%) 2.41s (± 0.65%) -0.01s (- 0.37%) 2.38s 2.45s
Bind Time 0.97s (± 0.57%) 0.97s (± 0.85%) +0.00s (+ 0.21%) 0.95s 0.99s
Check Time 4.78s (± 0.54%) 4.78s (± 0.55%) +0.00s (+ 0.02%) 4.73s 4.84s
Emit Time 5.98s (± 1.03%) 5.88s (± 1.07%) -0.10s (- 1.70%) 5.71s 5.98s
Total Time 14.15s (± 0.57%) 14.04s (± 0.43%) -0.11s (- 0.76%) 13.85s 14.17s
Monaco - node (v8.9.0, x86)
Memory used 193,139k (± 0.02%) 193,179k (± 0.01%) +40k (+ 0.02%) 193,126k 193,241k
Parse Time 1.89s (± 1.30%) 1.89s (± 0.68%) -0.01s (- 0.37%) 1.85s 1.91s
Bind Time 0.68s (± 0.59%) 0.68s (± 0.70%) -0.00s (- 0.29%) 0.67s 0.69s
Check Time 5.40s (± 1.97%) 5.45s (± 0.37%) +0.05s (+ 0.96%) 5.41s 5.50s
Emit Time 2.76s (± 3.52%) 2.67s (± 0.82%) 🟩-0.09s (- 3.23%) 2.62s 2.73s
Total Time 10.72s (± 0.53%) 10.68s (± 0.24%) -0.04s (- 0.40%) 10.63s 10.73s
TFS - node (v8.9.0, x86)
Memory used 173,321k (± 0.02%) 173,512k (± 0.02%) +191k (+ 0.11%) 173,448k 173,642k
Parse Time 1.58s (± 1.03%) 1.58s (± 0.73%) -0.01s (- 0.38%) 1.55s 1.60s
Bind Time 0.62s (± 1.08%) 0.61s (± 0.73%) -0.00s (- 0.81%) 0.60s 0.62s
Check Time 4.61s (± 0.44%) 4.61s (± 0.68%) 0.00s ( 0.00%) 4.56s 4.70s
Emit Time 2.78s (± 1.33%) 2.78s (± 1.18%) +0.00s (+ 0.14%) 2.70s 2.88s
Total Time 9.59s (± 0.43%) 9.59s (± 0.54%) -0.01s (- 0.08%) 9.49s 9.70s
material-ui - node (v8.9.0, x86)
Memory used 262,246k (± 0.02%) 262,174k (± 0.02%) -72k (- 0.03%) 262,061k 262,249k
Parse Time 2.42s (± 0.62%) 2.43s (± 0.52%) +0.00s (+ 0.21%) 2.39s 2.45s
Bind Time 0.66s (± 1.34%) 0.66s (± 0.89%) +0.01s (+ 1.07%) 0.65s 0.68s
Check Time 15.48s (± 0.68%) 15.46s (± 0.91%) -0.02s (- 0.13%) 15.22s 15.95s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 18.56s (± 0.64%) 18.56s (± 0.74%) -0.01s (- 0.03%) 18.32s 19.02s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-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 (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 38523 10
Baseline master 10

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

So to recap: we now have NodeFlags.Deprecated, SymbolFlags.Deprecated and ModifierFlags.Deprecated.

I would say we are well-positioned to deprecate things now. =)

PR Backlog automation moved this from Waiting on author to Needs merge Jun 18, 2020
@sandersn sandersn merged commit 59ad375 into microsoft:master Jun 19, 2020
PR Backlog automation moved this from Needs merge to Done Jun 19, 2020
@sandersn
Copy link
Member

It's in! Thanks so much for your persistence @Kingwl!

@G-Rath
Copy link

G-Rath commented Jun 19, 2020

I've not followed this feature much, so while I've done a quick read over the discussions, linked tickets & searched the issue tracker I still could have missed something or misunderstood entirely, but just in case:

Has anyone raised the behaviour of removeComments in regards to this feature? I know a few libraries that use that flag, and so this feature is a bit moot if it removes @deprecated comments.

Happy to open a new issue for discussion.


Just found #14619, which would resolve this problem.

@Kingwl Kingwl deleted the deprecated_support branch June 19, 2020 02:57
@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 19, 2020

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 19, 2020

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

@brabenetz
Copy link

brabenetz commented Aug 31, 2020

Does it works with a comment which explain why something is deprecated?
I saw not a singe example with a comment!?
Like:

/** @deprecated replaced by new method .... */
public myOldMethod() {
...

VSCode should than show that message.
And tslint should enforce a comment.

lukmccall pushed a commit to expo/expo that referenced this pull request Sep 1, 2020
# Why

_This follows the TS 4.0 update PR: #9960_

Typescript 4.0 comes with support for `@deprecated` annotations, which gives nice VS Code (and probably other IDE) messages informing users about API deprecation. It also marks deprecated methods/properties with ~~Strikethrough~~.

See [this](https://devblogs.microsoft.com/typescript/announcing-typescript-4-0/#deprecated-support) and microsoft/TypeScript#38523

# How

- Added appropriate annotations to every possible deprecated Expo API that I found.
- Updated existing comments to play nice with VS Code messages

Also, added note about dead code in Notifications, which is related to #9563 (I forgot to add it then)

> Not sure what to do with changelogs ❓ 

# Test Plan

Tested messages in VS Code with _TS 4.0.2_ enabled.
@Kingwl
Copy link
Contributor Author

Kingwl commented Sep 1, 2020

image

Seems work for me

Jamedjo pushed a commit to Jamedjo/expo that referenced this pull request Feb 4, 2021
# Why

_This follows the TS 4.0 update PR: #9960_

Typescript 4.0 comes with support for `@deprecated` annotations, which gives nice VS Code (and probably other IDE) messages informing users about API deprecation. It also marks deprecated methods/properties with ~~Strikethrough~~.

See [this](https://devblogs.microsoft.com/typescript/announcing-typescript-4-0/#deprecated-support) and microsoft/TypeScript#38523

# How

- Added appropriate annotations to every possible deprecated Expo API that I found.
- Updated existing comments to play nice with VS Code messages

Also, added note about dead code in Notifications, which is related to expo#9563 (I forgot to add it then)

> Not sure what to do with changelogs ❓ 

# Test Plan

Tested messages in VS Code with _TS 4.0.2_ enabled.
@paulyoung
Copy link

Is there a way to suppress this warning using // eslint-disable-next-line or similar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experiment A fork with an experimental idea which might not make it into master For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Render uses of deprecated symbols Mark deprecated suggestion @‌deprecated JSDoc tag