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 — Stricter generic signature checks by ahejlsberg · Pull Request #16368 · microsoft/TypeScript

Conversation

@ahejlsberg

@ahejlsberg Anders Hejlsberg (ahejlsberg) commented Jun 8, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This PR complements #16305 by implementing stricter checking of type relationships for generic signatures. Previously, when checking type relationships for generic signatures we would always first erase type parameters (by substituting type any for the type parameters). Now, we properly unify the type parameters by first inferring from the target signature to the source signature and then instantiating the source signature with the inferences. For example:

type A = <T, U>(x: T, y: U) => [T, U];
type B = <S>(x: S, y: S) => [S, S];

function f(a: A, b: B) {
    a = b;  // Error
    b = a;  // Ok
}

Previously, no errors were reported above because S, T, and U were replaced by any, causing the signatures to become identical. Now, once we unify the type parameters we can correctly determine that A is assignable to B, but not vice versa. Specifically, in the first assignment we infer T | U for S which after instantiation yields a signature that isn't assignable to A, whereas in the second assignment we infer S for each of T and U which after instantiation yields the same signature as B.

Note that we perform unification only when the source and target types each have a single signature. When either the source or target has multiple signatures (i.e. overloads) we still resort to type parameter erasure as it otherwise becomes prohibitively expensive to determine the relationships.

This PR is technically a breaking change as we now uncover errors we previously wouldn't catch. For this reason we have included a --noStrictGenericChecks compiler option to disable the stricter generic signature checks. Existing projects can use this as a stopgap solution until errors resulting from the stricter checking are corrected.

Fixes #138, #3410, #5616.

Sorry, something went wrong.

Uh oh!

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

zpdDG4gta8XKpMCd, krryan, gcnew, alitaheri, HerringtonDarkholme, jovdb, antogyn, SlurpTheo, Igorbek, aluanhaddad, and 2 more reacted with thumbs up emoji zpdDG4gta8XKpMCd, krryan, alitaheri, HerringtonDarkholme, Igorbek, aluanhaddad, nishanths, and gcnew reacted with hooray emoji zpdDG4gta8XKpMCd, krryan, alitaheri, HerringtonDarkholme, nishanths, and gcnew reacted with heart emoji
All reactions
  • 👍 12 reactions
  • 🎉 8 reactions
  • ❤️ 6 reactions
Anders Hejlsberg (ahejlsberg) added 11 commits June 6, 2017 09:49
@ahejlsberg
Stricter type relationship checking of generic signatures
368e5af
@ahejlsberg
Accept new baselines
de3ff39
@ahejlsberg
Ignore nullability in contextual signature instantiation
1143489
@ahejlsberg
Fix fourslash test
14f98f3
@ahejlsberg
Remove redundant fourslash test
3154a6c
@ahejlsberg
Merge branch 'master' into stricterGenericChecks
50b24ee
@ahejlsberg
Infer generic type for lambda with generic contextual signature
872e916
@ahejlsberg
Accept new baselines
10e1b54
@ahejlsberg
Remove convoluted code that attempts to re-fix type parameters
ff83327
@ahejlsberg
Accept new baselines
bc3689f
@ahejlsberg
Merge branch 'master' into stricterGenericChecks
6da9610
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Jun 8, 2017
@ahejlsberg Anders Hejlsberg (ahejlsberg) changed the title Stricter generic checks Stricter generic signature checks Jun 8, 2017
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Jun 8, 2017

Copy link
Copy Markdown
Member Author

Aleksey-Bykov Now you can retire the shirt!

zpdDG4gta8XKpMCd, krryan, alitaheri, kitsonk, Igorbek, KiaraGrouwstra, and wwwillchen reacted with thumbs up emoji RyanCavanaugh, krryan, s-panferov, tinganho, alitaheri, HerringtonDarkholme, hastebrot, markusjohnsson, SlurpTheo, prashaantt, and 5 more reacted with laugh emoji
All reactions
  • 👍 7 reactions
  • 😄 15 reactions

Sorry, something went wrong.

Uh oh!

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

@ahejlsberg
Anders Hejlsberg (ahejlsberg) requested a review from Mohamed Hegazy (mhegazy) June 8, 2017 19:49
@ahejlsberg Anders Hejlsberg (ahejlsberg) added the Breaking Change Would introduce errors in existing code label Jun 8, 2017
@zpdDG4gta8XKpMCd

ZpdDG4gta (zpdDG4gta8XKpMCd) commented Jun 8, 2017

Copy link
Copy Markdown

Anders Hejlsberg (@ahejlsberg) this is one of all time grand features of TS since unions, type guards and exhaustive switches, it brings point free programming style like what you can find in big boy's FP languages, thank you for your hard work and consideration

HerringtonDarkholme, sergeyzenchenko, hastebrot, SlurpTheo, sean-vieira, and JsonFreeman reacted with hooray emoji
All reactions
  • 🎉 6 reactions

Sorry, something went wrong.

Uh oh!

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

@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Jun 8, 2017
Check applicable signature fix #16104
Closed
@gcnew Marin Marinov (gcnew) mentioned this pull request Jun 9, 2017
Unexpected union type between generic and non-generic function #12970
Closed
@gcnew

Marin Marinov (gcnew) commented Jun 9, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Great work!

One issue that still stands is #16107. As #16368 is already a breaking change, is it now the time to correct the union inference wart as well? I hope it is.

All reactions

Sorry, something went wrong.

Uh oh!

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

gcnew
Marin Marinov (gcnew) reviewed Jun 9, 2017
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
}
}
else if (sourceSignatures.length === 1 && targetSignatures.length === 1) {
// For pure functions (functions with a single signature) we only erase type parameters for

@gcnew Marin Marinov (gcnew) Jun 9, 2017

Copy link
Copy Markdown
Contributor

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.

Maybe simple/just/only functions would be a better name than pure, as pure brings a different connotation.

Sorry, something went wrong.

Uh oh!

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

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Jun 9, 2017

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.

Agreed. Will fix.

Sorry, something went wrong.

Uh oh!

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

All reactions
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Jun 9, 2017
Design Meeting Notes, 6/9/2017 #16415
Closed
Anders Hejlsberg (ahejlsberg) added 4 commits June 9, 2017 15:35
@ahejlsberg
Merge branch 'master' into stricterGenericChecks
76093c2
# Conflicts:
#	src/compiler/checker.ts
@ahejlsberg
Fix comments
d27f4d4
@ahejlsberg
Add --noStrictGenericChecks compiler option
dc28aaa
@ahejlsberg
Add test
4e9e62d
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Jun 12, 2017

Copy link
Copy Markdown
Member Author

Latest commits add a --noStrictGenericChecks compiler option to disable the stricter generic signature checks. Existing projects can use this as a stopgap solution until errors resulting from the stricter checking are corrected.

All reactions

Sorry, something went wrong.

Uh oh!

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

mhegazy
Mohamed Hegazy (mhegazy) approved these changes Jun 12, 2017
View reviewed changes
@ahejlsberg Anders Hejlsberg (ahejlsberg) added this to the TypeScript 2.4 milestone Jun 12, 2017
@ahejlsberg
Anders Hejlsberg (ahejlsberg) merged commit dde60bb into master Jun 12, 2017
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Aug 15, 2017
Generics with 'extends' for return types cause type check failure. #13993
Closed
This was referenced Aug 17, 2017
split: Fix test DefinitelyTyped/DefinitelyTyped#19067
Closed
Node pipe<T> methods DefinitelyTyped/DefinitelyTyped#19069
Closed
meteor: Fix compile errors DefinitelyTyped/DefinitelyTyped#19075
Merged
lory.js: Fix compile errors DefinitelyTyped/DefinitelyTyped#19076
Merged
bookshelf: Replace typeof Model with ModelSubclass DefinitelyTyped/DefinitelyTyped#19095
Merged
backbone-associations: use new() => T instead of typeof T DefinitelyTyped/DefinitelyTyped#19097
Merged
go: Fix compile error DefinitelyTyped/DefinitelyTyped#19101
Merged
Fix mongoose-promise and mpromise compatibility DefinitelyTyped/DefinitelyTyped#19105
Merged
mongoose: Fix compile errors DefinitelyTyped/DefinitelyTyped#19107
Merged
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Aug 18, 2017
Bug report about keyof with class implements. #17822
Closed
@ikatyang Ika (ikatyang) mentioned this pull request Aug 21, 2017
MethodDecorator gets TS2322/TS2315 #17936
Closed
This was referenced Aug 21, 2017
i18next: Fix compile errors DefinitelyTyped/DefinitelyTyped#19205
Merged
ramda: Fix compile errors DefinitelyTyped/DefinitelyTyped#19209
Merged
redux-action: Fix compile error DefinitelyTyped/DefinitelyTyped#19213
Merged
transducers-js new compile errors DefinitelyTyped/DefinitelyTyped#19276
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Aug 28, 2017
Generic type mismatch with Readonly<T> #18094
Closed
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Sep 5, 2017
Cannot implement generic type guarding function #18202
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Sep 6, 2017
Optimize strict generic signature checking performance #18279
Merged
@tinganho Tingan Ho (tinganho) mentioned this pull request Sep 11, 2017
Generic checking error #18387
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Sep 11, 2017
string is not assignable to unbounded generic type T #18397
Closed
@ghost ghost mentioned this pull request Oct 4, 2017
typeof React.Component stopped working #18946
Closed
Rado Kirov (rkirov) added a commit to rkirov/zone.js that referenced this pull request Oct 11, 2017
@rkirov
Upgrade to typescript 2.5.2
5686b98
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
Rado Kirov (rkirov) added a commit to rkirov/zone.js that referenced this pull request Oct 11, 2017
@rkirov
Upgrade to typescript 2.5.2
85cf3e3
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
@rkirov Rado Kirov (rkirov) mentioned this pull request Oct 11, 2017
Upgrade to typescript 2.5.2 angular/zone.js#924
Merged
Rado Kirov (rkirov) added a commit to rkirov/zone.js that referenced this pull request Oct 12, 2017
@rkirov
Upgrade to typescript 2.5.2
8529662
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
@ghost ghost mentioned this pull request Oct 20, 2017
TypeScript: type definitions for async middleware reduxjs/redux#2602
Closed
Miško Hevery (mhevery) pushed a commit to angular/zone.js that referenced this pull request Dec 27, 2017
@rkirov @mhevery
build: Upgrade to typescript 2.5.2 (#924)
1acab39
Due to stricter generic type checks, signatures for ZoneAwarePromise
need to be changed. See
microsoft/TypeScript#16368

The signatures from lib.es5.d.ts were copied over for .then and .catch.
@weswigham Wesley Wigham (weswigham) mentioned this pull request Feb 7, 2018
Differing results for inference produced when identical overload count changes #21733
Open
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@gcnew Marin Marinov (gcnew) gcnew left review comments
@mhegazy Mohamed Hegazy (mhegazy) mhegazy approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

Breaking Change Would introduce errors in existing code

Projects

None yet

Milestone

TypeScript 2.4

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

5 participants

@ahejlsberg @zpdDG4gta8XKpMCd @gcnew @mhegazy @msftclas
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.