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.

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

Revert this-parameter filtering for methods #31349

Closed
DanielRosenwasser opened this issue May 10, 2019 · 5 comments · Fixed by #31377
Closed

Revert this-parameter filtering for methods #31349

DanielRosenwasser opened this issue May 10, 2019 · 5 comments · Fixed by #31377
Assignees
@ahejlsberg
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 10, 2019 •
edited
Loading

We believe #20616 may be the cause of some recent slowness in editor completions. We are considering reverting the feature to increase performance (admittedly at the expense of some editor UX).
@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels May 10, 2019
@DanielRosenwasser
Copy link
Member Author

Removing the logic doesn't seem to do much from what we've seen. Completions are still ridiculously slow.

@ahejlsberg
Copy link
Member

See #31354 for the real fix.

@ahejlsberg
Copy link
Member

I'm reopening this issue. Turns out this-parameter filtering is also a contributor to the slowdown, it's just that the eager conditional type true/false branch resolution is the main offender. I have verified that removing the this-parameter filtering greatly speeds up completions with styled-components. We basically need both fixes.

@ahejlsberg ahejlsberg reopened this May 13, 2019
@DanielRosenwasser
Copy link
Member Author

Understandable if we have to, but it would be unfortunate. Libraries like fp-ts v2 would be affected.

@mattiamanzati
Copy link

Is it really worth it? Will some flag to turn this on be present? DX will suffer a lot :/

@ahejlsberg ahejlsberg mentioned this issue May 13, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ahejlsberg ahejlsberg

Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Revert this-parameter filtering in completions microsoft/TypeScript
3 participants
@DanielRosenwasser @ahejlsberg @mattiamanzati