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 — Add 'extends' clause to 'infer' type by rbuckton · Pull Request #48112 · microsoft/TypeScript

Conversation

@rbuckton

@rbuckton Ron Buckton (rbuckton) commented Mar 3, 2022

Copy link
Copy Markdown
Contributor

This adds an optional extends clause to infer T to specify an explicit constraint for the T type parameter. When specified,
the explicit constraint overrides the constraint we would have attempted to infer for T.

For example, if you wished to infer a type from the first element of a tuple but also constrain that type, you might currently need to write something like the following:

// This type alias serves as an example of any type with a constraint you might need to satisfy
type MustBeNumber<T extends number> = { y: T };

// Option 1, use a nested conditional
type X1<T> =
  T extends [infer U] ?
    U extends number ? MustBeNumber<U> :
    never :
  never;

// Option 2, use a secondary type to constrain the inference
type Is<T extends U, U> = T;
type X2<T> =
  T extends [Is<infer U, number>] ? MustBeNumber<U> :
  never;

For Option 1, you are forced to create two conditional types, resulting in two alternatives. If you are testing multiple other conditions, this could result in an unmanageable branching structure which results in the need to define additional type aliases for repeated branches:

type X1<T> =
  T extends [infer U] ? U extends number ? MustBeNumber<U> : 
    T extends {x: infer U} ? U extends number ? MustBeNumber<U> :
      ...
  // repeats...
  T extends {x: infer U} ? U extends number ? MustBeNumber<U> :
    ...

For Option 2, you are required to define a second type to enforce the constraint. While this is trivial, it is unreliable when inferring to the same type variable in more than one position, as it is possible to define disjoint constraints:

type X2<T> = T extends [Is<infer U, number>, Is<infer U, string>] ? MustBeNumber<U> : never; // U will always be `never`

In contrast, specifying an explicit constraint via extends allows us to achieve the simpler branching structure of Option 2, while providing additional safety by checking that the constraints are consistent:

// simpler branching due to combining the constraint
type X3<T> = T extends [infer U extends number] ? MustBeNumber<U> : never;

// all constraints for same type parameter must match, if present
type X4<T> = T extends [infer U extends number, infer U extends number] ? MustBeNumber<U> : never;

// only one type parameter needs to be constrained (similar to class/interface merging)
type X5<T> = T extends [infer U extends number, infer U] ? MustBeNumber<U> : never;
type X6<T> = T extends [infer U, infer U extends number] ? MustBeNumber<U> : never;

// errors on inconsistent constraints
type X7<T> = T extends [infer U extends string, infer U extends number] ? U : never;
// error: All declarations of 'U' must have identical constraints.

Related #48094

Sorry, something went wrong.

Uh oh!

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

tonivj5, zyhou, whzx5byb, tjjfvi, somebody1234, sno2, DaniGuardiola, val1984, maxdavidson, anuraghazra, and 17 more reacted with thumbs up emoji tjjfvi, DaniGuardiola, mkantor, anuraghazra, Oblosys, moonqube, ycmjason, cypherfunc, p810, Evoque, and arutkowski00 reacted with hooray emoji tonivj5, treybrisbane, tjjfvi, rraziel, DaniGuardiola, anuraghazra, moonqube, ycmjason, zerobias, cypherfunc, and 6 more reacted with heart emoji olivermrose, harrysolovay, tjjfvi, sno2, moonqube, ycmjason, Evoque, and Vtec234 reacted with rocket emoji
All reactions
  • 👍 27 reactions
  • 🎉 11 reactions
  • ❤️ 16 reactions
  • 🚀 8 reactions
@typescript-bot TypeScript Bot (typescript-bot) assigned Ron Buckton (rbuckton) Mar 3, 2022
@typescript-bot TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 3, 2022
@rbuckton
Ron Buckton (rbuckton) requested review from Anders Hejlsberg (ahejlsberg) and Wesley Wigham (weswigham) March 3, 2022 23:53
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Mar 3, 2022
More specific inference for constrained 'infer' types in template literal types #48094
Merged
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Mar 4, 2022

Copy link
Copy Markdown
Member

Do we have a sense of what kind of libraries would benefit from this?

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton

Ron Buckton (rbuckton) commented Mar 4, 2022

Copy link
Copy Markdown
Contributor Author

Probably anyone doing tuple type or template literal type manipulation, plus #48112. I regularly find myself needing to define something like Is or use Extract when inferring from tuple types when I need a more specific type than inference allows.

