Do not measure variance for a conditional type extendsType #31277

jack-williams · 2019-05-06T21:49:44Z

Attempts to fix #31251

falsandtru · 2019-05-20T15:39:42Z

jack-williams · 2019-05-21T08:25:41Z

This PR does not address variance issues stemming from check types (see #31295 as an example). I wonder if it is best to try and fix them together?

falsandtru · 2019-05-24T23:43:09Z

ping @ahejlsberg @weswigham

falsandtru · 2019-05-27T20:56:02Z

ping @ahejlsberg @weswigham

weswigham · 2019-06-11T23:29:08Z

@jack-williams can you sync this? We'd want to test this across DT with perf in mind before merging it, which is our primary concern with using Unmeasurable and Unreliable more.

jack-williams · 2019-06-12T00:38:06Z

@weswigham done!

weswigham · 2019-06-12T01:27:40Z

@typescript-bot run dt
@typescript-bot test this

typescript-bot · 2019-06-12T01:27:44Z

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 70336e6. You can monitor the build here. It should now contribute to this PR's status checks.

typescript-bot · 2019-06-12T01:27:44Z

Heya @weswigham, I've started to run the extended test suite on this PR at 70336e6. You can monitor the build here. It should now contribute to this PR's status checks.

falsandtru · 2019-06-16T04:48:01Z

@jack-williams Can you resolve the errors?

@weswigham Can you run @typescript-bot pack this?

weswigham · 2019-06-16T09:57:08Z

Sure - @typescript-bot pack this

typescript-bot · 2019-06-16T09:57:36Z

Heya @weswigham, I've started to run the tarball bundle task on this PR at 70336e6. You can monitor the build here. It should now contribute to this PR's status checks.

typescript-bot · 2019-06-16T10:12:22Z

Hey @weswigham, 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/33400/artifacts?artifactName=tgz&fileId=B10B0A88C3D1C8DCA0C6AA5DDED4DD385013554114808F4B3A5FBB56317A12D102&fileName=/typescript-3.6.0-insiders.20190616.tgz"
    }
}

and then running npm install.

falsandtru · 2019-06-16T12:23:44Z

Thanks.

@jack-williams Comparison targets are still reversed:

Type 'typeof Book' does not satisfy the constraint 'typeof AnyModel'.
  Types of property 'reducer' are incompatible.
    Type '(action: RootAction, Book: ModelType<Book>) => void' is not assignable to type '(action: any, modelType: ModelType<any>, session: Pick<Pick<Session<any>, never> & Pick<{ [x: string]: ModelType<any>; }, never> & Pick<{ [x: string]: ModelType<any>; }, string | number | symbol>, string | ... 1 more ... | symbol>) => void'.
      Types of parameters 'Book' and 'modelType' are incompatible.
        Type 'ModelType<any>' is not assignable to type 'ModelType<Book>'.
          ...

It should be

-        Type 'ModelType<any>' is not assignable to type 'ModelType<Book>'.
+        Type 'ModelType<Book>' is not assignable to type 'ModelType<any>'.

jack-williams · 2019-06-16T15:35:17Z

@falsandtru can you give me the full example? This might be an issue with the check type, not the extends type.

falsandtru · 2019-06-16T15:41:32Z

See the 4352th line of https://typescript.visualstudio.com/TypeScript/_build/results?buildId=33063&view=logs&jobId=ae5f67bc-b508-52d0-18c7-3e0886a35c88&taskId=e2afb42f-d9e0-5ec8-c417-e22c6ef5223e&lineStart=4352&lineEnd=4353&colStart=1&colEnd=1

jack-williams · 2019-06-16T16:14:44Z

It should be
Type 'ModelType<Book>' is not assignable to type 'ModelType<any>'.

Should it? The type appears in an argument position so I would expect them to be flipped when running strictFunctionTypes

I don't really have time to debug DT right now, but I wouldn't be surprised if this was correctly an error, something along the line of:

interface Foo<T,U> {
	x: T extends U ? number : string;
}
declare const a: Foo<any, boolean>;
let b: Foo<boolean, boolean> = a; // no error, but it should be

falsandtru · 2019-06-16T16:27:51Z

I referred to that line to explain the comparison order. Of course it should be no error if any type makes no errors.

jack-williams · 2019-08-18T14:03:45Z

@typescript-bot run dt because the old builds logs were cleared up.

typescript-bot · 2019-08-18T14:03:53Z

Heya @jack-williams, I've started to run the parallelized Definitely Typed test suite on this PR at 52c7dff. You can monitor the build here. It should now contribute to this PR's status checks.

jack-williams · 2019-08-19T09:26:04Z

I need to look into some of the changes to see whether they are reasonable, and more likely, what is the corresponding fix. Most of them seem to be cases where something is constrained with a type Interface<any> that previously passed on variance checks, but no longer does.

weswigham · 2019-08-19T10:16:22Z

I believe that. What we've been trying to do as of late is to fix the breaks on DT (via PRs to DT) such that we can get a clean run on the PR. It normally works out pretty well in cases like these where you can say "this was a possible way the old types could have been misused, TS with this new PR catches this, and this change preemptively fixes the package so it won't be broken by the next TS release".

weswigham · 2019-08-19T10:18:24Z

src/compiler/checker.ts

-                        if (isTypeIdenticalTo((<ConditionalType>source).extendsType, (<ConditionalType>target).extendsType) &&
-                            (isRelatedTo((<ConditionalType>source).checkType, (<ConditionalType>target).checkType) || isRelatedTo((<ConditionalType>target).checkType, (<ConditionalType>source).checkType))) {
+                        const u1 = instantiateType((<ConditionalType>source).extendsType, reportUnmeasurableMarkers);
+                        const u2 = instantiateType((<ConditionalType>target).extendsType, reportUnmeasurableMarkers);


The source and target are identical sans type parameter instantiations during variance checking, so I've been wondering if we can get away with only instantiating one of the source or target.

Yeah that feels like it should work. At least, I don't see how replacing a type parameter with a sub/super type of itself could affect whether it gets instantiated.

falsandtru · 2019-08-29T21:48:54Z

Can you merge?

falsandtru · 2019-12-18T15:14:12Z

I found a regression on https://github.com/falsandtru/spica.

src/list.ts(46,13): Error TS2352: Conversion of type '(as extends [unknown, unknown, ...unknown[]] ? as : never)["length"] extends 1 ? Nil : Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' to type 'Cons<Split<as>[1]>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Nil | Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<Split<as>[1]>'.
    Type 'Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<Split<as>[1]>'.
      Types of property 'tail' are incompatible.
        Type 'Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]["length"] extends 1 ? Nil : Cons<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>' is not comparable to type 'Split<as>[1]["length"] extends 1 ? Nil : Cons<Split<Split<as>[1]>[1]>'.
          Type 'Cons<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>' is not comparable to type 'Cons<Split<Split<as>[1]>[1]>'.
            Types of property 'tail' are incompatible.
              Type 'Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>' is not comparable to type 'Split<Split<as>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<as>[1]>[1]>[1]>'.
                Type 'Cons<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>' is not comparable to type 'Cons<Split<Split<Split<as>[1]>[1]>[1]>'.
                  Types of property 'tail' are incompatible.
                    Type 'Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Split<Split<Split<as>[1]>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'.
                      Type 'Cons<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Cons<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'.      
                        The types returned by 'reverse()' are incompatible between these types.
                          Type 'Reverse<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Reverse<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'.

falsandtru · 2019-12-18T22:36:05Z

The error come from this.tail as Cons<Split<as>[1]> but the error message of this.tail as Cons<Split<AtLeast<2, unknown>>[1]> would be more readable.

Conversion of type '(as extends [unknown, unknown, ...unknown[]] ? as : never)["length"] extends 1 ? Nil : Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' to type 'Cons<[unknown, ...unknown[]]>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Nil | Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<[unknown, ...unknown[]]>'.
    Type 'Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<[unknown, ...unknown[]]>'.
      Types of property 'CONS' are incompatible.
        Type 'Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]' is not comparable to type '[unknown, ...unknown[]]'.
          Type 'Split<as>[1]' is not comparable to type '[unknown, ...unknown[]]'.
            Property '0' is missing in type 'unknown[]' but required in type '[unknown, ...unknown[]]'.ts(2352)

At the following place,

          Type 'Split<as>[1]' is not comparable to type '[unknown, ...unknown[]]'.
            Property '0' is missing in type 'unknown[]' but required in type '[unknown, ...unknown[]]'.ts(2352)

Split<as>[1] must be [unknown, ...unknown[]] but looks like the compiler assumes unknown[].

DanielRosenwasser · 2020-01-11T19:20:08Z

@jack-williams pointed out this is still open on #36138. Did you get the chance to take a look at this @ahejlsberg?

falsandtru · 2020-01-27T01:16:53Z

#31277 (comment) is missing. Can you pack again?

jack-williams · 2020-01-27T08:29:10Z

@typescript-bot pack this

typescript-bot · 2020-01-27T08:29:16Z

Heya @jack-williams, I've started to run the tarball bundle task on this PR at aa4138d. You can monitor the build here. It should now contribute to this PR's status checks.

falsandtru · 2020-01-27T08:51:04Z

Hm, the task has failed.

…nds-type-is-unmeasurable

jack-williams · 2020-01-27T12:25:07Z

@typescript-bot pack this

typescript-bot · 2020-01-27T12:25:12Z

Heya @jack-williams, I've started to run the tarball bundle task on this PR at 81943cc. You can monitor the build here. It should now contribute to this PR's status checks.

typescript-bot · 2020-01-27T12:40:26Z

Hey @jack-williams, 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/61972/artifacts?artifactName=tgz&fileId=053FA4BCC915C26D801DF489ED85164BFCAFFD0BBA6F61B75D49F244F4916BDF02&fileName=/typescript-3.8.0-insiders.20200127.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

falsandtru · 2020-01-27T12:54:58Z

Thanks. I cleaned the related type functions by removing unnecessary constraints but that made no effect. The current errors are as follows.

spica/src/list.ts(46,13): Error TS2352: Conversion of type '(as extends [unknown, unknown, ...unknown[]] ? as : never)["length"] extends 1 ? Nil : Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' to type 'Cons<Split<as>[1]>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Nil | Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<Split<as>[1]>'.
    Type 'Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<Split<as>[1]>'.
      Types of property 'tail' are incompatible.
        Type 'Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]["length"] extends 1 ? Nil : Cons<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>' is not comparable to type 'Split<as>[1]["length"] extends 1 ? Nil : Cons<Split<Split<as>[1]>[1]>'.
          Type 'Cons<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>' is not comparable to type 'Cons<Split<Split<as>[1]>[1]>'.
            Types of property 'tail' are incompatible.
              Type 'Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>' is not comparable to type 'Split<Split<as>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<as>[1]>[1]>[1]>'.
                Type 'Cons<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>' is not comparable to type 'Cons<Split<Split<Split<as>[1]>[1]>[1]>'.
                  Types of property 'tail' are incompatible.
                    Type 'Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Split<Split<Split<as>[1]>[1]>[1]["length"] extends 1 ? Nil : Cons<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'. 
                      Type 'Cons<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Cons<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'.
                        The types returned by 'reverse()' are incompatible between these types.
                          Type 'Reverse<Split<Split<Split<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>[1]>[1]>[1]>' is not comparable to type 'Reverse<Split<Split<Split<Split<as>[1]>[1]>[1]>[1]>'.

spica/src/list.ts(46,13): Error TS2352: Conversion of type '(as extends [unknown, unknown, ...unknown[]] ? as : never)["length"] extends 1 ? Nil : Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' to type 'Cons<[unknown, ...unknown[]]>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Nil | Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<[unknown, ...unknown[]]>'.
    Type 'Cons<Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]>' is not comparable to type 'Cons<[unknown, ...unknown[]]>'.
      Types of property 'CONS' are incompatible.
        Type 'Split<as extends [unknown, unknown, ...unknown[]] ? as : never>[1]' is not comparable to type '[unknown, ...unknown[]]'.
          Type 'Split<as>[1]' is not comparable to type '[unknown, ...unknown[]]'.
            Property '0' is missing in type 'unknown[]' but required in type '[unknown, ...unknown[]]'.

sandersn · 2020-03-03T23:53:13Z

It sounds like this PR is currently blocked on #30639. Is that a correct reading of the comment history @jack-williams?

jack-williams · 2020-03-04T08:45:46Z

@sandersn Blocked may be too strong, but I think it makes sense to merge #30639 first, if possible. This PR makes the compiler check more conditional types structurally, so having more relation rules seems good.

I believe #30639 would change some of the breaks introduced by this PR, but I haven't checked DT in a while.

weswigham · 2021-04-29T10:58:35Z

@jack-williams #30639 got merged this release! Do we need to refresh/reevaluate breaks? I imagine so since the last update in here was march of last year.

…nds-type-is-unmeasurable

jack-williams · 2021-04-29T14:31:54Z

@weswigham I've started trying to revive this. There is one new regression in the compiler tests which I have not been able to figure out yet, but I have been able to reduce to a small example:

type EqualsTest<T> = <A>() => A extends T ? 1 : 0;
type EqualsTest1<T> = <A>() => A extends T ? 1 : 0;

const x: EqualsTest<number> = undefined as any as EqualsTest<string>; // was error, now no error
const y: EqualsTest<number> = undefined as any as EqualsTest1<string>; // error;

Both used to fail, now only y does.

weswigham · 2021-04-29T17:01:55Z

Information loss during generic signature relating due to local type parameters erasure is I think likely at fault there, like I (tried to) explain in #43867.

weswigham · 2021-04-29T20:10:21Z

Specifically the self-assignment case lacks the error because we have a "fast path" in signature comparison (in signaturesRelatedTo) for when we're relating two signatures from the same type in which both copies of the signature have their type parameters erased to any, which in turn causes us to fully evaluate the conditional (to a union of both branches) when we do the structural comparison, which is why we fail to issue an error. Type erasure has another outstanding issue - #41067 - which I don't think would help here, since the wildcard is just a special any, so the conditionals would still incorrectly execute and eliminate the extends clause, but I think points to a possible solution - we need the "erased" type parameter to be assignable like an any, but still prevent execution of conditionals, and thus look like a generic. That's neither any, nor the wildcardType right now, but I don't think it'd be too bad to clone one of the two and consider it "instantiable" in the context of constructing a conditional (and thus prevent its execution).

I have just put together a composition of this change, the change in #41067, and what I just explained, which seems to neatly tie all this together and get our conditionals-in-generic-signatures relationships to where I think it should be. I've put up #43887 with that.

jack-williams · 2021-04-29T21:14:27Z

Thanks @weswigham, your description on the first thread was clear, I just didn't read it in enough detail. The any from the generic erasure causing problems makes sense.

I will close this up in favor of your new PR. Without your change this PR break Equal from ts-toolbelt, and likely other type heavy packages.

Do not measure variance for a conditional type extendsType

5e81ad4

RyanCavanaugh requested a review from weswigham May 20, 2019 22:48

RyanCavanaugh assigned weswigham May 20, 2019

RyanCavanaugh requested a review from ahejlsberg May 20, 2019 22:48

Merge branch 'master' into variance-for-extends-type-is-unmeasurable

70336e6

Merge branch 'master' into variance-for-extends-type-is-unmeasurable

52c7dff

weswigham reviewed Aug 19, 2019

View reviewed changes

jack-williams mentioned this pull request Jan 11, 2020

Design Meeting Notes, 1/10/2020 #36138

Closed

Merge remote-tracking branch 'upstream/master' into variance-for-exte…

81943cc

…nds-type-is-unmeasurable

sandersn added the For Milestone Bug PRs that fix a bug with a specific milestone label Feb 1, 2020

weswigham mentioned this pull request Apr 29, 2021

redux-orm broken by #43624 #43867

Open

jack-williams added 2 commits April 29, 2021 12:21

Merge remote-tracking branch 'upstream/master' into variance-for-exte…

fc23686

…nds-type-is-unmeasurable

Promote reportUnmeasurableMarkers to type mapper for conditional type

51f7d27

weswigham mentioned this pull request Apr 29, 2021

Mark conditional extends as Unmeasurable and use a conditional-opaque wildcard for type erasure #43887

Closed

jack-williams closed this Apr 29, 2021

Do not measure variance for a conditional type extendsType #31277

Do not measure variance for a conditional type extendsType #31277

Conversation

jack-williams commented May 6, 2019

falsandtru commented May 20, 2019

jack-williams commented May 21, 2019

falsandtru commented May 24, 2019

falsandtru commented May 27, 2019

weswigham commented Jun 11, 2019

jack-williams commented Jun 12, 2019

weswigham commented Jun 12, 2019

typescript-bot commented Jun 12, 2019 • edited Loading

typescript-bot commented Jun 12, 2019 • edited Loading

falsandtru commented Jun 16, 2019

weswigham commented Jun 16, 2019

typescript-bot commented Jun 16, 2019 • edited Loading

typescript-bot commented Jun 16, 2019

falsandtru commented Jun 16, 2019

jack-williams commented Jun 16, 2019

falsandtru commented Jun 16, 2019

jack-williams commented Jun 16, 2019

falsandtru commented Jun 16, 2019

jack-williams commented Aug 18, 2019

typescript-bot commented Aug 18, 2019 • edited Loading

jack-williams commented Aug 19, 2019

weswigham commented Aug 19, 2019

weswigham Aug 19, 2019

Choose a reason for hiding this comment

jack-williams Aug 21, 2019

Choose a reason for hiding this comment

falsandtru commented Aug 29, 2019

falsandtru commented Dec 18, 2019

falsandtru commented Dec 18, 2019

DanielRosenwasser commented Jan 11, 2020

falsandtru commented Jan 27, 2020

jack-williams commented Jan 27, 2020

typescript-bot commented Jan 27, 2020 • edited Loading

falsandtru commented Jan 27, 2020

jack-williams commented Jan 27, 2020

typescript-bot commented Jan 27, 2020 • edited Loading

typescript-bot commented Jan 27, 2020 • edited Loading

falsandtru commented Jan 27, 2020

sandersn commented Mar 3, 2020

jack-williams commented Mar 4, 2020

weswigham commented Apr 29, 2021

jack-williams commented Apr 29, 2021

weswigham commented Apr 29, 2021

weswigham commented Apr 29, 2021 • edited Loading

jack-williams commented Apr 29, 2021

typescript-bot commented Jun 12, 2019 •

edited

Loading

typescript-bot commented Jun 12, 2019 •

edited

Loading

typescript-bot commented Jun 16, 2019 •

edited

Loading

typescript-bot commented Aug 18, 2019 •

edited

Loading

typescript-bot commented Jan 27, 2020 •

edited

Loading

typescript-bot commented Jan 27, 2020 •

edited

Loading

typescript-bot commented Jan 27, 2020 •

edited

Loading

weswigham commented Apr 29, 2021 •

edited

Loading