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

getConstraintDeclaration gets the first declaration with a constraint… #33426

Conversation

weswigham
Copy link
Member

…, rather than just the first declaration

Fixes #33395

@HerringtonDarkholme if you're still watching, what was the original justification for this to be order dependent, anyway? You just commented that it was and it was just.... accepted. It leads to some odd and hard to understand behavior, as in #33395.

@HerringtonDarkholme
Copy link
Contributor

Thanks for letting me know! My OSS participation is bogged by day job and other stuffs in real life. But it's so nice to be mentioned here again!

The typical usage for skipping constraint check is that library author used to release a definition where an interface has no constraint, and later the author changed to impose constraint on type parameter. So library user won't break existing interface merging in which no constraint presents.

I remember I don't want to deal with order. Also, interface merging is used by user to augment library definition. In user side it is more likely to relax constraint than adding constraints. It is unlikely a more generic function will be "augmented" to accept fewer type parameter.

Lodash usage from #33395 (comment) isn't considered that time. This change should also allow changing interface from having constraint to no constraint.

@weswigham
Copy link
Member Author

This change should also allow changing interface from having constraint to no constraint.

Not quite - this change make sure such that if any declaration we found was constrained, the type parameter is seen as constrained. You can't erase the constraint (though if a definition is valid for a constraint-less type it's... usually... valid for a type with a constraint)

@weswigham weswigham merged commit 5e06bea into microsoft:master Sep 18, 2019
@weswigham weswigham deleted the remove-order-dependence-on-constraint-check branch September 18, 2019 20:56
@DanielRosenwasser
Copy link
Member

@typescript-bot cherry-pick this to release-3.6

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Sep 19, 2019
Component commits:
4ae62c3 getConstraintDeclaration gets the first declaration with a constraint, rather than just the first declaration

b1ad54b Add type annotation

2232f5e Update comment
@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, I've opened #33512 for you.

DanielRosenwasser added a commit that referenced this pull request Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TS 3.6 generates broken types references in .d.ts files
5 participants