Skip to content

Don't emit duplicate triple-slash directives when using API to print a .d.ts #40968

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 23, 2020
Merged

Don't emit duplicate triple-slash directives when using API to print a .d.ts #40968

merged 1 commit into from
Oct 23, 2020

Conversation

rbuckton
Copy link
Contributor

@rbuckton rbuckton commented Oct 6, 2020

We have a special case when we emit declaration files where we emit /// <reference ... directives stored on the SourceFile as a result of declaration transforms as part of the call to emitTripleSlashDirectivesIfNeeded. This was intended for when our compiler transforms a .ts to a .d.ts. However, if you use the createPrinter API directly to print a raw .d.ts file, we end up emitting the directives twice. Once from the call to emitTripleSlashDirectivesIfNeeded, and again from a later call to emitLeadingComments.

This PR changes emitLeadingComments to elide /// <reference ... directives from the top of a source file if that source file is a declaration file, which avoids the duplication.

Fixes #36242

@rbuckton rbuckton requested review from sandersn and weswigham October 6, 2020 22:09
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Oct 6, 2020
@rbuckton
Copy link
Contributor Author

rbuckton commented Oct 6, 2020

@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 Oct 6, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 6, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 6, 2020 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 6, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..40968

Metric master 40968 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 349,757k (± 0.02%) 349,748k (± 0.02%) -9k (- 0.00%) 349,640k 349,913k
Parse Time 2.01s (± 0.80%) 2.00s (± 0.89%) -0.01s (- 0.30%) 1.97s 2.06s
Bind Time 0.82s (± 1.07%) 0.82s (± 0.83%) +0.00s (+ 0.24%) 0.81s 0.84s
Check Time 4.94s (± 0.54%) 4.94s (± 0.55%) +0.00s (+ 0.06%) 4.89s 5.01s
Emit Time 5.19s (± 0.41%) 5.19s (± 0.56%) -0.00s (- 0.04%) 5.14s 5.25s
Total Time 12.96s (± 0.34%) 12.96s (± 0.46%) -0.01s (- 0.05%) 12.87s 13.12s
Monaco - node (v10.16.3, x64)
Memory used 354,351k (± 0.02%) 354,392k (± 0.02%) +41k (+ 0.01%) 354,167k 354,494k
Parse Time 1.57s (± 0.57%) 1.55s (± 0.22%) -0.01s (- 0.83%) 1.55s 1.56s
Bind Time 0.71s (± 0.56%) 0.72s (± 0.65%) +0.01s (+ 1.27%) 0.71s 0.73s
Check Time 5.07s (± 0.54%) 5.11s (± 0.62%) +0.04s (+ 0.71%) 5.00s 5.16s
Emit Time 2.76s (± 0.93%) 2.77s (± 0.55%) +0.00s (+ 0.11%) 2.73s 2.80s
Total Time 10.11s (± 0.37%) 10.15s (± 0.36%) +0.04s (+ 0.35%) 10.03s 10.21s
TFS - node (v10.16.3, x64)
Memory used 307,640k (± 0.04%) 307,637k (± 0.01%) -4k (- 0.00%) 307,563k 307,717k
Parse Time 1.22s (± 0.76%) 1.22s (± 0.48%) -0.00s (- 0.41%) 1.21s 1.23s
Bind Time 0.66s (± 1.37%) 0.67s (± 0.92%) +0.01s (+ 1.51%) 0.65s 0.68s
Check Time 4.57s (± 0.94%) 4.58s (± 0.57%) +0.01s (+ 0.22%) 4.54s 4.65s
Emit Time 2.90s (± 1.72%) 2.92s (± 1.18%) +0.02s (+ 0.66%) 2.83s 2.98s
Total Time 9.35s (± 0.59%) 9.38s (± 0.40%) +0.04s (+ 0.39%) 9.32s 9.48s
material-ui - node (v10.16.3, x64)
Memory used 489,125k (± 0.02%) 489,031k (± 0.01%) -94k (- 0.02%) 488,935k 489,115k
Parse Time 1.99s (± 0.54%) 1.98s (± 0.37%) -0.01s (- 0.65%) 1.96s 1.99s
Bind Time 0.65s (± 0.90%) 0.65s (± 0.86%) +0.00s (+ 0.15%) 0.63s 0.66s
Check Time 13.42s (± 0.51%) 13.47s (± 0.64%) +0.04s (+ 0.32%) 13.27s 13.70s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.06s (± 0.45%) 16.09s (± 0.54%) +0.03s (+ 0.19%) 15.87s 16.31s
Angular - node (v12.1.0, x64)
Memory used 326,934k (± 0.02%) 326,784k (± 0.08%) -149k (- 0.05%) 325,692k 327,062k
Parse Time 2.00s (± 0.53%) 1.99s (± 0.71%) -0.01s (- 0.30%) 1.97s 2.03s
Bind Time 0.81s (± 1.12%) 0.80s (± 0.45%) -0.00s (- 0.62%) 0.80s 0.81s
Check Time 4.83s (± 0.40%) 4.87s (± 1.16%) +0.03s (+ 0.68%) 4.80s 5.02s
Emit Time 5.39s (± 0.84%) 5.40s (± 0.54%) +0.00s (+ 0.06%) 5.32s 5.45s
Total Time 13.04s (± 0.52%) 13.06s (± 0.56%) +0.03s (+ 0.20%) 12.93s 13.24s
Monaco - node (v12.1.0, x64)
Memory used 336,578k (± 0.02%) 336,563k (± 0.02%) -15k (- 0.00%) 336,431k 336,680k
Parse Time 1.54s (± 0.72%) 1.53s (± 0.63%) -0.01s (- 0.46%) 1.51s 1.55s
Bind Time 0.69s (± 0.58%) 0.69s (± 0.80%) +0.00s (+ 0.44%) 0.68s 0.71s
Check Time 4.88s (± 0.50%) 4.89s (± 0.51%) +0.01s (+ 0.12%) 4.83s 4.94s
Emit Time 2.82s (± 0.91%) 2.82s (± 0.93%) -0.01s (- 0.21%) 2.77s 2.90s
Total Time 9.93s (± 0.39%) 9.92s (± 0.41%) -0.01s (- 0.06%) 9.83s 10.00s
TFS - node (v12.1.0, x64)
Memory used 291,933k (± 0.03%) 291,864k (± 0.02%) -70k (- 0.02%) 291,723k 291,996k
Parse Time 1.23s (± 0.49%) 1.24s (± 0.55%) +0.01s (+ 0.82%) 1.22s 1.25s
Bind Time 0.65s (± 1.38%) 0.64s (± 1.28%) -0.00s (- 0.31%) 0.63s 0.66s
Check Time 4.47s (± 0.30%) 4.49s (± 0.33%) +0.03s (+ 0.60%) 4.46s 4.53s
Emit Time 2.92s (± 0.99%) 2.93s (± 0.75%) +0.01s (+ 0.38%) 2.89s 2.99s
Total Time 9.26s (± 0.36%) 9.30s (± 0.35%) +0.04s (+ 0.46%) 9.24s 9.40s
material-ui - node (v12.1.0, x64)
Memory used 467,034k (± 0.05%) 467,187k (± 0.01%) +153k (+ 0.03%) 467,116k 467,271k
Parse Time 2.01s (± 0.37%) 2.01s (± 0.60%) -0.01s (- 0.30%) 1.99s 2.03s
Bind Time 0.64s (± 1.05%) 0.64s (± 0.70%) +0.00s (+ 0.16%) 0.63s 0.65s
Check Time 12.05s (± 0.94%) 12.05s (± 0.75%) +0.01s (+ 0.06%) 11.93s 12.35s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.70s (± 0.81%) 14.70s (± 0.70%) +0.00s (+ 0.01%) 14.54s 15.02s
Angular - node (v8.9.0, x64)
Memory used 346,442k (± 0.01%) 346,471k (± 0.02%) +29k (+ 0.01%) 346,290k 346,629k
Parse Time 2.54s (± 0.51%) 2.55s (± 0.34%) +0.00s (+ 0.12%) 2.53s 2.57s
Bind Time 0.85s (± 0.43%) 0.86s (± 0.34%) +0.01s (+ 0.94%) 0.86s 0.87s
Check Time 5.56s (± 0.52%) 5.60s (± 0.53%) +0.04s (+ 0.68%) 5.51s 5.63s
Emit Time 6.15s (± 1.07%) 5.99s (± 1.34%) -0.16s (- 2.65%) 5.75s 6.12s
Total Time 15.11s (± 0.55%) 14.99s (± 0.52%) -0.12s (- 0.79%) 14.77s 15.11s
Monaco - node (v8.9.0, x64)
Memory used 355,704k (± 0.02%) 355,740k (± 0.01%) +36k (+ 0.01%) 355,603k 355,848k
Parse Time 1.89s (± 0.77%) 1.88s (± 0.45%) -0.00s (- 0.27%) 1.87s 1.90s
Bind Time 0.89s (± 0.45%) 0.89s (± 0.41%) -0.01s (- 0.56%) 0.88s 0.89s
Check Time 5.62s (± 0.37%) 5.64s (± 0.49%) +0.02s (+ 0.34%) 5.60s 5.70s
Emit Time 3.29s (± 1.17%) 3.23s (± 0.46%) -0.05s (- 1.67%) 3.20s 3.27s
Total Time 11.69s (± 0.28%) 11.64s (± 0.29%) -0.05s (- 0.40%) 11.56s 11.72s
TFS - node (v8.9.0, x64)
Memory used 309,391k (± 0.02%) 309,366k (± 0.01%) -26k (- 0.01%) 309,305k 309,417k
Parse Time 1.55s (± 0.64%) 1.55s (± 0.38%) -0.01s (- 0.45%) 1.53s 1.56s
Bind Time 0.68s (± 0.73%) 0.67s (± 0.89%) -0.00s (- 0.15%) 0.66s 0.68s
Check Time 5.31s (± 0.58%) 5.29s (± 0.39%) -0.02s (- 0.32%) 5.24s 5.33s
Emit Time 2.94s (± 0.34%) 2.92s (± 0.84%) -0.02s (- 0.75%) 2.88s 2.98s
Total Time 10.48s (± 0.34%) 10.43s (± 0.31%) -0.05s (- 0.49%) 10.35s 10.52s
material-ui - node (v8.9.0, x64)
Memory used 493,431k (± 0.01%) 493,530k (± 0.01%) +99k (+ 0.02%) 493,424k 493,655k
Parse Time 2.40s (± 0.46%) 2.41s (± 0.34%) +0.00s (+ 0.12%) 2.39s 2.43s
Bind Time 0.81s (± 1.02%) 0.80s (± 1.37%) -0.02s (- 1.97%) 0.78s 0.82s
Check Time 17.95s (± 0.69%) 17.94s (± 0.88%) -0.01s (- 0.04%) 17.55s 18.22s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.17s (± 0.56%) 21.15s (± 0.74%) -0.02s (- 0.11%) 20.78s 21.43s
Angular - node (v8.9.0, x86)
Memory used 198,690k (± 0.01%) 198,681k (± 0.03%) -9k (- 0.00%) 198,549k 198,756k
Parse Time 2.48s (± 1.02%) 2.46s (± 0.49%) -0.02s (- 0.65%) 2.43s 2.49s
Bind Time 1.00s (± 1.00%) 1.01s (± 0.64%) +0.00s (+ 0.50%) 0.99s 1.02s
Check Time 5.05s (± 0.56%) 5.03s (± 0.45%) -0.02s (- 0.42%) 4.95s 5.07s
Emit Time 5.89s (± 0.83%) 5.88s (± 0.64%) -0.01s (- 0.10%) 5.83s 6.00s
Total Time 14.42s (± 0.58%) 14.38s (± 0.32%) -0.04s (- 0.26%) 14.28s 14.49s
Monaco - node (v8.9.0, x86)
Memory used 201,493k (± 0.01%) 201,486k (± 0.02%) -7k (- 0.00%) 201,386k 201,585k
Parse Time 1.94s (± 1.05%) 1.92s (± 1.32%) -0.02s (- 1.18%) 1.87s 2.00s
Bind Time 0.71s (± 0.47%) 0.70s (± 0.48%) -0.01s (- 0.99%) 0.70s 0.71s
Check Time 5.47s (± 0.38%) 5.56s (± 1.75%) +0.09s (+ 1.66%) 5.41s 5.75s
Emit Time 3.06s (± 0.59%) 2.87s (± 4.54%) 🟩-0.18s (- 5.99%) 2.67s 3.06s
Total Time 11.18s (± 0.25%) 11.05s (± 0.50%) -0.12s (- 1.11%) 10.93s 11.17s
TFS - node (v8.9.0, x86)
Memory used 176,867k (± 0.03%) 176,883k (± 0.03%) +16k (+ 0.01%) 176,777k 176,973k
Parse Time 1.60s (± 1.10%) 1.59s (± 0.77%) -0.01s (- 0.87%) 1.57s 1.63s
Bind Time 0.66s (± 2.04%) 0.64s (± 0.47%) -0.02s (- 2.60%) 0.63s 0.64s
Check Time 4.82s (± 0.58%) 4.83s (± 0.93%) +0.01s (+ 0.15%) 4.72s 4.94s
Emit Time 2.80s (± 0.66%) 2.82s (± 0.55%) +0.01s (+ 0.50%) 2.79s 2.85s
Total Time 9.88s (± 0.51%) 9.88s (± 0.57%) -0.01s (- 0.09%) 9.72s 10.00s
material-ui - node (v8.9.0, x86)
Memory used 277,878k (± 0.02%) 277,863k (± 0.02%) -15k (- 0.01%) 277,735k 277,960k
Parse Time 2.46s (± 0.43%) 2.47s (± 0.82%) +0.01s (+ 0.41%) 2.41s 2.50s
Bind Time 0.69s (± 1.35%) 0.70s (± 2.70%) +0.01s (+ 1.31%) 0.67s 0.76s
Check Time 16.45s (± 0.73%) 16.56s (± 0.93%) +0.11s (+ 0.66%) 16.27s 16.92s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.60s (± 0.63%) 19.73s (± 0.81%) +0.13s (+ 0.66%) 19.40s 20.08s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory3 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 40968 10
Baseline master 10

@typescript-bot typescript-bot mentioned this pull request Oct 6, 2020
Open
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@rbuckton
Copy link
Contributor Author

@DanielRosenwasser any issues with taking this for 4.1? It shouldn't affect any existing code since this is a very specific API-usage use case that was broken.

@DanielRosenwasser
Copy link
Member

It sounds acceptable and low-risk enough for the RC.

@rbuckton rbuckton merged commit 8ed645a into master Oct 23, 2020
@rbuckton rbuckton deleted the fix36242 branch October 23, 2020 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandersn sandersn sandersn approved these changes

@weswigham weswigham weswigham approved these changes

Assignees

@sandersn sandersn

@rbuckton rbuckton

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Triple-slash references duplicated in .d.ts with printer.printFile
5 participants
@rbuckton @typescript-bot @DanielRosenwasser @sandersn @weswigham