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

defineProps compiler macro can't handle imported types #157

Closed
NozomuIkuta opened this issue Oct 18, 2022 · 1 comment · Fixed by #161
Closed

defineProps compiler macro can't handle imported types #157

NozomuIkuta opened this issue Oct 18, 2022 · 1 comment · Fixed by #161
Assignees
Labels
bug Something isn't working

Comments

@NozomuIkuta
Copy link
Contributor

The official docs says that:

In dev mode, the compiler will try to infer corresponding runtime validation from the types. For example here foo: String is inferred from the foo: string type. If the type is a reference to an imported type, the inferred result will be foo: null (equal to any type) since the compiler does not have information of external files.

import type { TableCellPillColor } from '../composables/Table'
const props = defineProps<{
value?: any
record: any
getter?: string | ((value: any) => string)
color?: TableCellPillColor | ((value: any) => TableCellPillColor)
}>()

color?: TableCellPillColor | ((value: any) => TableCellPillColor) 

// will be converted to prop type validation logic like below

[null, function()]

I tried to use a function like color: () => 'success', but then browser throws an error of "Maximum call stack size exceeded", if there are lots of table cells that call the function synchronously.


I discussed with @kiaking how we should handle this issue until Vue 3 supports props of imported types, and here is our conclusion.

  • Define types (e.g. TableCellPillColor in this time) in 2 places

    • One is where the type is supposed to be defined (e.g. composables/Table in this time)

    • The other is where the type is actually used for defineProps compiler macro

      type TableCellPillColor = 'info' | 'success' | 'warning' | 'danger' | 'mute'
      
      const props = defineProps<{
        color?: TableCellPillColor | ((value: any) => TableCellPillColor)
      }>()
  • We don't duplicate all the possible types in 2 places at once: rather, we do it for each type as we found such kind of error messages

  • Once defineProps macro supports props of imported types, we delete the duplicated type definition

  • Until then, we will make it sure by careful PR review that type definition is defined/managed in 2 places


If you have any other good ideas, please let us know. 🙋‍♂️

@NozomuIkuta NozomuIkuta added the bug Something isn't working label Oct 18, 2022
@NozomuIkuta NozomuIkuta changed the title defineProps compiler macro can't handle imported types defineProps compiler macro can't handle imported types Oct 19, 2022
@kiaking kiaking self-assigned this Oct 20, 2022
@kiaking
Copy link
Member

kiaking commented Oct 20, 2022

I'll take this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants