Adds support for type parameter defaults #13487

rbuckton · 2017-01-14T07:15:32Z

This PR adds support for defaults for generic type parameters and replaces #6354 as it is fairly out of date.

Default types for generic type parameters have the following syntax:

TypeParameter :
  BindingIdentifier Constraint? DefaultType?

DefaultType :
  `=` Type

Default types have the following rules:

Required type parameters must not follow optional type parameters.
When the checker applies defaults for type parameters, they are applied from left to right:
- If the default for a type parameter refers to a type parameter that occurs earlier in the list, the type argument for that parameter will be used.
- If the default for a type parameter refers to a type parameter that occurs later in the list, the empty object type {} will be used.
- NOTE: This helps to avoid the cost of a circularity check for defaults.
Default types for a type parameter must satisfy the constraint for the type parameter, if it exists.
A class or interface declaration that merges with an existing class or interface declaration may introduce a default for an existing type parameter.
A class or interface declaration that merges with an existing class or interface declaration may introduce a new type parameter as long as it specifies a default.
When resolving a symbol with the same name as a Type Parameter introduced in a merged declaration, the type parameter will not be considered and resolution will continue on to the next container.
When specifying type arguments, you are only required to specify type arguments for the required type parameters. Unspecified type parameters will resolve to their default types.
If a default type is not specified and inference cannot choose a candidate, the empty object type {} is inferred. This is the existing behvaior.
If a default type is specified and inference cannot chose a candidate, the default type is inferred.

This PR also updates the signature for Promise<T> and PromiseLike<T> to take full advantage of the new support for type parameter defaults.

Fixes #2175
Fixes #10977
Fixes #12725
Fixes #12886
Fixes #12910
Fixes #13008
Fixes #13015
Fixes #13117

rbuckton · 2017-01-14T07:27:49Z

@RyanCavanaugh: this is my take on refreshing the genericDefaults PR. It should resolve the open items you had on #6354 regarding checking defaults against constraints. I also added missing support for typeToString and the declaration emitter. Please let me know if you can suggest any additional tests.

@mhegazy: when we discussed this, I suggested that interface declarations that have defaults must have identical defaults, allowing you to mix declarations with and without defaults:

interface X<A, B> {}
interface X<A, B = number> {}

However, this PR does not reflect that suggestion and is more strict, requiring all declarations to have identical defaults as well. It seems more consistent and we can loosen this restriction at any point if we so choose.

@ahejlsberg: The getResolvedDefault function in checker.ts is somewhat similar to getResolvedBaseConstraint. However, I chose to reuse the existing pushTypeResolution/popTypeResolution functions to perform circularity checks. Is there a specific advantage for getResolvedBaseConstraint to have its own circularity logic? Would you recommend I use that approach for getResolvedDefault, or would it make sense to modify getResolvedBaseConstraint to use the type-resolution functions?

DanielRosenwasser · 2017-01-14T21:29:15Z

src/compiler/checker.ts

+                const types = (<UnionOrIntersectionType>type).types;
+                const defaultTypes = filter(map(types, getResolvedDefaultWorker), x => x !== circularConstraintOrDefaultType);
+                return type.flags & TypeFlags.Union && defaultTypes.length === types.length ? getUnionType(defaultTypes) :
+                    type.flags & TypeFlags.Intersection && defaultTypes.length ? getIntersectionType(defaultTypes) :


This is a little confusing - so if there were any circular types in a union, you return undefined. But you disregard all circularities in an intersection type?

I reused this from line 4827, above. It does seem like it should also be performing a comparison against types.length as well, however I am unsure as to whether there was a specific reason for 4827 to ignore circularity in an intersection type.

I made this more consistent in 25cb02e and I've added some additional circularity tests in ca16ba8.

DanielRosenwasser · 2017-01-14T21:32:33Z

src/compiler/checker.ts

+                        typeArguments = [];
+                    }
+                    const mapper: TypeMapper = t => {
+                        for (let i = 0; i < numTypeParameters; i++) {


You technically don't need to run through the initial type arguments that were already supplied, do you? Could you just start at numTypeArguments as you did below?

No, if a type parameter has a default type that is a reference to another type parameter, then we need to map it to the other, possibly earlier, type parameter.

Otherwise you end up with this:

declare function f<T, U = T>(); f<number>() // becomes f<number, T>()

The T is invalid at the call site, so we replace it with the type number for the supplied type argument.

DanielRosenwasser · 2017-01-14T21:34:56Z

src/compiler/checker.ts

+                    }
+                    const mapper: TypeMapper = t => {
+                        for (let i = 0; i < numTypeParameters; i++) {
+                            if (t === typeParameters[i]) {


if (t !== typeParameters[i]) { continue; }

I have restructured and simplified this in 25cb02e

rbuckton · 2017-01-19T20:54:23Z

@mhegazy: when we discussed this, I suggested that interface declarations that have defaults must have identical defaults, allowing you to mix declarations with and without defaults...
...this PR does not reflect that suggestion and is more strict, requiring all declarations to have identical defaults as well. It seems more consistent and we can loosen this restriction at any point if we so choose.

The latest commit makes the following changes (also noted above):

~~If any declaration of an interface or class has a default type for a type parameter, all declarations must have an identical default type.~~

A class or interface declaration that merges with an existing class or interface declaration may introduce a default for an existing type parameter.

A class or interface declaration that merges with an existing class or interface declaration may introduce a new type parameter as long as it specifies a default.

For example:

interface X { } // no type parameters
interface X<T = never> { } // ok, add a new default type parameter

interface Y<T> { } // one required type parameter
interface Y<T = never> { } // ok, make an existing type parameter optional

interface Z { } // no type parameters
interface Z<T> { } // error, type parameter lists aren't identical

//// uncomment the following to remove the error as the declarations will merge:
// interface Z<T = never> { }

rbuckton · 2017-01-20T21:01:32Z

@ahejlsberg Per our conversation in the design meeting, I've further modified chooseOverload to avoid additional inference overhead when all type arguments are supplied.

RyanCavanaugh · 2017-01-20T21:34:15Z

src/compiler/checker.ts

        function getInferredType(context: InferenceContext, index: number): Type {
-            let inferredType = context.inferredTypes[index];
+            let inferredType = context.inferredTypes[index] || getSuppliedType(context, index);


Weird newline thing here

RyanCavanaugh · 2017-01-20T21:40:48Z

src/compiler/checker.ts

+                    minTypeArgumentCount = maxTypeArgumentCount;
+                }
+                if (typeParameters && typeParameters.length > maxTypeArgumentCount) {
+                    // Trim the type parameters to the maximum length


What's an example of how this could happen?

This is to preserve the current behavior with respect to where we report errors for type parameter lists:

interface X<T> {} // no error on first decl interface X<T, U> {} // no defaults so we trim to `<T>` and report error for this decl

Makes sense, thanks

I don't think it works to trim like this. You might have members that reference the trimmed type parameter and those members would never be properly instantiated. In other words, the trimmed type parameters could "leak".

The intent is that excess type parameters would result in an error, consistent with our behavior today as in the example above. Today we only get the type parameters of the first declaration, so this shouldn't be any different than how we would instantiate X above today.

RyanCavanaugh · 2017-01-20T21:44:00Z

src/compiler/diagnosticMessages.json

@@ -2035,6 +2035,18 @@
        "category": "Error",
        "code": 2704
    },
+    "Required type parameters may not follow optional type parameters": {


Missing trailing period in this and in line 2046

rbuckton · 2017-01-21T00:24:22Z

Following the discussion during the design meeting, I am making the following changes:

When specifying type arguments, you are only required to specify type arguments for the required type parameters. ~~Unspecified type parameters can be inferred.~~ Unspecified type parameters will resolve to their default types.

For example:

declare function f<T, U = T>(a: T, b?: U): void;

f(1); // ok. Using inference: T is number, U is number.
f(1, "a"); // ok. Using inference: T is number, U is string.
f<number>(1, "a"); // error. Not using inference: T is number, U is number.

While this is generally stricter, we can loosen this restriction at a later date if necessary.

rbuckton · 2017-01-21T02:35:00Z

I have added the following restriction:

It is an error to reference a type parameter in a class or interface declaration that is not specified in the declaration, but was introduced in a merged declaration.

This is to provide an error for the following case:

interface A { }
interface B {
  x: A; // error
}
interface B<A = number> {
}

rbuckton · 2017-02-11T00:46:15Z

I have looked into including #13609 in this change, but it results in significant change in behavior that may be too much to take on if we want to take this for 2.2.

@ahejlsberg Do you have any other feedback for this change?

mramos-dev · 2017-02-15T21:31:59Z

@rbuckton You mentioned taking this for 2.2 in an earlier comment but I see that this issue still has the 2.3 milestone attached. Will this be in 2.2?

mhegazy · 2017-02-15T21:35:40Z

@rbuckton You mentioned taking this for 2.2 in an earlier comment but I see that this issue still has the 2.3 milestone attached. Will this be in 2.2?

No. this is scheduled for TS 2.3

jeffijoe · 2017-02-19T15:58:14Z

Can't wait for this, writing a library in TS and this will save the consumers from a lot of confusion. Great work!

Adds support for type parameter defaults

3d3dae0

msftclas added the cla-not-required label Jan 14, 2017

rbuckton requested review from sandersn, DanielRosenwasser, ahejlsberg, RyanCavanaugh and mhegazy January 14, 2017 07:15

Updated Promise and PromiseLike to use defaults

442f540

DanielRosenwasser requested changes Jan 14, 2017

View reviewed changes

rbuckton added 2 commits January 14, 2017 15:09

Fix circularity check, simplify default type mapper

25cb02e

Added comments and additional circularity tests

ca16ba8

rbuckton mentioned this pull request Jan 17, 2017

Casting generics ignores --strictNullCheck #13513

Closed

Flexible declaration merging

0b44a2c

aluanhaddad mentioned this pull request Jan 20, 2017

[angular] enhancement: stricter and more precise type inference in ng.IPromise<T> and ng.IQService DefinitelyTyped/DefinitelyTyped#13905

Merged

11 tasks

Avoid inference for fully-supplied type arguments

0500065

RyanCavanaugh reviewed Jan 20, 2017

View reviewed changes

Diagnostic message punctuation

5ff0f81

RyanCavanaugh approved these changes Jan 20, 2017

View reviewed changes

DanielRosenwasser mentioned this pull request Jan 20, 2017

Design Meeting Notes, 1/20/2017 #13607

Closed

rbuckton added 2 commits January 20, 2017 17:58

Report error using type parameter from merged declaration

a2be5e2

Remove partial inference

fd228a9

rbuckton added 2 commits January 20, 2017 20:33

Defaults for type aliases

6b2c8cb

Merge branch 'master' into genericDefaults

76ba6a7

rbuckton added 2 commits February 3, 2017 16:34

Simplify checkTypeParameterListsIdentical

5bb2fe0

Shortcut for class/namespace merge

96181c0

thorn0 mentioned this pull request Feb 4, 2017

FilterOrderBy allows null and undefined DefinitelyTyped/DefinitelyTyped#14024

Closed

10 tasks

RyanCavanaugh mentioned this pull request Feb 8, 2017

Default generic type variables #2175

Closed

Artazor mentioned this pull request Feb 12, 2017

Allow classes to be parametric in other parametric classes #1213

Open

RyanCavanaugh mentioned this pull request Feb 13, 2017

Generic defaults #6354

Closed

2 tasks

ahejlsberg approved these changes Feb 13, 2017

View reviewed changes

rbuckton added this to the TypeScript 2.3 milestone Feb 14, 2017

Merge branch 'master' into genericDefaults

23216f9

rbuckton merged commit 9be853f into master Feb 15, 2017

rbuckton deleted the genericDefaults branch February 15, 2017 03:32

MichalLytek mentioned this pull request Feb 16, 2017

MongoDB support typeorm/typeorm#148

Closed

KiaraGrouwstra mentioned this pull request Mar 5, 2017

Proposal: Get the type of any expression with typeof #6606

Closed

kube mentioned this pull request Mar 5, 2017

Optional Generic Type Inference #14400

Open

kitsonk mentioned this pull request Mar 9, 2017

Migration to TypeScript 2.3 dojo/meta#153

Closed

p-jackson mentioned this pull request Apr 4, 2017

React: Use specific event type for nativeEvent property DefinitelyTyped/DefinitelyTyped#15610

Merged

8 tasks

goodmind mentioned this pull request Apr 5, 2017

Support Cycle Unified staltz/cycle-onionify#18

Merged

unional mentioned this pull request Apr 22, 2017

Treating undefined parameters as optional #12400

Open

ErikCupal mentioned this pull request Jun 2, 2017

feat(typescript): Improved theme typing and tests paypal/glamorous#160

Merged

5 tasks

WasabiFan mentioned this pull request Jul 10, 2017

"Maximum call stack size exceeded" with recursive type parameter default #17049

Closed

rokoroku mentioned this pull request Sep 11, 2017

Generics for the model attributes? types/sequelize#125

Open

weswigham mentioned this pull request Sep 12, 2017

Suggestion: Generic Parameter Overloads #209

Closed

Joge97 mentioned this pull request Sep 21, 2017

Default generic return types for class methods #18636

Closed

HerringtonDarkholme mentioned this pull request Sep 22, 2017

Further improve Vue type declarations for canonical usage vuejs/vue#6391

Merged

mceachen mentioned this pull request Oct 12, 2017

Typings don't allow custom query builders Vincit/objection.js#319

Closed

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for type parameter defaults #13487

Adds support for type parameter defaults #13487

rbuckton commented Jan 14, 2017 •

edited

Loading

rbuckton commented Jan 14, 2017 •

edited

Loading

DanielRosenwasser Jan 14, 2017

rbuckton Jan 14, 2017

rbuckton Jan 14, 2017 •

edited

Loading

DanielRosenwasser Jan 14, 2017

rbuckton Jan 14, 2017 •

edited

Loading

DanielRosenwasser Jan 14, 2017

rbuckton Jan 14, 2017

rbuckton commented Jan 19, 2017

rbuckton commented Jan 20, 2017

RyanCavanaugh Jan 20, 2017

RyanCavanaugh Jan 20, 2017

rbuckton Jan 20, 2017

RyanCavanaugh Jan 20, 2017

ahejlsberg Jan 27, 2017

rbuckton Jan 27, 2017

RyanCavanaugh Jan 20, 2017

rbuckton commented Jan 21, 2017 •

edited

Loading

rbuckton commented Jan 21, 2017

rbuckton commented Feb 11, 2017

mramos-dev commented Feb 15, 2017

mhegazy commented Feb 15, 2017

jeffijoe commented Feb 19, 2017

Adds support for type parameter defaults #13487

Adds support for type parameter defaults #13487

Conversation

rbuckton commented Jan 14, 2017 • edited Loading

rbuckton commented Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton Jan 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton commented Jan 19, 2017

rbuckton commented Jan 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton commented Jan 21, 2017 • edited Loading

rbuckton commented Jan 21, 2017

rbuckton commented Feb 11, 2017

mramos-dev commented Feb 15, 2017

mhegazy commented Feb 15, 2017

jeffijoe commented Feb 19, 2017

rbuckton commented Jan 14, 2017 •

edited

Loading

rbuckton commented Jan 14, 2017 •

edited

Loading

rbuckton Jan 14, 2017 •

edited

Loading

rbuckton Jan 14, 2017 •

edited

Loading

rbuckton commented Jan 21, 2017 •

edited

Loading