I found a few places that could leverage this:

  • type-fest
    • https://cs.github.com/sindresorhus/type-fest/blob/cbd7ec510bd136ac045bbc74e391ee686b8a9a2f/source/get.d.ts?q=infer#L15-L19
    • https://cs.github.com/sindresorhus/type-fest/blob/cbd7ec510bd136ac045bbc74e391ee686b8a9a2f/source/get.d.ts#L59-L66 (could be written to a form that's easier to read)
    • https://cs.github.com/sindresorhus/type-fest/blob/cbd7ec510bd136ac045bbc74e391ee686b8a9a2f/source/join.d.ts?q=infer#L28-L29 (though there's possibly an inference bug that could be fixed here as well)
    • https://cs.github.com/sindresorhus/type-fest/blob/cbd7ec510bd136ac045bbc74e391ee686b8a9a2f/source/delimiter-case.d.ts?q=infer#L8-L20 (explicit constraints would reduce the number of : never branches)
  • material-ui
    • https://cs.github.com/mui/material-ui/blob/2a81d58c7b9e10718474639dc613078050e512b9/packages/mui-joy/src/styles/defaultTheme.ts?q=infer#L31-L39
  • redux-orm (on DefinitelyTyped)
    • https://cs.github.com/DefinitelyTyped/DefinitelyTyped/blob/2280eb45b474bf8e92ff34b1d2775afc03cffc6f/types/redux-orm/db/Table.d.ts#L34-L46
    • https://cs.github.com/DefinitelyTyped/DefinitelyTyped/blob/2280eb45b474bf8e92ff34b1d2775afc03cffc6f/types/redux-orm/Model.d.ts#L383-L389
  • react-router (on DefinitelyTyped)
    • https://cs.github.com/DefinitelyTyped/DefinitelyTyped/blob/2280eb45b474bf8e92ff34b1d2775afc03cffc6f/types/react-router/index.d.ts?q=path%3A*.ts+%22infer%22#L157-L167 (could be simplified)
  • convict (on DefinitelyTyped)
    • https://cs.github.com/DefinitelyTyped/DefinitelyTyped/blob/2280eb45b474bf8e92ff34b1d2775afc03cffc6f/types/convict/index.d.ts#L121-L129
electron-space, ICodeMyOwnLife, and shippokun reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

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

@phryneas

Lenz Weber-Tronic (phryneas) commented Mar 4, 2022

Copy link
Copy Markdown

You can add the Redux ecosystem (Redux Toolkit, Reselect) to that list - we have a few cases where we have a
Something<X> extends infer Y ? Y extends Bar ? /* actual logic */.

I'm pretty sure Tanner Linsley (@tannerlinsley) will have quite a few uses in React Table for this, but that's speculation from my side.

All reactions

Sorry, something went wrong.

Uh oh!

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

@tannerlinsley

Tanner Linsley (tannerlinsley) commented Mar 4, 2022

Copy link
Copy Markdown

I would greatly benefit from this in React Table and React Location where I am already writing a lot branching conditional types to both enforce and extract generics. It’s brittle. This would make those contracts easier to define and much more reliable.

electron-space, qcom, and steida reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton
Ron Buckton (rbuckton) force-pushed the inferTypeExtends branch from 695e440 to bafe193 Compare March 4, 2022 18:31
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Mar 4, 2022

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack 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 Mar 4, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the tarball bundle task on this PR at bafe193. You can monitor the build here.

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 Mar 4, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Hey Daniel Rosenwasser (@DanielRosenwasser), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/121221/artifacts?artifactName=tgz&fileId=9A84662ABF9C9E378556EF91C66AB21BF7E1E2513A39ED6E4C61DFA8C11CBA6302&fileName=/typescript-4.7.0-insiders.20220304.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton

Ron Buckton (rbuckton) commented Mar 7, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

In the design meeting we discussed the syntax ambiguity with infer T extends U ?. There are three approaches we can take:

  1. Leave the precedence as written in this PR. This would be a breaking change since the following is perfectly legal TS today:

    type X<T> = T extends (infer U extends number ? 1 : 0) ? 1 : 0;

    If we choose this option, the above would become a parse error and existing code would need to change to parenthesize the infer U.

  2. Use speculative parsing disambiguate infer T extends U ?. This means that we would speculatively parse the extends clause of an infer type, and if we encounter a ? we would rewind. This is similar to how we speculatively parse arrow functions, but has the same drawback: If the type in the extends clause is very large we would have to re-parse it again. This may be acceptable as infer T extends U ? doesn't seem to be a common occurrence in the wild, and if a project does encounter a slowdown due to the lookahead, they can easily wrap the infer type in parens to avoid rewinding.

  3. Use a cover grammar to disambiguate infer T extends U ?. This means that we would parse the infer T extends U as an infer type, and if we encounter a ? following that type we would recreate the infer type without the constraint and reuse the node for the constraint. This avoids the performance implications of speculative parsing, but will require more thorough testing to ensure that it doesn't break incremental parsing scenarios.

  4. Use a different keyword instead of extends for an infer type's constraint, such as of (i.e., infer T of U). This avoids the ambiguity, but introduces a second term with the same meaning as extends, which could be a source of confusion.

I investigated using something like JS's [In] parser context, however that approach didn't seem viable. The places we would want to allow extends in an infer type (such as in a parenthesized type) are the same places you might want to infer to the check type of a conditional type, so there was no way to separate them using a context flag.

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton

Ron Buckton (rbuckton) commented Mar 7, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

You can add the Redux ecosystem (Redux Toolkit, Reselect) to that list - we have a few cases where we have a Something<X> extends infer Y ? Y extends Bar ? /* actual logic */.

[...]

I have been a bit wary of classifying all cases of ... infer Y ? Y extends Bar ? ... as possible use cases since Y extends Bar ? ... could also indicate the need to distribute over Y rather than merely constraining it. Switching to infer T extends U will definitely need to be assessed on a case-by-case basis.

All reactions

Sorry, something went wrong.

Uh oh!

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

@phryneas

Lenz Weber-Tronic (phryneas) commented Mar 7, 2022

Copy link
Copy Markdown

You can add the Redux ecosystem (Redux Toolkit, Reselect) to that list - we have a few cases where we have a Something<X> extends infer Y ? Y extends Bar ? /* actual logic */.
[...]

I have been a bit wary of classifying all cases of ... infer Y ? Y extends Bar ? ... as possible use cases since Y extends Bar ? ... could also indicate the need to distribute over Y rather than merely constraining it. Switching to infer T extends U will definitely need to be assessed on a case-by-case basis.

Of course, but we only really use it in cases where we have the else case be never - we only really add that inner extends to make the compiler happy. Might not be the core of this PR, but it definitely helps us write code.

All reactions

Sorry, something went wrong.

Uh oh!

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

@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Mar 8, 2022

Copy link
Copy Markdown
Member

I'd also toss in "require parenthesization" as a possible parsing strategy

type X<T> = T extends (infer U extends number) ? 1 : 0;
All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton

Ron Buckton (rbuckton) commented Mar 9, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

I'd also toss in "require parenthesization" as a possible parsing strategy

type X<T> = T extends (infer U extends number) ? 1 : 0;

Unfortunately, requiring parens alone doesn't resolve the ambiguity, since T extends (infer U extends number ? 1 : 0) ? 1 : 0 is already legal (and thus we could still encounter a trailing ?).

Right now I'm leaning towards (2) for its simplicity, but changing the noConditionalTypes parameter of parseTypeWorker into a parser context flag so that we can relax the need for parens in situations where we can be sure a trailing ? wouldn't be ambiguous:

// ok, parsed as conditional
type X1<T> = T extends ((infer U) extends number ? 1 : 0) ? 1 : 0; 

// ok, parsed as `infer..extends` (speculative parse succeeds due to no trailing `?`)
type X2<T> = T extends (infer U extends number) ? 1 : 0; 

// ok, parsed as `infer..extends` (conditional types not allowed in 'extends type' of conditional)
type X3<T> = T extends infer U extends number ? 1 : 0; 

// ok, parsed as `infer..extends` (precedence wouldn't have parsed the `?` as part of a type operator)
type X4<T> = T extends keyof infer U extends number ? 1 : 0; 

// ok, parsed as conditional (speculative parse rewinds when it sees the first `?`)
type X5<T> = T extends { [P in infer U extends keyof T ? 1 : 0]: 1; } ? 1 : 0; 

// ok, parsed as `infer..extends` (no trailing `?`)
type X6<T> = T extends { [P in infer U extends keyof T]: 1; } ? 1 : 0; 

// ok, parsed as conditional (speculative parse rewinds when it sees the first `?`)
type X7<T> = T extends { [P in keyof T as infer U extends P ? 1 : 0]: 1; } ? 1 : 0; 

// ok, parsed as `infer..extends` (speculative parse succeeds due to no trailing `?`)
type X8<T> = T extends { [P in keyof T as infer U extends P]: 1; } ? 1 : 0; 

It doesn't work quite the same way as [In], since this still requires speculative parsing, but its reliable and consistent and isn't a breaking change.

All reactions

Sorry, something went wrong.

Uh oh!

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

@treybrisbane

Trey Brisbane (treybrisbane) commented Mar 9, 2022

Copy link
Copy Markdown

