Skip to content
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

fix(runtime-core): add InferDefaults handling of array and object union types #4925

Merged
merged 2 commits into from
Nov 15, 2021

Conversation

cathrinevaage
Copy link
Contributor

This fixes an issue where InferDefaults fails to handle union types with base types and array or object types.

TS2322: Type 'number' is not assignable to type '((props: Readonly<{ foo: number | number[]; }>) => number | number[]) | undefined'.
would throw for

withDefault(
  defineProps<{ foo: number | number[] }>(),
  { foo: 123 },
)

This adds handling of base types and array or object types separately, so that both types can exist in prop type unions.

cathrinevaage and others added 2 commits November 9, 2021 14:42
…on types

This fixes an issue where `InferDefaults` fails to handle union types with base types and array or object types.

`TS2322: Type 'number' is not assignable to type '((props: Readonly<{ foo: number | number[]; }>) => number | number[]) | undefined'.`
would throw for
```ts
withDefault(
  defineProps<{ foo: number | number[] }>(),
  { foo: 123 },
)
```

This adds handling of base types and array or object types separately, so that both types can exist in prop type unions.
{
[K in keyof Defaults]: K extends keyof Base ? NotUndefined<Base[K]> : never
}
type InferDefault<P, T> = T extends
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original logic seemed unnecessarily complex so I refactored it - thanks for the fix and the test case though!

@yyx990803 yyx990803 merged commit 04e5835 into vuejs:master Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants