GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix preserveSourceNewlines sibling node comparison (fixes extra newlines in organize imports) by andrewbranch · Pull Request #42630 · microsoft/TypeScript

Conversation

@andrewbranch

@andrewbranch Andrew Branch (andrewbranch) commented Feb 3, 2021

Copy link
Copy Markdown
Member

This is Armando Aguirre (@armanio123)’s #41927 with an extra two commits on top.

Fixes #41417.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
TypeScript Bot (typescript-bot) and others added 30 commits November 20, 2020 06:08
@typescript-bot
Update package-lock.json
cb05c28
@typescript-bot
Update package-lock.json
2f4d4c1
@typescript-bot
Update package-lock.json
b94544a
@typescript-bot
Update package-lock.json
60d63dd
@typescript-bot
Update package-lock.json
3d45ac0
@typescript-bot
Update package-lock.json
7dda7e3
@typescript-bot
Update package-lock.json
24f0d6b
@typescript-bot
Update package-lock.json
a9a8cbb
@typescript-bot
Update package-lock.json
9e42a69
@typescript-bot
Update package-lock.json
12cc980
@typescript-bot
Update package-lock.json
a43ec41
@typescript-bot
Update package-lock.json
3a6a36a
@typescript-bot
Update package-lock.json
202873a
@typescript-bot
Update package-lock.json
78ea60a
@typescript-bot
Update package-lock.json
2b0578d
@typescript-bot
Update package-lock.json
e7c832a
@typescript-bot
Update package-lock.json
62604bc
@typescript-bot
Update package-lock.json
4d807de
@typescript-bot
Update package-lock.json
dda079d
@typescript-bot
Update package-lock.json
9c121aa
@typescript-bot
Update package-lock.json
8777dc7
@typescript-bot
Update package-lock.json
f1aa0f9
@typescript-bot
Update package-lock.json
e4e4cf2
@typescript-bot
Update package-lock.json
dd79363
@armanio123
Merge remote-tracking branch 'upstream/master'
2e93e31
@andrewbranch @armanio123
More sophisticated check for source position comparability
c709f17
@armanio123
Merge branch 'master' of https://github.com/armanio123/TypeScript
67ad6b4
@armanio123
Fix organize imports by looking at the nodes positions
5215977
@armanio123
Merge remote-tracking branch 'upstream/master' into FixOrganizeImports
7cdbdb9
@armanio123
Rollback formatting changes
b51345e
@andrewbranch
Merge branch 'master' into FixOrganizeImports
a368a48
@typescript-bot TypeScript Bot (typescript-bot) assigned Andrew Branch (andrewbranch) Feb 3, 2021
@typescript-bot TypeScript Bot (typescript-bot) added the Author: Team label Feb 3, 2021
@typescript-bot TypeScript Bot (typescript-bot) assigned Armando Aguirre (armanio123) Feb 3, 2021
@typescript-bot TypeScript Bot (typescript-bot) added the For Milestone Bug PRs that fix a bug with a specific milestone label Feb 3, 2021
andrewbranch
Andrew Branch (andrewbranch) commented Feb 3, 2021
View reviewed changes
Comment thread tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js
//// [parserErrorRecovery_ObjectLiteral2.js]
var v = { a: a, "return": };
var v = { a: a,
"return": };

@andrewbranch Andrew Branch (andrewbranch) Feb 3, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

These JS baseline changes are reverting back to their original state pre-#36688. That PR was never really meant to change JS emit, but the couple changes that did occur seemed ok so we went with it. Since siblingNodePositionsAreComparable might do a little more work than what’s currently happening in master, I reverted the codepath that non-codefix/refactor emit takes back to the way it originally was.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@andrewbranch Andrew Branch (andrewbranch) Feb 9, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

siblingNodePositionsAreComparable is now quite cheap, but I think it still makes sense to keep it out of JS emit for now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@andrewbranch
Andrew Branch (andrewbranch) force-pushed the FixOrganizeImports branch from 5d49a2a to 76220f3 Compare February 3, 2021 23:54
@andrewbranch
Move getNodeAtPosition back
de698f9
@andrewbranch
Andrew Branch (andrewbranch) force-pushed the FixOrganizeImports branch from 76220f3 to de698f9 Compare February 3, 2021 23:55
@andrewbranch
Andrew Branch (andrewbranch) requested review from Armando Aguirre (armanio123) and Ron Buckton (rbuckton) February 4, 2021 00:01
rbuckton
Ron Buckton (rbuckton) reviewed Feb 4, 2021
View reviewed changes
Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@rbuckton

Ron Buckton (rbuckton) commented Feb 4, 2021

Copy link
Copy Markdown
Contributor

TypeScript Bot (@typescript-bot) perf test

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@andrewbranch
Andrew Branch (andrewbranch) requested a review from Ron Buckton (rbuckton) February 5, 2021 18:39
@andrewbranch
Optimize
77b1902
@rbuckton

Ron Buckton (rbuckton) commented Feb 6, 2021

Copy link
Copy Markdown
Contributor

Looks like the bot trigger for perf tests didn't pick up my command due to a typo in the regexp.

TypeScript Bot (@typescript-bot) perf test this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Feb 6, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Ron Buckton (@rbuckton), I've started to run the perf test suite on this PR at 77b1902. You can monitor the build here.

Update: The results are in!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Feb 6, 2021

Copy link
Copy Markdown
Contributor

Ron Buckton (@rbuckton)
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..42630

Metric master 42630 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 345,596k (± 0.01%) 345,661k (± 0.03%) +65k (+ 0.02%) 345,435k 345,850k
Parse Time 1.93s (± 0.76%) 1.91s (± 0.61%) -0.01s (- 0.62%) 1.88s 1.94s
Bind Time 0.83s (± 1.16%) 0.83s (± 1.35%) +0.00s (+ 0.24%) 0.81s 0.86s
Check Time 4.96s (± 0.64%) 4.97s (± 0.65%) +0.01s (+ 0.16%) 4.86s 5.03s
Emit Time 5.27s (± 0.81%) 5.30s (± 0.86%) +0.03s (+ 0.57%) 5.22s 5.41s
Total Time 12.98s (± 0.63%) 13.01s (± 0.54%) +0.03s (+ 0.20%) 12.82s 13.14s
Compiler-Unions - node (v10.16.3, x64)
Memory used 214,377k (± 0.08%) 214,459k (± 0.03%) +82k (+ 0.04%) 214,314k 214,611k
Parse Time 0.78s (± 0.61%) 0.78s (± 0.83%) +0.00s (+ 0.13%) 0.76s 0.79s
Bind Time 0.50s (± 1.45%) 0.50s (± 1.29%) 0.00s ( 0.00%) 0.49s 0.51s
Check Time 10.65s (± 0.55%) 10.68s (± 0.69%) +0.03s (+ 0.23%) 10.54s 10.83s
Emit Time 2.34s (± 1.94%) 2.33s (± 1.04%) -0.01s (- 0.21%) 2.28s 2.39s
Total Time 14.27s (± 0.63%) 14.29s (± 0.58%) +0.02s (+ 0.14%) 14.17s 14.52s
Monaco - node (v10.16.3, x64)
Memory used 355,241k (± 0.03%) 355,277k (± 0.03%) +36k (+ 0.01%) 354,974k 355,564k
Parse Time 1.55s (± 0.50%) 1.55s (± 0.66%) -0.00s (- 0.13%) 1.53s 1.57s
Bind Time 0.73s (± 1.04%) 0.72s (± 1.05%) -0.00s (- 0.14%) 0.71s 0.74s
Check Time 5.12s (± 0.66%) 5.12s (± 0.38%) +0.01s (+ 0.14%) 5.07s 5.16s
Emit Time 2.81s (± 1.08%) 2.78s (± 0.74%) -0.03s (- 1.07%) 2.75s 2.84s
Total Time 10.21s (± 0.44%) 10.18s (± 0.29%) -0.02s (- 0.24%) 10.14s 10.26s
TFS - node (v10.16.3, x64)
Memory used 308,147k (± 0.02%) 308,157k (± 0.01%) +10k (+ 0.00%) 308,093k 308,279k
Parse Time 1.21s (± 0.55%) 1.21s (± 0.66%) +0.00s (+ 0.17%) 1.20s 1.23s
Bind Time 0.68s (± 0.65%) 0.68s (± 0.81%) 0.00s ( 0.00%) 0.67s 0.70s
Check Time 4.60s (± 0.50%) 4.61s (± 0.58%) +0.01s (+ 0.28%) 4.57s 4.69s
Emit Time 2.96s (± 0.87%) 2.93s (± 1.37%) -0.03s (- 0.95%) 2.83s 3.00s
Total Time 9.45s (± 0.41%) 9.43s (± 0.59%) -0.02s (- 0.21%) 9.34s 9.60s
material-ui - node (v10.16.3, x64)
Memory used 495,587k (± 0.01%) 495,603k (± 0.01%) +16k (+ 0.00%) 495,475k 495,664k
Parse Time 1.98s (± 0.82%) 1.99s (± 0.78%) +0.00s (+ 0.20%) 1.96s 2.03s
Bind Time 0.66s (± 0.76%) 0.66s (± 0.98%) +0.00s (+ 0.46%) 0.65s 0.67s
Check Time 14.04s (± 0.81%) 14.02s (± 0.66%) -0.02s (- 0.14%) 13.83s 14.17s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.68s (± 0.75%) 16.67s (± 0.63%) -0.01s (- 0.07%) 16.44s 16.87s
Angular - node (v12.1.0, x64)
Memory used 323,221k (± 0.01%) 323,276k (± 0.02%) +55k (+ 0.02%) 323,184k 323,444k
Parse Time 1.91s (± 0.43%) 1.91s (± 0.68%) -0.01s (- 0.37%) 1.88s 1.93s
Bind Time 0.80s (± 0.93%) 0.80s (± 0.87%) +0.00s (+ 0.13%) 0.79s 0.82s
Check Time 4.85s (± 0.53%) 4.86s (± 0.56%) +0.01s (+ 0.27%) 4.82s 4.92s
Emit Time 5.47s (± 1.46%) 5.45s (± 1.42%) -0.02s (- 0.33%) 5.34s 5.69s
Total Time 13.03s (± 0.54%) 13.02s (± 0.56%) -0.01s (- 0.08%) 12.90s 13.22s
Compiler-Unions - node (v12.1.0, x64)
Memory used 199,817k (± 0.09%) 199,790k (± 0.07%) -27k (- 0.01%) 199,512k 200,137k
Parse Time 0.77s (± 0.91%) 0.77s (± 1.04%) -0.00s (- 0.39%) 0.75s 0.79s
Bind Time 0.50s (± 0.80%) 0.50s (± 0.68%) -0.00s (- 0.80%) 0.49s 0.50s
Check Time 9.72s (± 0.92%) 9.75s (± 0.79%) +0.03s (+ 0.32%) 9.54s 9.94s
Emit Time 2.35s (± 1.50%) 2.36s (± 1.59%) +0.01s (+ 0.51%) 2.30s 2.49s
Total Time 13.34s (± 0.69%) 13.37s (± 0.69%) +0.04s (+ 0.28%) 13.13s 13.53s
Monaco - node (v12.1.0, x64)
Memory used 337,612k (± 0.03%) 337,552k (± 0.02%) -60k (- 0.02%) 337,407k 337,628k
Parse Time 1.53s (± 0.78%) 1.53s (± 0.82%) +0.00s (+ 0.07%) 1.51s 1.57s
Bind Time 0.71s (± 0.85%) 0.71s (± 1.26%) +0.00s (+ 0.14%) 0.69s 0.73s
Check Time 4.93s (± 0.80%) 4.93s (± 0.75%) -0.01s (- 0.18%) 4.82s 5.01s
Emit Time 2.87s (± 0.98%) 2.85s (± 0.84%) -0.02s (- 0.77%) 2.81s 2.91s
Total Time 10.05s (± 0.58%) 10.02s (± 0.72%) -0.03s (- 0.30%) 9.87s 10.22s
TFS - node (v12.1.0, x64)
Memory used 292,323k (± 0.02%) 292,389k (± 0.02%) +65k (+ 0.02%) 292,273k 292,485k
Parse Time 1.23s (± 0.56%) 1.23s (± 0.98%) +0.00s (+ 0.08%) 1.20s 1.25s
Bind Time 0.66s (± 0.68%) 0.65s (± 0.56%) -0.00s (- 0.61%) 0.65s 0.66s
Check Time 4.51s (± 0.56%) 4.51s (± 0.53%) -0.01s (- 0.20%) 4.45s 4.55s
Emit Time 2.96s (± 1.94%) 2.91s (± 0.95%) -0.05s (- 1.69%) 2.84s 2.97s
Total Time 9.36s (± 0.80%) 9.30s (± 0.58%) -0.06s (- 0.62%) 9.17s 9.39s
material-ui - node (v12.1.0, x64)
Memory used 472,781k (± 0.01%) 472,674k (± 0.05%) -107k (- 0.02%) 471,732k 472,862k
Parse Time 2.00s (± 0.82%) 2.00s (± 0.38%) -0.01s (- 0.40%) 1.98s 2.01s
Bind Time 0.64s (± 0.69%) 0.64s (± 0.87%) 0.00s ( 0.00%) 0.63s 0.65s
Check Time 12.54s (± 0.61%) 12.58s (± 0.84%) +0.04s (+ 0.31%) 12.41s 12.80s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.19s (± 0.52%) 15.22s (± 0.73%) +0.03s (+ 0.19%) 15.05s 15.46s
Angular - node (v14.15.1, x64)
Memory used 321,864k (± 0.01%) 321,851k (± 0.01%) -14k (- 0.00%) 321,808k 321,904k
Parse Time 1.91s (± 0.49%) 1.90s (± 0.49%) -0.01s (- 0.31%) 1.89s 1.93s
Bind Time 0.85s (± 0.53%) 0.85s (± 0.47%) +0.00s (+ 0.35%) 0.84s 0.86s
Check Time 4.85s (± 0.39%) 4.86s (± 0.25%) +0.00s (+ 0.08%) 4.82s 4.88s
Emit Time 5.47s (± 0.20%) 5.47s (± 0.54%) -0.01s (- 0.15%) 5.39s 5.53s
Total Time 13.08s (± 0.15%) 13.08s (± 0.33%) -0.01s (- 0.05%) 12.99s 13.16s
Compiler-Unions - node (v14.15.1, x64)
Memory used 200,440k (± 0.56%) 200,757k (± 0.57%) +317k (+ 0.16%) 199,013k 202,940k
Parse Time 0.80s (± 0.60%) 0.80s (± 0.70%) +0.00s (+ 0.13%) 0.79s 0.81s
Bind Time 0.53s (± 0.76%) 0.53s (± 0.76%) 0.00s ( 0.00%) 0.52s 0.54s
Check Time 9.68s (± 0.78%) 9.68s (± 0.40%) -0.01s (- 0.06%) 9.60s 9.76s
Emit Time 2.36s (± 1.24%) 2.38s (± 2.18%) +0.03s (+ 1.15%) 2.30s 2.54s
Total Time 13.36s (± 0.64%) 13.39s (± 0.33%) +0.02s (+ 0.18%) 13.26s 13.46s
Monaco - node (v14.15.1, x64)
Memory used 336,824k (± 0.00%) 336,844k (± 0.01%) +20k (+ 0.01%) 336,771k 336,901k
Parse Time 1.57s (± 0.57%) 1.56s (± 0.74%) -0.01s (- 0.57%) 1.54s 1.60s
Bind Time 0.73s (± 1.06%) 0.73s (± 0.55%) -0.00s (- 0.27%) 0.72s 0.74s
Check Time 4.85s (± 0.47%) 4.86s (± 0.62%) +0.00s (+ 0.08%) 4.82s 4.96s
Emit Time 2.91s (± 0.68%) 2.93s (± 1.06%) +0.02s (+ 0.72%) 2.88s 3.04s
Total Time 10.06s (± 0.34%) 10.07s (± 0.53%) +0.01s (+ 0.09%) 10.01s 10.27s
TFS - node (v14.15.1, x64)
Memory used 291,545k (± 0.01%) 291,557k (± 0.01%) +12k (+ 0.00%) 291,532k 291,603k
Parse Time 1.25s (± 1.03%) 1.25s (± 2.13%) -0.00s (- 0.40%) 1.21s 1.34s
Bind Time 0.69s (± 0.86%) 0.69s (± 1.18%) -0.00s (- 0.58%) 0.68s 0.71s
Check Time 4.49s (± 0.36%) 4.49s (± 0.68%) -0.01s (- 0.18%) 4.44s 4.58s
Emit Time 3.04s (± 0.94%) 3.04s (± 0.94%) -0.00s (- 0.03%) 2.98s 3.10s
Total Time 9.48s (± 0.30%) 9.46s (± 0.60%) -0.02s (- 0.18%) 9.37s 9.62s
material-ui - node (v14.15.1, x64)
Memory used 471,451k (± 0.06%) 471,464k (± 0.05%) +12k (+ 0.00%) 470,464k 471,615k
Parse Time 2.06s (± 0.46%) 2.05s (± 0.90%) -0.01s (- 0.29%) 2.01s 2.09s
Bind Time 0.70s (± 0.67%) 0.70s (± 0.68%) -0.00s (- 0.43%) 0.69s 0.71s
Check Time 12.67s (± 1.39%) 12.59s (± 0.74%) -0.07s (- 0.59%) 12.40s 12.84s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.42s (± 1.15%) 15.34s (± 0.58%) -0.08s (- 0.52%) 15.16s 15.57s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-198-generic
Architecturex64
Available Memory16 GB
Available Memory8 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 42630 10
Baseline master 10

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rbuckton
Ron Buckton (rbuckton) suggested changes Feb 6, 2021
View reviewed changes
Comment thread src/compiler/emitter.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/utilities.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/utilitiesPublic.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Andrew Branch (andrewbranch) added 2 commits February 9, 2021 15:19
@andrewbranch
Use node array index check instead of tree walk
bcbec24
@andrewbranch
Revert unneeded change
af14aef
@andrewbranch
Andrew Branch (andrewbranch) requested a review from Ron Buckton (rbuckton) February 9, 2021 23:20
@andrewbranch Andrew Branch (andrewbranch) mentioned this pull request Feb 16, 2021
Organize Import creates whitespace that needs to be deleted manually #40736
Closed
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) approved these changes Feb 25, 2021
View reviewed changes
@andrewbranch
Merge branch 'master' into FixOrganizeImports
a0bf137
@andrewbranch
Andrew Branch (andrewbranch) merged commit 3c32f6e into microsoft:master Feb 26, 2021
@andrewbranch
Andrew Branch (andrewbranch) deleted the FixOrganizeImports branch February 26, 2021 18:37
@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Mar 10, 2021
Fix organize imports inserting extra blank lines #41927
Closed
@adeolemon Jordi Bunster (adeolemon) mentioned this pull request Mar 11, 2021
Organize all imports at once Azure/cosmos-explorer#537
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) RyanCavanaugh approved these changes
@armanio123 Armando Aguirre (armanio123) Awaiting requested review from armanio123
@rbuckton Ron Buckton (rbuckton) Awaiting requested review from rbuckton

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@andrewbranch Andrew Branch (andrewbranch)

@armanio123 Armando Aguirre (armanio123)

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.

Typescript "Organize Imports" action inserts loads of blank lines in import statements

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@andrewbranch @rbuckton @typescript-bot @RyanCavanaugh @armanio123
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.