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

Type Support: Properties with conditional types #835

Open
mlmoravek opened this issue Mar 2, 2024 · 5 comments
Open

Type Support: Properties with conditional types #835

mlmoravek opened this issue Mar 2, 2024 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mlmoravek
Copy link
Member

mlmoravek commented Mar 2, 2024

Description

Some components work with different modelValue values based on another property.
For example, the modelValue property of the dropdown component can be a single value, or if the multiple property is true, the modelValue property will be an array of values.

These type distinctions could be solved by using conditional types for such use cases.


Help wanted:
But I don't know how to implement conditional types with the composition api. A similar question can be found here: stackoverflow.

Any suggestions for implementation are welcome.

Why Oruga need this feature

Improve component TypeScript support.

@mlmoravek mlmoravek added enhancement New feature or request help wanted Extra attention is needed labels Mar 2, 2024
@blm768
Copy link
Contributor

blm768 commented Mar 4, 2024

My first thought was to do something along these lines:

diff --git a/packages/oruga/src/components/dropdown/Dropdown.vue b/packages/oruga/src/components/dropdown/Dropdown.vue
index 589d4e3f..0ea6ed52 100644
--- a/packages/oruga/src/components/dropdown/Dropdown.vue
+++ b/packages/oruga/src/components/dropdown/Dropdown.vue
@@ -1,5 +1,12 @@
 <script setup lang="ts">
-import { computed, nextTick, ref, watch, type PropType } from "vue";
+import {
+    computed,
+    nextTick,
+    ref,
+    watch,
+    type PropType,
+    type ComponentObjectPropsOptions,
+} from "vue";
 
 import PositionWrapper from "../utils/PositionWrapper.vue";
 
@@ -35,11 +42,6 @@ defineOptions({
 const props = defineProps({
     /** Override existing theme classes completely */
     override: { type: Boolean, default: undefined },
-    /** @model */
-    modelValue: {
-        type: [String, Number, Boolean, Object, Array],
-        default: undefined,
-    },
     /** The active state of the dropdown, use v-model:active to make it two-way binding. */
     active: { type: Boolean, default: false },
     /** Trigger label, unnecessary when trgger slot is used */
@@ -85,8 +87,6 @@ const props = defineProps({
         type: String,
         default: () => getOption("dropdown.animation", "fade"),
     },
-    /** Allows multiple selections */
-    multiple: { type: Boolean, default: false },
     /** Trap focus inside the dropdown. */
     trapFocus: {
         type: Boolean,
@@ -237,6 +237,27 @@ const props = defineProps({
         type: [String, Array, Function] as PropType<ComponentClass>,
         default: undefined,
     },
+    ...({
+        /** @model */
+        modelValue: {
+            type: [String, Number, Boolean, Object, Array],
+            default: undefined,
+        },
+        /** Allows multiple selections */
+        multiple: { type: Boolean, default: false },
+    } satisfies ComponentObjectPropsOptions as
+        | {
+              modelValue: {
+                  type: PropType<string | number | boolean | object>;
+              };
+              multiple: { type: PropType<false> };
+          }
+        | {
+              modelValue: {
+                  type: PropType<(string | number | boolean | object)[]>;
+              };
+              multiple: { type: PropType<true> };
+          }),
 });
 
 const emits = defineEmits<{

I'm not sure that this will actually play nicely with the defineProps macro, though, and based on some rough testing I was doing, I don't think the type inference quite cooperates as desired.

@mlmoravek
Copy link
Member Author

I maybe found a solution explained in this quide: https://medium.com/@ademyalcin27/crafting-type-safe-components-in-vue-js-with-typescript-3724276c6155

I created an example PR for the input component #884.

@blm768
Copy link
Contributor

blm768 commented Apr 3, 2024

Based on a quick glance at the PR, it seems like a pretty reasonable approach. I'd be tempted to use T as the generic argument instead of TNumber, although I don't know if that would play well with TypeScript's type inference.

@messenjer
Copy link
Contributor

messenjer commented Apr 5, 2024

And a better name for the type T more readable and self-documenting, making it easier to understand the purpose and function of this type.

type T = TNumber extends true ? number : string;

-->

type ConditionalNumberOrString = TNumber extends true ? number : string;
    /**
     * @type string | number
     * @model
     */
    modelValue?: ConditionalNumberOrString;

@mlmoravek
Copy link
Member Author

Hmm, I don't like that it is not possible to define the conditional type with the runtime declaration approach and we have to use the type-based declaration approach with the withDefaults compile macro...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants