storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
83.94k stars 9.21k forks source link

Props not showing in the DocsPage when they're imported from another file into the component (Vue) #11774

Open puyt opened 4 years ago

puyt commented 4 years ago

Describe the bug I have a component where the props are in a different JavaScript file because it's used for multiple button components but the props aren't rendered in the docs, am i doing something wrong?

To Reproduce Steps to reproduce the behavior:

  1. Run storybook with the 3 example files below

Expected behavior Props being rendered in the DocsPage

Screenshots image

image

Code snippets

// ButtonBrops.js
export default {
    label: {
        type: String,
        default: 'Button',
    },
};
// Button.vue
<template>
    <button @click="clickButton">{{ label }}</button>
</template>

<script>
    import ButtonProps from './ButtonProps';

    export default {
        name: 'button',
        props: ButtonProps,
        methods: {
            clickButton() {
                this.$emit('click-button');
            },
        },
    };
</script>
// Button.stories.mdx
import { Meta, Props } from '@storybook/addon-docs/blocks';
import Button from './Button.vue';

<Meta title='Components/Button' component={Button} />

## Props, Events & Slots
<Props of={ Button } />

System:

Environment Info:

  System:
    OS: Linux 5.4 Ubuntu 19.10 (Eoan Ermine)
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
  Binaries:
    Node: 12.13.0 - ~/.nvm/versions/node/v12.13.0/bin/node
    npm: 6.12.0 - ~/.nvm/versions/node/v12.13.0/bin/npm
  Browsers:
    Chrome: 84.0.4147.125
    Firefox: 78.0.2
  npmPackages:
    @storybook/addon-actions: ^6.0.16 => 6.0.16
    @storybook/addon-docs:  ^6.0.16 => 6.0.16
    @storybook/addon-essentials:  ^6.0.16 => 6.0.16
    @storybook/addon-links:  ^6.0.16 => 6.0.16
    @storybook/vue:  ^6.0.16 => 6.0.16 
shilman commented 4 years ago

@elevatebart is this a known limitation with vue-docgen-api?

elevatebart commented 4 years ago

Hello @puyt, Thank you @shilman,

It is indeed a known limitation of vue-docgen-api. The library does not resolve variables in props. Since it is a static analyzer of the code, not its runtime, it often can't guess what variables will contain.

Though, @puyt you make me think of a scenario where it would make sense to have the props as a separate object. Tell me if you have another scenario where it would be necessary.

Here it is:

Button.vue has 3 props color, size and variant. We then make an ActionButton.vue, who encapsulates the button and keeps all it's props using v-bind. The props validations now have to be defined at 2 levels, and copied and pasted.

It would be better to have them defined once and imported.

This sounds feasible but not easy.

For reference here are other issues related to resolving non-explicit prop types: https://github.com/vue-styleguidist/vue-styleguidist/issues/923 https://github.com/vue-styleguidist/vue-styleguidist/issues/881

shilman commented 4 years ago

Thanks for clarifying @elevatebart! Static analysis is a beast--you're really doing the Lord's work. I'm fine to say it's a limitation of Storybook right now.

@puyt in 6.0 it's possible to specify types by hand if needed as a workaround: https://github.com/storybookjs/storybook/blob/next/addons/controls/README.md#my-controls-arent-being-auto-generated-what-should-i-do

puyt commented 4 years ago

Button.vue has 3 props color, size and variant. We then make an ActionButton.vue, who encapsulates the button and keeps all it's props using v-bind. The props validations now have to be defined at 2 levels, and copied and pasted.

@elevatebart This is actually the way we implemented Button.vue but with a few more props and we're creating multiple variants which are frequently used to prevent other developers from using a different color, icon, label, rounded,... for those cases.

So we have variants such as AddButton, DeleteButton, EditButton,... We could have used something to variant but there would be too many if else code or computed values inside the button to handle the styling that's why we moved the props to different object and merge that object with the props with the settings for a specific button. This way the props and default values are always in that other object instead of v-binding it every new variant.

I can't really think of another use-case except for the one you mentioned.

I'm oke with the suggested workaround that @shilman mentioned, it's a edge case anyway and we're currently only having this issue with the button component.

Thanks for the fast reply and the great work on the libraries both!

elevatebart commented 4 years ago

If you were to use a mixin for these props instead of an object, docgen would be able to pick it up out of the box. I hope it helps you at all ;)

visualjerk commented 4 years ago

Just wanted to add a similar usecase:

When sharing component functionality with the composition api, quite often prop definitions also need to be shared.

Example:

import { boxProps, useBox } from '@components/box'

export const tabbableProps = {
  ...boxProps,
  disabled: {
    type: Boolean,
    default: false,
  },
  focusable: {
    type: Boolean,
    default: false,
  },
}

export function useTabbable(props, context) {
  const box = useBox(props, context)
  ...
}

export const Tabbable = {
  props: tabbableProps,
  setup: useTabbable,
}
elevatebart commented 4 years ago

@visualjerk good added feedback!

Spreads and multi-level props resolving makes static parsing all the more interesting... and complicated.

I will try and integrate these use cases in version 5 of docgen. Hopefully they can be sorted out simply.

Would you be interested in learning how I do that?

visualjerk commented 4 years ago

For sure šŸ˜„ Is there a related issue, that I can watch?

elevatebart commented 4 years ago

Not yet. will probably work on it this Week end

stale[bot] commented 3 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

gburning commented 3 years ago

Sorry to revive an old thread but was there ever any progress on this @elevatebart?

I appreciate that parsing extracted props is not a core feature for vue-docgen-api but if there was a gist or perhaps some sort of plugin to do this I think that might be useful. It does seem like a fairly commonly requested feature (https://github.com/storybookjs/storybook/issues?q=vue+mixin+props).

I have a bunch of component types (form controls, button components) that have common props that I'd like to selectively share between them (all control components have to support a "label" prop but only some might support a "icon" or "placeholder" prop). Mixins don't work for this as it's all or nothing. They also obscure the source of the props and make code readability a lot worse in my opinion.

So, if anyone has made any progress on this I'd greatly appreciate it.

gburning commented 3 years ago

For the time being I hacked together some code to do the prop to argType mapping manually: https://gist.github.com/gburning/7727cb834760460a796fbf103b89986e

Not great but will work fine until a better solution exists.

elevatebart commented 3 years ago

Hello @gburning,

The problem of dealing with runtime generated is a little deeper than just extracting those props. It also asks the question "Where do I add the comments to describe each prop?" or "What if I have to redocument a specific case in a component?". If you prefer mixins to generated props, you will get all those questions answered and will still keep the DRY code.

So my question is what would be wrong with using a set of mixins in this use case? How can I help you adopt them?

I hope you understand what I mean, if not join me on discord. I know I can be unclear and take shortcuts.

Cheers

gburning commented 3 years ago

Thanks for the answer!

In my particular case those props are written in the same way as props usually are. Just not in the same file.

I understand that there's an issue of parsing runtime code and potential feature/scope creep. If the answer is that this is not something vue-docgen-api will do, then I can live with that. Just figured I'd add my vote and explain my use case. :)

Mixins are unfortunately a no-go in my case. Mainly for the reasons I mentioned in my previous post.

If there was a way to import props from another component and have vue-docgen-api work with that I could go in that direction. Basically, as long as the imported props come from a SFC file. Not the prettiest, but it would work.

On a side note, I'm curious if there's a plan to support the composition API in the future? In my mind that seems to be the main way of reusing Vue code going forward. Just an assumption of course, but wouldn't there be similar problems there?

Lastly, if this can't be solved with vue-docgen-api then it might be good if storybook-addon-docs exported some utilities to map an object of props to argTypes during runtime. Similar to my clumsy approach in the gist I posted earlier.

(The last part is obviously not directed at you @elevatebart)

lijing666 commented 3 years ago

Hello @gburning,

The problem of dealing with runtime generated is a little deeper than just extracting those props. It also asks the question "Where do I add the comments to describe each prop?" or "What if I have to redocument a specific case in a component?". If you prefer mixins to generated props, you will get all those questions answered and will still keep the DRY code.

So my question is what would be wrong with using a set of mixins in this use case? How can I help you adopt them?

I hope you understand what I mean, if not join me on discord. I know I can be unclear and take shortcuts.

Cheers

Vue mixins is an old fashion mode to compose complex vue component, now Vue recommends composition-api to write component, Vue 2 with @vue/composition-api or Vue 2.7 later and Vue 3 native support, so props mixins will be deprecated sometimes later, instead import props from another file will be the primary mode. So new vue developers just maybe don't use mixins anymore.

craigrileyuk commented 2 years ago

Having the same problem here. I'm coding a library of components and many of the components share the same sets of props.

// src/composables/props/dimensions.js
export default {
   /**
   * Sets the default width for the component
   */
    width: {
        type: [Number,String],
        default: null
    },
    /**
   * Sets the default height for the component
   */
    height: {
        type: [Number,String],
        default: null
    },
    /**
   * Sets the maximum width for the component
   */
    maxWidth: {
        type: [Number,String],
        default: null
    },
    /**
   * Sets the maximum height for the component
   */
    maxHeight: {
        type: [Number,String],
        default: null
    }
}
<script setup>
import dimensionProps from "@/src/composables/props/dimensions.js";

defineProps({
    ...dimensionProps
});
</script>

Obviously it saves a hell of a lot of code if even 2 or 3 components share those same props to import them rather than code in every component that uses them.

I have no idea if covering this (fairly common) use case would be possible with static analysis, but it would be cool if possible.

elevatebart commented 2 years ago

It is possible with a little elbow grease. I am planning support for this and advanced typescript types for 5.0.

I know how to do it, I have no idea when it is going to be done.

It is progressing though. I am about to share the first RFC for a breaking change on advanced types (union, intersection, interfaces,... )

treardon17 commented 1 year ago

Curious if there has been any progress or plans for this? My use case is exactly as described above -- reusing props by spreading them in from another file. Mixins aren't an option because we're using script setup and the composition API. The docs are fantastic, and this would be a pretty awesome addition to make our docs more complete!

scramlo commented 1 year ago

I am here as well sharing props between components in Vue 3 on an enterprise app. We are investigating using Storybook or https://histoire.dev/ for component library. I used Storybook at my last company with Vue 2 and loved it. But, this might be a deal breaker šŸ˜¢ As stated earlier, it sounds like there is a lot of forks in the road when trying to execute and scan code and put it where it is meant to go in the storybook UI. Blessings on the journey!