I'm gonna go ahead and ask the dumb question: Why can't we automatically derive the constraint of an inferred type binding from the constraint of the type being matched?

For example, given

type Quote<AString extends string> = `"${AString}"`;

type QuotedFirstElement<AStringTuple extends readonly string[]> =
  AStringTuple extends readonly [infer FirstElement, ...infer Rest]
    ? Quote<FirstElement> // Currently errors here, because `FirstElement` is only constrained to `unknown`
    : never;

why can't we say "well, since AStringTuple has a constraint of readonly string[], any elements we infer from it should therefore have a constraint of string"?

Intuitively, I suspect there's a very good reason for this and that I just haven't thought it through enough, but someone's gotta spring the trap! 😁

cypherfunc and jcalz reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton

Ron Buckton (rbuckton) commented Mar 9, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

I'm gonna go ahead and ask the dumb question: Why can't we automatically derive the constraint of an inferred type binding from the constraint of the type being matched?

We do have automatic inference, and there are ways we can improve it. However, that still wouldn't solve the issue at hand. There are times where you need a more-specific type than what the inferred constraint might allow:

type X<T extends any[]> =
  T extends [infer U extends string, ...infer R] ? ... :
  T extends [infer U extends number, ...infer R] ? ... :
  ...;

Above, I might want to make branching decisions based on a more specific constraint. This also ties into #48094, which uses more specific inferences for infer in a template literal type to pull out numeric literals.

treybrisbane and electron-space reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Mar 10, 2022
Design Meeting Notes, 3/4/2022 #48208
Closed
@rbuckton

Ron Buckton (rbuckton) commented Mar 11, 2022

Copy link
Copy Markdown
Contributor Author

This last change modifies the parse based on #48112 (comment).

I wanted to ensure that parenthesization was accurate, so I took a pass through the parenthesizer rule logic for types to be more accurate. One side-effect of that is that we'll emit fewer parens in declarations and diagnostics. Where we might have previously written (A & B) | (C & D), we may now write A & B | C & D. The parens weren't strictly necessary, but they may have made the diagnostics more readable. I can make union/intersection parenthesization more aggressive in general, if necessary, or possibly just pass a more aggressive parenthesizer to the printer for typeToString if we only want it for diagnostics/quickinfo.

Daniel Rosenwasser (@DanielRosenwasser), can you provide your thoughts on the change in diagnostics evidenced here: 38eb412?show-viewed-files=true&file-filters%5B%5D=#diff-daa0fded8957f3b2eecdd68e6f89377663f965eb6e9c2b83f36955200b6e858fL5-R6

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton
Add 'extends' clause to 'infer' type
8fd5eda
@rbuckton

Ron Buckton (rbuckton) commented Mar 11, 2022

Copy link
Copy Markdown
Contributor Author

TypeScript Bot (@typescript-bot) perf test
TypeScript Bot (@typescript-bot) run dt
TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton
Ron Buckton (rbuckton) force-pushed the inferTypeExtends branch from 38eb412 to 2fb093c Compare March 11, 2022 23:44
@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 11, 2022 •
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 extended test suite on this PR at 2fb093c. You can monitor the build here.

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 Mar 11, 2022 •
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 parallelized Definitely Typed test suite on this PR at 2fb093c. You can monitor the build here.

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 Mar 11, 2022 •
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 parallelized community code test suite on this PR at 2fb093c. You can monitor the build here.

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 Mar 11, 2022 •
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 2fb093c. 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.

@rbuckton
More aggressive parens to match existing DT tests
326389b
@rbuckton
Ron Buckton (rbuckton) force-pushed the inferTypeExtends branch from 1731ad3 to 326389b Compare March 15, 2022 21:45
@rbuckton

Ron Buckton (rbuckton) commented Mar 15, 2022

Copy link
Copy Markdown
Contributor Author

TypeScript Bot (@typescript-bot) run dt

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 Mar 15, 2022 •
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 parallelized Definitely Typed test suite on this PR at 326389b. You can monitor the build here.

All reactions

Sorry, something went wrong.

Uh oh!

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

Ron Buckton (rbuckton) added 2 commits March 15, 2022 17:50
@rbuckton
tests 'infer' constraint using outer type parameter
4a7019e
@rbuckton
Adds a test showing 'infer' cannot reference other 'infer' in same 'e…
02fa9c7
…xtends'
@rbuckton
Ron Buckton (rbuckton) requested review from Ryan Cavanaugh (RyanCavanaugh) and Nathan Shively-Sanders (sandersn) March 18, 2022 23:41
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Mar 18, 2022
Updated to support 'infer..extends' microsoft/TypeScript-TmLanguage#902
Merged
@rbuckton

Ron Buckton (rbuckton) commented Mar 22, 2022

Copy link
Copy Markdown
Contributor Author

Ping Anders Hejlsberg (@ahejlsberg), Wesley Wigham (@weswigham), Nathan Shively-Sanders (@sandersn), Ryan Cavanaugh (@RyanCavanaugh)

sno2 reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

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

@rbuckton
Merge branch 'main' into inferTypeExtends
0aded71
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Mar 23, 2022
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

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.

The parser changes look good but I can't adequately comment on the checker changes.

Sorry, something went wrong.

Uh oh!

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

All reactions
weswigham
Wesley Wigham (weswigham) requested changes Mar 23, 2022
View reviewed changes

@weswigham Wesley Wigham (weswigham) left a comment •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

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.

typeToTypeNodeHelper in the checker needs to include the constraint in the context.inferTypeParameters case of type.flags & TypeFlags.TypeParameter. If it's not there, we'll elide the constraints in error messages and inferred type declaration emit (eg, if one of these is inlined into a function return type).

Other than that, looks good - pretty simple.

Sorry, something went wrong.

Uh oh!

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

rbuckton reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@rbuckton

Ron Buckton (rbuckton) commented Mar 24, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

typeToTypeNodeHelper in the checker needs to include the constraint in the context.inferTypeParameters case of type.flags & TypeFlags.TypeParameter. If it's not there, we'll elide the constraints in error messages and inferred type declaration emit (eg, if one of these is inlined into a function return type).

I think the latest change should cover this. I reuse getInferredTypeParameterConstraint, and elide the extends clause if the constraint is something we would have inferred from the surrounding context. Constraints inferred from a type argument to a type reference are preserved since the type reference may have been elided during typeToTypeNode.

All reactions

Sorry, something went wrong.

Uh oh!

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

@rbuckton
Emit extends clause for synthetic infer typesduring declaration emit
01b9a2d
@rbuckton
Ron Buckton (rbuckton) force-pushed the inferTypeExtends branch from 2e4fa4f to 01b9a2d Compare March 24, 2022 04:42
weswigham
Wesley Wigham (weswigham) approved these changes Mar 24, 2022
View reviewed changes
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Mar 28, 2022
Source provides no match for required element at position 0 in target despite source extending [any, ...any[]] #48401
Closed
raghavthind2005
Raghav Thind (raghavthind2005) approved these changes Apr 1, 2022
View reviewed changes
@rbuckton Ron Buckton (rbuckton) mentioned this pull request Apr 4, 2022
Extract to typedef from (invalid) type with comments in JS file causes assertion failure #48540
Closed
@rbuckton
Ron Buckton (rbuckton) merged commit 7da80d7 into main Apr 5, 2022
@rbuckton
Ron Buckton (rbuckton) deleted the inferTypeExtends branch April 5, 2022 19:35
@g-plane Pig Fang (g-plane) mentioned this pull request Apr 13, 2022
feat(es/parser): Support 'extends' clause to 'infer' type swc-project/swc#4326
Merged
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Apr 13, 2022
Design Meeting Notes, 3/23/2022 #48684
Closed
@artysidorenko Arty (artysidorenko) mentioned this pull request Apr 22, 2022
fix: type issue when void or undefined is returned from query function TanStack/query#3541
Merged
@JLHwung Huáng Jùnliàng (JLHwung) mentioned this pull request Apr 22, 2022
[ts 4.7] Support extends constraints for infer babel/babel#14476
Merged
@Andarist Mateusz Burzyński (Andarist) mentioned this pull request Apr 25, 2022
Add tests case for function check type being correctly paranthesized in quick info #48836
Merged
@yudai-nkt Yudai Nakata (yudai-nkt) mentioned this pull request Apr 29, 2022
Allowing infer after extends T in conditional types would simplify complicated type definitions massively #47330
Closed
5 tasks done
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request May 4, 2022
Typescript Quick Info Formatter Removes parentheses () from declared type on hovering, for conditionally inferring types #48729
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
@weswigham Wesley Wigham (weswigham) weswigham approved these changes
@ahejlsberg Anders Hejlsberg (ahejlsberg) Awaiting requested review from ahejlsberg
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Awaiting requested review from RyanCavanaugh
+1 more reviewer
@raghavthind2005 Raghav Thind (raghavthind2005) raghavthind2005 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@rbuckton Ron Buckton (rbuckton)

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

10 participants

@rbuckton @RyanCavanaugh @phryneas @tannerlinsley @DanielRosenwasser @typescript-bot @treybrisbane @sandersn @weswigham @raghavthind2005
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.