Skip to content

TS 5.1.0 dev type inference regression when combining conditional/intersection/recursive type #53211

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

Closed
HerringtonDarkholme opened this issue Mar 12, 2023 · 4 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Mar 12, 2023

Bug Report

Vue team is witnessing a new weird type behavior difference when upgrading TypeScript.
The typing difference is very subtle. I'm sorry I cannot provide a more concise example here. (The example has already been reduced from 500+ lines to 70 lines ).

The original usage is simple https://tsplay.dev/mxjRXw. I have attached the standalone example in the issue. All critical parts are commented.

🔎 Search Terms

Sorry I could not find a good search term.

🕗 Version & Regression Information

  • This changed between versions 4.9.2 and 5.1.0dev

⏯ Playground Link

5.1.0

4.9.5

💻 Code

type ComponentPublicInstance<C, Options> = {
  $options: Options & 'intersection anything'  // intersection here is critical
} & ExtractComputedReturns<C>   // this type intersection is important

type WhatEver<T> = T extends 1 ? true : false  

type MixinToOptionTypes<T> = T extends ComponentOptionsBase<
  infer C, 1, 1
>
  ? OptionTypesType<C>
  : never

type IntersectionMixin<T> = WhatEver<T> extends true  // a conditional is required here for reproduction
  ? OptionTypesType<{}>
  : MixinToOptionTypes<T>

type CreateComponentPublicInstance<
  Mixin extends 1 ,
  Extends extends 1,
> = ComponentPublicInstance<
  (IntersectionMixin<Mixin> & IntersectionMixin<Extends>)['C'],
  ComponentOptionsBase<{}, Mixin, Extends>
>

interface ComponentOptionsBase<
  _C extends ComputedOptions,
  Mixin extends 1,
  Extends extends 1,
>  {
  data?: (
    this: CreateComponentPublicInstance<
      Mixin,
      Extends
    >,
  ) => {}  // a function property is required, method syntax cannot reproduce.
}

type ComputedGetter = (...args: any[]) => any
type ComputedOptions = Record<
  string,
  ComputedGetter | {get: ComputedGetter }  // a union type for getter is required
>

type ExtractComputedReturns<T extends any> = {
  [key in keyof T]: T[key] extends { get: (...args: any[]) => infer TReturn }
    ? TReturn
    : T[key] extends (...args: any[]) => infer TReturn
    ? TReturn
    : never
}

type OptionTypesType<C = {}> = { C: C }

type Mixin = 1; type Extend = 1;

type A1 = ComponentOptionsBase<any,Mixin,Extend> &{sth?:never}
export type Test1 = A1 extends ComponentOptionsBase<any,any,any>?'yes':'no'
//   ^? expect Yes, got No in TS 5.1.0, but Yes in 4.9.5

// following types are for reference. They behave the same.
type A2 = ComponentOptionsBase<any,Mixin,Extend>
export type Test2 = A2 extends ComponentOptionsBase<any,any,any>?'yes':'no'
//   ^?  yes

type A3 = ComponentOptionsBase<any,Mixin,any> &{sth?:never}
export type Test3 = A3 extends ComponentOptionsBase<any,any,any>?'yes':'no'
//   ^? yes

type A4 = ComponentOptionsBase<any,any,Extend> &{sth?:never}
export type Test4 = A4 extends ComponentOptionsBase<any,any,any>?'yes':'no'
//   ^? yes

🙁 Actual behavior

A1 should always be 'yes' in TS 5.1.0.

🙂 Expected behavior

A1 is 'no' in TS5.1.0, but 'yes' in TS4.9.5

@HerringtonDarkholme HerringtonDarkholme changed the title TS 5.1.0 dev gave wrong answer when combining conditional/intersection/recursive type TS 5.1.0 dev type inference regression when combining conditional/intersection/recursive type Mar 12, 2023
@Andarist
Copy link
Contributor

This changed in #52392 , cc @ahejlsberg

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Apr 4, 2023
@RyanCavanaugh
Copy link
Member

It's interesting that this changed, but given that the originating PR is a bug fix, this needs a clearer demonstration (i.e. short repro, or one that clearly shows why something is wrong) of an actual defect for us to investigate further.

@natew
Copy link

natew commented Jun 7, 2023

Tamagui is breaking after upgrading to 5.1, but it's also hard to set up an exact reproduction (yet easy to see in the monorepo). If you run yarn up typescript and then yarn build you can see a ton of type errors coming in, it seems due to a very simple GetVariantProps helper which just does:

type GetVariantProps<A extends StylableComponent> = A extends TamaguiComponent<
  any,
  any,
  any,
  infer V
>
  ? V
  : {}

I can see the type is TamaguiComponent and the fourth argument is a real value, but for some reason in TS 5.1 it returns unknown now. Will investigate further, but we're getting a lot of bug reports and confused users suddenly from this.

@natew
Copy link

natew commented Jun 7, 2023

Found the answer, I had defined TamaguiComponent like this:

export type TamaguiComponent<
  Props = any,
  Ref = any,
  BaseProps = {},
  VariantProps = {},
  ParentStaticProperties = {}
> = ReactComponentWithRef<Props, Ref> &
  StaticComponentObject<Props, Ref> &
  ParentStaticProperties

Basically we didn't use the BaseProps and VariantProps, I guess TS throws that information away. So I changed to:

export type TamaguiComponent<
  Props = any,
  Ref = any,
  BaseProps = {},
  VariantProps = {},
  ParentStaticProperties = {}
> = ReactComponentWithRef<Props, Ref> &
  StaticComponentObject<Props, Ref> &
  ParentStaticProperties & {
    __baseProps: BaseProps
    __variantProps: VariantProps
  }

And it keeps the information around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

4 participants