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

Vue3: Improve CSF3 types #19602

Merged
merged 5 commits into from
Oct 25, 2022
Merged

Vue3: Improve CSF3 types #19602

merged 5 commits into from
Oct 25, 2022

Conversation

kasperpeulen
Copy link
Contributor

@kasperpeulen kasperpeulen commented Oct 24, 2022

What I did

This PR provides improved type safety for Vue3 stories (similar to what we changed to React and Svelte) but requires vue-tsc instead of tsc (And the Volar VS Code extension for editor support):
https://marketplace.visualstudio.com/items?itemName=Vue.vscode-typescript-vue-plugin

To unlock the full potential of this PR, TS 4.9 is needed, for the new satisfies operator. But, this also improves the types for TS<4.9 users.

Typesafe args

We changed StoryObj and Meta to increase type safety for when the user provides args partially in meta.

Considering a Component like this:

<script setup lang="ts">
defineProps<{ disabled: boolean; label: string }>();

const emit = defineEmits<{
    (e: 'myChangeEvent', id: number): void;
    (e: 'myClickEvent', id: number): void;
}>();

</script>

<template>
    <button :disabled="disabled" @change="emit('myChangeEvent', 0)" @click="emit('myClickEvent', 0)">
        {{ label }}
    </button>
</template>

<style scoped>

</style>

It is valid to provide args like this:

import Button from './Button.vue';

// ✅ valid
const meta = {
  component: Button,
  args: { disabled: false },
} satisfies Meta<typeof Button>;

// or const meta = Meta<typeof Button> = { ... } in TS<4.9

export default meta;

type Story = StoryObj<typeof meta>;

export const Basic: Story = {
  args: { label: 'good' }
};

While it is invalid to forget an arg, in either meta or the story:

// ❌ invalid
const meta = {
  component: Button,
} satisfies Meta<typeof Button>;

export const Basic: Story = {
  args: { label: 'good' }
};

// ❌ invalid
const meta = {
  component: Button,
  args: { label: 'good' }
} satisfies Meta<typeof Button>;

export const Basic: Story = {};

Changed Meta to make sure both a Component, as the Props of the component can be used:

const meta = {
  component: Button,
} satisfies Meta<typeof Button>;

export default meta;

export const meta = {
  component: Button,
} satisfies Meta<{label: string; disabled: boolean} >;

export default meta;

If the component is used, you will have enhanced autocompletion for events (maybe in a future version also for slots, when we support that):

const meta = {
  component: Button,
  args: {
    label: 'good',
    disabled: false,
    onMyChangeEvent: (value) => {
      expectTypeOf(value).toEqualTypeOf<number>();
    },
  },
} satisfies Meta<{ disabled: boolean; label: string }>;

Typesafe decorators/loaders

Decorators now accept a new generic arg argument to be specified:

const withDecorator: DecoratorFn<{ decoratorArg: string }> = (
  storyFn,
  { args: { decoratorArg } }
) => h(Decorator, { decoratorArg }, h(storyFn()));

And the type of meta/story will check if this arg is part of the generic type:

type Props = ButtonProps & { decoratorArg: string };

const meta = satisfies<Meta<Props>>()({
  component: Button,
  args: { disabled: false },
  decorators: [withDecorator],
});

type Story = StoryObj<typeof meta>;
const Basic: Story = { args: { decoratorArg: 0, label: 'good' } };

I also fixed some typescript issues in other files in this package.

How to test

You can test it by running: yarn nx run @storybook/vue3:check

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -27,11 +34,35 @@ export type StoryFn<TArgs = Args> = AnnotatedStoryFn<VueFramework, TArgs>;
*
* @see [Named Story exports](https://storybook.js.org/docs/formats/component-story-format/#named-story-exports)
*/
export type StoryObj<TArgs = Args> = StoryAnnotations<VueFramework, TArgs>;
export type StoryObj<MetaOrCmpOrArgs = Args> = MetaOrCmpOrArgs extends {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export type StoryObj<MetaOrCmpOrArgs = Args> = MetaOrCmpOrArgs extends {
export type StoryObj<TMetaOrCmpOrArgs = Args> = MetaOrCmpOrArgs extends {

Generally we've gone with a convention that a type variable is prefaced with T to distinguish from a concrete type. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, not a big fan, but also not necessarily against it. I understand that it may be something useful if you want to do:

type GenericType<TComponent extends Component> = ...

But I'm also not sure if it really becomes more readable if you use it everywhere.

I'm okay with adopting it, I would have to change some stuff then in the other renderers as well :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I adopt this convention in a separate PR?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff 🙌

code/renderers/vue3/src/__tests__/Button.vue Outdated Show resolved Hide resolved
@@ -13,8 +13,7 @@ export const render: ArgsStoryFn<VueFramework> = (props, context) => {
);
}

// TODO remove this hack
return h(Component as Parameters<typeof h>[0], props);
return h(Component, props);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 nice!!! ❤️

@kasperpeulen kasperpeulen removed the request for review from ndelangen October 25, 2022 12:12
@kasperpeulen kasperpeulen merged commit 7bcf944 into next Oct 25, 2022
@kasperpeulen kasperpeulen deleted the future/CSF3-vue3 branch October 25, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants