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): withDefaults & union types #11949

Closed
wants to merge 1 commit into from

Conversation

davidmatter
Copy link
Contributor

Improves typing of props when using union types in combination with withDefaults.

Fixes

Affects

#9125

Explanation

We can skip the Omit<T, Defaults> in

Readonly<Omit<T, keyof Defaults>>

because we're overloading the omitted keys with key/value of Defaults and the mapped type modifier anyways:

  & {
    readonly [K in keyof Defaults as K extends keyof T ? K : never]-?: ...

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 100 kB 37.9 kB 34.1 kB
vue.global.prod.js 159 kB 57.8 kB 51.4 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 48.8 kB 18.8 kB 17.2 kB
createApp 55.4 kB 21.3 kB 19.4 kB
createSSRApp 59.4 kB 23 kB 21 kB
defineCustomElement 60.2 kB 22.9 kB 20.9 kB
overall 69.1 kB 26.4 kB 24 kB

Copy link

pkg-pr-new bot commented Sep 17, 2024

Open in Stackblitz

@vue/compiler-core

pnpm add https://pkg.pr.new/@vue/compiler-core@11949

@vue/compiler-dom

pnpm add https://pkg.pr.new/@vue/compiler-dom@11949

@vue/compiler-sfc

pnpm add https://pkg.pr.new/@vue/compiler-sfc@11949

@vue/compiler-ssr

pnpm add https://pkg.pr.new/@vue/compiler-ssr@11949

@vue/reactivity

pnpm add https://pkg.pr.new/@vue/reactivity@11949

@vue/runtime-core

pnpm add https://pkg.pr.new/@vue/runtime-core@11949

@vue/runtime-dom

pnpm add https://pkg.pr.new/@vue/runtime-dom@11949

@vue/server-renderer

pnpm add https://pkg.pr.new/@vue/server-renderer@11949

@vue/shared

pnpm add https://pkg.pr.new/@vue/shared@11949

vue

pnpm add https://pkg.pr.new/vue@11949

@vue/compat

pnpm add https://pkg.pr.new/@vue/compat@11949

commit: ee315b6

@jh-leong
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Sep 17, 2024

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools failure failure
nuxt success success
pinia success success
primevue success success
quasar success success
radix-vue success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros failure success
vuetify success success
vueuse success success
vue-simple-compiler success success

@edison1105
Copy link
Member

#9125, #11758, and #8952 are all runtime issues. This PR doesn't really solve them.

@davidmatter
Copy link
Contributor Author

davidmatter commented Sep 18, 2024

@edison1105 what's the runtime issue of #11758 and #8952?

The way I see it: There are runtime issues in regards to default props as well as type issues with certain typescript constructs (union types) when using the withDefaults notation.

@edison1105
Copy link
Member

@davidmatter

  color:{
    type:String,
    validator:(value, props)=>{
      if(value==='white'){
        return props.appearance === 'outline'
      }else{
        //...
      }
    }
  },

@davidmatter
Copy link
Contributor Author

Ah, yes. Thanks for the clarification. I think I will use this PR to add the needed runtime changes. Btw: My assumption is that changing the props options interface to allow computed required would be even better than "misusing" prop validators for this. With computed require we could just generate simple logic based on the TS union type definition.

...
  required?: boolean | ((props: Data) => boolean)
...

The following behavior in the compiler will need to be changed as well.

 case 'TSUnionType':
 case 'TSIntersectionType':
      return mergeElements(
        node.types.map(t => resolveTypeElements(ctx, t, scope, typeParameters)),
        node.type,
      )

@davidmatter davidmatter marked this pull request as draft September 18, 2024 12:03
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.

4 participants