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(language-core): correctly infer union props to options #4908

Closed
wants to merge 4 commits into from

Conversation

KazariEX
Copy link
Member

@KazariEX KazariEX commented Oct 5, 2024

Copy link

pkg-pr-new bot commented Oct 5, 2024

Open in Stackblitz

vue-component-meta

pnpm add https://pkg.pr.new/vuejs/language-tools/vue-component-meta@4908

vue-component-type-helpers

pnpm add https://pkg.pr.new/vuejs/language-tools/vue-component-type-helpers@4908

@vue/language-core

pnpm add https://pkg.pr.new/vuejs/language-tools/@vue/language-core@4908

@vue/language-plugin-pug

pnpm add https://pkg.pr.new/vuejs/language-tools/@vue/language-plugin-pug@4908

@vue/language-server

pnpm add https://pkg.pr.new/vuejs/language-tools/@vue/language-server@4908

@vue/language-service

pnpm add https://pkg.pr.new/vuejs/language-tools/@vue/language-service@4908

vue-tsc

pnpm add https://pkg.pr.new/vuejs/language-tools/vue-tsc@4908

@vue/typescript-plugin

pnpm add https://pkg.pr.new/vuejs/language-tools/@vue/typescript-plugin@4908

commit: 858cb81

@KazariEX KazariEX changed the title fix(language-core): infer union props to options correctly fix(language-core): correctly infer union props to options Oct 5, 2024
@darkbasic
Copy link

Thanks, I've tested your patch and it fixed my issue.

@KazariEX KazariEX closed this Nov 2, 2024
@KazariEX KazariEX deleted the fix/union-props-options branch November 2, 2024 11:46
@hendrikheil
Copy link

@KazariEX I think this would be a great addition. Currently there are typescript errors when you do the following:

<script setup lang="ts">
import { useCssModule } from 'vue';

const styles = useCssModule();

defineProps<
  | {
      option: 'a';
      aValue: string;
    }
  | {
      option: 'b';
      bValue: number;
    }
>();
</script>

<template>
  <div :class="styles.example">Option: {{ option }}</div>
</template>

<style lang="css" module>
.example {
  color: red;
}
</style>

As soon as you add the style module tag, vue and typescript are unable to properly see props and things like imported components. I tested your code locally and it seemed to have made the typescript errors in the template tag go away immediately. Do you think it makes sense to reconsider this PR?

@KazariEX
Copy link
Member Author

KazariEX commented Dec 5, 2024

According to Johnson's suggestion, a better approach is to place union types into generics to skip redundant type inferring and directly use the original types, which avoids injecting too much difficult to maintain code.

@darkbasic
Copy link

There are many issues with that approach. I started writing a comment about that but then I stumbled upon an even bigger bug regarding union props and unfortunately I still have to file it.

@hendrikheil
Copy link

I did try the generics approach as well, but that does cause other issues for me as well. I'll try to create a minimal component reproduction of the errors I'm encountering with it and post it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants