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

withDefaults disables the use of discriminated unions (only in scripts, not in templates) #9335

Closed
davidmatter opened this issue Oct 4, 2023 · 7 comments · Fixed by #9336
Labels
has PR A pull request has already been submitted to solve the issue scope: types

Comments

@davidmatter
Copy link
Contributor

Vue version

3.3.4

Link to minimal reproduction

https://play.vuejs.org/#eNrtVcFu2kAQ/ZWRLyZSsIXaEwWapsmhVdJGTdQe4hxcPIZN7F1rdw20hH/v7NgGQ2hKpPbWC4SdN2/ezM6+LL13RRHMSvT63sCMtSgsGLRlAVksJ8PIsybyYIIStRjTzxvAhUWZGDBWCzmJvFEkIynyQmkL71VeQKpVDpEXhO6Xo468LUBvG9FrIJEchJUCphxYzIsstki/BkwM/VwlmHVncVYiabn1LRrr35FAF6CTvMysKDKiC5us3ovSwIif7uSC/mSKcK3CO6ZZjJVMxSS4N0rSwJaRBOpjTGVEhvpzYYWSNK8+cMTF4ixT8498ZnWJx835eIrjhz3n92bhziLvSqNBPSNN65iN9QRtFT6//kQX0QpSK6Xr4JngFzQqK53GCnZayoRkt3Cs9gNfFd3tjTl3d22appxQh1wxPvLo3tyEf9f6Ru6r4BXnRXJFU2zW4tCN4xWr92cJbtilxQRW1Rr5xOS/cQtjfxQIp7HBK60KA8NKibvQt33wL3x4BP+SP699UsJrKy3qNB4jcM6l24P1gm+omMjtCvE0u0IU1Vn21e1VH25u7/azXtMoM/wDrWHQU9Kakntr+mqzPraUOyAtqLFQ1Mi5sNMzTGOKmk6CqZBV6QF/jjpHx5sh8Yyo3NGGZkEUzbw7nSMYjiq4SKHDJQKnFYbDYWsuR80GaLpULSstDKyaCu6VkB2qwguxhvnr2s/5wLQ3OlNopG9hrvQDN7jV5SAkCAETMYNZV6TudT/VyCsFUKNSpR0MhGzNvoYALJf5akWxFDXXNA7XaGKWkGjYK+pv97VcLtZZSVsxJddLz35EgpvsttNU697N42LHa6rA9kNj/3QPemptYfphOE4kpVEnYqYDiTaURR6eECzUpbQix26i8hN6lcFrqm5s+zhAk3e/azUn9yGSljm4fLYk3dW0yaiddRxWdietXXon9KT8jnHwf4u/7Rz/vWOPd+yzi39sDbuewFM5xBO+8btkM2jLfokXHOgGa0eoc3bffusVr34BHdY3dA==

Steps to reproduce

Check Comp.vue - in the template, the discriminated union can be inferred correctly. Not so in the script.
This is probably because withDefaults uses Omit to combine the required and optional props and because Omit follows the following behavior: microsoft/TypeScript#31501.

I'm not sure why the discriminated union is inferred correctly in the template, though.
defineProps itself, without defaults, works correctly - check Comp1.vue.

image

What is expected?

Discriminated unions should work in scripts as well as in templates.

What is actually happening?

Only works in templates.

System Info

No response

Any additional comments?

No response

@davidmatter
Copy link
Contributor Author

Added PR #9336 - my first one, be kind :)

@haoqunjiang haoqunjiang added scope: types has PR A pull request has already been submitted to solve the issue labels Oct 9, 2023
@andredewaard
Copy link

andredewaard commented Dec 4, 2023

Looks good! Question, how can we use this in combination with defineModel?
So have mode: multiple on the defineProps type and modelValue: T[] on defineModel.

interface BaseProps {
  label: string
}

interface PropsMulti extends BaseProps {
  mode: 'multiple'
  modelValue: T[]
}

interface PropsSingle extends BaseProps {
  mode: 'single'
  modelValue: T
}

type Props = PropsMulti | PropsSingle

const props = withDefaults(defineProps<Omit<Props, 'modelValue'>>(), {
  mode: 'single'
})
const modelValue = defineModel<Pick<Props, 'modelValue'>>('modelValue', { required: true })

I don't think this is doable at all?

@davidmatter
Copy link
Contributor Author

Good point! I too have been wondering how to connect defineProps and defineModel on a type level with regards to discriminated unions. Let me think about it. Maybe @pikax has an idea too?

@pikax
Copy link
Member

pikax commented Dec 4, 2023

@andredewaard defineModel is a macro to simplify the model manipulation, you won't be able to discriminate the type in your example, since those are two completely different types. As an exercise I would recommend trying to achieve that in plain Typescript :)

@StepanMynarik
Copy link

Tried the same thing as @andredewaard , I just used:
const modelValue = defineModel<Props['modelValue']>({ required: true })
instead of:
const modelValue = defineModel<Pick<Props, 'modelValue'>>('modelValue', { required: true })

This compiles, but the property type just includes all possible types of 'modelValue' property (both T and T[]).
And I don't think it is possible to solve this, since as @pikax says, these are two separate types (the omit one used in defineProps and lookup one used in defineModel).
So TypeScript can't infer the type properly.

My conclusion - don't use defineModel in components where dicriminated union props are required.
A little unfortunate, not being able to use the new feature always, but hey... what can you do 😆

@andredewaard
Copy link

@StepanMynarik Thanks, i think i would be using the old way with modelValue and emit the only problem with this is that we still emit all possible options because they are separate types.
Example:

export type OptionProps = {
  value: string | number
  label: string
}

interface BaseProps {
  options: OptionProps[]
}

interface PropsSingle extends BaseProps {
  modelValue: OptionProps['value']
  multiple: false
}

interface PropsMulti extends BaseProps {
  modelValue: [OptionProps['value']]
  multiple: true
}

type Props = PropsMulti | PropsSingle

const props = defineProps<Props>()

const emit = defineEmits<{
  (event: 'update:modelValue', value: Props['modelValue']): void
}>()
Screenshot 2024-01-26 at 10 16 45

@yyx990803
Copy link
Member

This has been fixed by #10596

@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR A pull request has already been submitted to solve the issue scope: types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants