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

Handle when associated name can be undefined #41285

Closed
wants to merge 1 commit into from
Closed

Handle when associated name can be undefined #41285

wants to merge 1 commit into from

Conversation

sheetalkamat
Copy link
Member

Fixes #40251

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Oct 27, 2020
weswigham
weswigham approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

associatedNames being present-but-empty/differing-in-length-from-the-types seems like a smell that should be fixed elsewhere, but I'll accept this as a fix.

weswigham
weswigham reviewed Oct 27, 2020
View reviewed changes
tests/baselines/reference/parameterNamesInRestParameter.errors.txt
~~~~~~~~
!!! error TS2684: The 'this' context of type '<T>(...items: T[]) => T[]' is not assignable to method's 'this' of type '(this: () => void) => unknown[]'.
!!! error TS2684: Types of parameters 'items' and 'args_0' are incompatible.
!!! error TS2684: Type 'any' is not assignable to type 'never'.
Copy link
Member

Choose a reason for hiding this comment

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

....? This error seems.... odd. Why are we even comparing non-this parameter types (one of the signatures doesn't have any non-this parameters)?

ahejlsberg
ahejlsberg reviewed Oct 28, 2020
View reviewed changes
Copy link
Member

@ahejlsberg ahejlsberg left a comment

Choose a reason for hiding this comment

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

I don't think this is the right fix. The issue here is that we're asking for a parameter name with an out of bounds index, which shouldn't happen. The real fix is to make sure it doesn't. I will add more info in the original issue and put up a new PR.

@ahejlsberg
Copy link
Member

I have put up a new fix in #41308.

@sheetalkamat sheetalkamat deleted the parameterName branch October 28, 2020 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weswigham weswigham weswigham approved these changes

@ahejlsberg ahejlsberg ahejlsberg left review comments

Assignees

@sheetalkamat sheetalkamat

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AllowJs + ES6 lib result in compiler crash
4 participants
@sheetalkamat @ahejlsberg @weswigham @typescript-bot