Storybook Docs cannot handle union types in Vue/TS setup

[HerrBertling]

Describe the bug

Hey :wave:

I tried to update SB to version 6. Our project setup is Vue/NuxtJS/Typescript + vue-property-decorator for writing class components.

Initially, no components would render and I'd get this error:

Cannot read property 'map' of undefined
TypeError: Cannot read property 'map' of undefined
    at convert (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:5805:22)
    at convert (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:5688:51)
    at javaScriptFactory (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:6183:41)
    at extractProp (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:6328:19)
    at http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:6292:12
    at Array.map (<anonymous>)
    at extractComponentSectionArray (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:6291:24)
    at extractComponentProps (http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:6318:41)
    at http://localhost:1337/vendors~main.dcec9cf333cc0105409b.bundle.js:5554:51
    at Array.forEach (<anonymous>)

Going through single stories, I found out that apparently a union type declared within the props produces this error.

To Reproduce Steps to reproduce the behavior:

1. Clone https://github.com/HerrBertling/nuxt-ts-storybook-docs-issue & run yarn
2. Start Storybook via yarn storybook
3. See error

If you switch the brokenMsg type to one of the working ones explained below, Storybook works as expected.

Expected behavior

This should™ just™ work™ 😄

Code snippets

Working prop:

<script lang="ts">
import Vue from 'vue'
import { Prop, Component } from 'vue-property-decorator'

@Component({})
export default class BaseCheckbox extends Vue {
  @Prop({ default: '' }) id!: string
  [… more props here]
}
</script>

Also working, but feels rather stupid

<script lang="ts">
import Vue from 'vue'
import { Prop, Component } from 'vue-property-decorator'

@Component({})
export default class BaseCheckbox extends Vue {
  @Prop({ default: '' }) id!: StringOrNumber
  [… more props here]
}
</script>

with a global type declared somewhere:

declare type StringOrNumber = string | number

Passing along different strings works also without issues, like for a button:

<script lang="ts">
import Vue from 'vue'
import { Prop, Component } from 'vue-property-decorator'

@Component({})
export default class BaseButton extends Vue {
  @Prop({ default: 'primary' }) variant!: 'primary' | 'secondary' | 'text'
  [… more props here]
}
</script>

System:

I guess this is not needed, but just in case:

System info

``` System: OS: macOS 10.15.6 CPU: (4) x64 Intel(R) Core(TM) i7-7660U CPU @ 2.50GHz Binaries: Node: 12.16.3 - ~/.nvm/versions/node/v12.16.3/bin/node Yarn: 1.22.4 - ~/Sites/my/fancy/project/node_modules/.bin/yarn npm: 6.14.4 - ~/.nvm/versions/node/v12.16.3/bin/npm Browsers: Chrome: 84.0.4147.125 Firefox: 77.0.1 Safari: 13.1.2 npmPackages: @storybook/addon-a11y: ^6.0.4 => 6.0.4 @storybook/addon-essentials: ^6.0.4 => 6.0.4 @storybook/addon-knobs: ^6.0.4 => 6.0.4 @storybook/theming: ^6.0.4 => 6.0.4 @storybook/vue: ^6.0.4 => 6.0.4 ```

[shilman]

@elevatebart Is this supported by vue-docgen-api?

[elevatebart]

@shilman @HerrBertling It should definitely not throw such a blocking error. I do not know how to transfer this issue to the styleguidist repo. We could create another one there.

...Sorry for being late to the party here, Chicago has been without power for a few days because of high winds. I'm back though.

[HerrBertling]

I can transfer this if you want to?

[elevatebart]

Thank you @HerrBertling that would be awesome.

[HerrBertling]

Done :)

[elevatebart]

@shilman Can you look at the format exported by vue-docgen-api for union types. It might not correspond to what you expect for prop definitions.

If you find where this is parsed and could be using .map to create the knobs I could create an adapter or a fix.

[shilman]

@elevatebart thanks for the heads up! 🙏 we have framework-specific extractors so can probably handle it on the storybook side if needed as well. i'll take a look!

[HerrBertling]

Okay, what's more fun than digging deep into node_modules and console.logging the sh*t out of everything to track down the issue – right? :smile:

I found the problem, I guess? This bit in the convert.ts destructures entries that should exist within the type argument:

https://github.com/storybookjs/storybook/blob/f3a9117937205f88d8da0264b59942edcfe29cb6/addons/docs/src/lib/convert/proptypes/convert.ts#L10

But for a union type in a Vue component, value does not exist and is undefined. That's why the map cannot work here:

https://github.com/storybookjs/storybook/blob/f3a9117937205f88d8da0264b59942edcfe29cb6/addons/docs/src/lib/convert/proptypes/convert.ts#L40

But whatever should exist in type.value is present as type.elements. My nasty fix to get this working was falling back to type.elements. Yes, of course within the compiled source 😄

var convert = function convert(type) {
  var name = type.name,
      raw = type.raw,
      computed = type.computed,
      value = type.value ? type.value : type.elements;
     // … (other stuff)

The convert method is called here:

https://github.com/storybookjs/storybook/blob/f3a9117937205f88d8da0264b59942edcfe29cb6/addons/docs/src/lib/docgen/createPropDef.ts#L74-L79

So either the elements should be used or they should be converted into a value entry before passing it on? I guess converting beforehand is the sensible thing to to. Probably here:

https://github.com/storybookjs/storybook/blob/f3a9117937205f88d8da0264b59942edcfe29cb6/addons/docs/src/lib/docgen/extractDocgenProps.ts#L32-L39

Because that's the part that explicitly targets vue-docgen-api, so it should be the best place to handle this edge case?

https://github.com/storybookjs/storybook/blob/f3a9117937205f88d8da0264b59942edcfe29cb6/addons/docs/src/lib/docgen/extractDocgenProps.ts#L64-L67

I guess @elevatebart knows how to handle this best?

EDIT: I tried this and it works™

export const extractComponentSectionArray = (docgenSection: any) => {
  const typeSystem = getTypeSystem(docgenSection[0]);
  const createPropDef = getPropDefFactory(typeSystem);

  return docgenSection
    .map((item: any) => {
      const sanitizedItem = {...item, value: item.elements }
      return extractProp(sanitizedItem.name, sanitizedItem, typeSystem, createPropDef)
    })
    .filter(Boolean);
};

[HerrBertling]

Sorry for the branch confusion – I shouldn't have put documentation additions into the forked next branch 😅

[shilman]

Jiminy cricket!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.15 containing PR #12158 that references this issue. Upgrade today to try it out!

[shilman]

Good golly!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.16 containing PR #12165 that references this issue. Upgrade today to try it out!

[MineDrum]

I expected storybook to generate an "options" control when given a union prop type, but I haven't been able to get that to work. Am I doing something wrong or is that not actually how it works?