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

Addon-docs: Dynamic source rendering for Vue #12812

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

pocka
Copy link
Contributor

@pocka pocka commented Oct 18, 2020

Issue: #11400

What I did

Added a decorator takes Vue story and emits source code for that.

NOTE: I didn't test directives (e.g. v-model, v-on, custom directives)

How to test

  • Is this testable with Jest or Chromatic screenshots? ... yes
  • Does this need a new example in the kitchen sink apps? ... no
  • Does this need an update to the documentation? ... no?

If your answer is yes to any of these, please make sure to include it in your PR. ... done (addons/docs/src/frameworks/vue/sourceDecorator.test.ts)

To see, run cd examples/vue-kitchen-sink && yarn storybook

#11400

This commit adds dynamic source code rendering feature to Docs addon for
Vue.js. The goals are 1) reflecting Controls' value to code block and
2) showing a code similar to what component consumers would write.

To archive these goals, this feature compiles a component with Vue, then
walks through vdom and stringifys the result. It could be possible to
parse components' template or render function then stringify results,
but it would be costly and hard to maintain (especially for parsing).
We can use vue-template-compiler for the purpose, but it can't handle
render functions so it's not the way to go IMO.

Speaking of the goal 2, someone wants events to be in the output code.
But it's so hard to retrieve component definitions (e.g. `methods`,
`computed`). I think it's okay to skip events until we figure there is a
high demand for that.
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.

This is looking awesome--great work @pocka ! A few minor questions and/or suggestions below.

Mouse_Highlight_Overlay

Any idea why it's :rounded and not :color?` Seems inconsistent but I don't know Vue well enough to say.. 🙈

addons/docs/src/frameworks/vue/sourceDecorator.ts Outdated Show resolved Hide resolved
</div>`,
})
)
).toMatchInlineSnapshot(`<div ><form ><button >Button</button></form></div>`);
Copy link
Member

Choose a reason for hiding this comment

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

Anyway to get rid of the extra spaces? E.g. <div > => <div>? Or is that standard in Vue?

Copy link
Contributor Author

@pocka pocka Oct 18, 2020

Choose a reason for hiding this comment

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

Or is that standard in Vue?

Nope, AFAIK.

It's ugly but will be erased by Prettier before emitting. Should we remove it before formatting?

Copy link
Member

Choose a reason for hiding this comment

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

Aha didn't realize this was the pre-prettier output. If prettier fixes, that's fine by me.

Choose a reason for hiding this comment

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

I wonder how I can write a story to achieve this effect!

@pocka
Copy link
Contributor Author

pocka commented Oct 19, 2020

@shilman

Any idea why it's :rounded and not :color?` Seems inconsistent but I don't know Vue well enough to say.. 🙈

Yes, you can use the v-bind shorthand syntax (:) for string props as well. But, for me, it feels not straight-forward and is hard to read... I guess opinions from Vue users would help since I don't write Vue app often :neckbeard:

<template>
  <my-component
    :bool-prop="true"
    :number-prop="1"
    :string-prop="'extra quotes needed since this is a JS expression'"
  />
</template>

@shilman shilman added this to the 6.1 docs milestone Oct 19, 2020
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.

LGTM! ❤️

@shilman shilman merged commit dc17a9d into next Oct 19, 2020
@shilman shilman deleted the pocka/feature/vue-dynamic-source branch October 19, 2020 09:57
@dallinja
Copy link

@pocka

I guess opinions from Vue users would help since I don't write Vue app often

I would prefer to see it like this...

<MyComponent
  :number-prop="1"
  string-prop="no need to bind strings"
  bool-prop
/>

My goal would be to just click copy and then paste it into my code, without any editing. This means:

  1. Remove <template> tag. I don't want to have to delete the <template> tag every time. It should be extremely obvious to any Vue developer that the code goes inside a <template> tag. And never would the component be the only thing in a <template> tag, so leaving them in the source here would required editing 100% of the time. It would be the same as doing return (<MyComponent />) in React. No need to show the return.
  2. Remove v-bind for strings. You're right, binding them is weird.
  3. No need to v-bind booleans. Just extra code that I'd want to delete. If it's false, don't include it.

Lastly, the Vue style guide recommends using PascalCase for Components.

I was gonna jump on this request, but really excited someone already did! Awesome work @pocka!

@pocka
Copy link
Contributor Author

pocka commented Oct 27, 2020

The <template> tag is just for Prettier to work, I think we can add a strip-templates feature (an option like noTemplateTag or do it by default idk).
That v-bind shortcut for boolean is for demonstrating purpose, not show up in an actual result 👍

Lastly, the Vue style guide recommends using PascalCase for Components.

Probably the casing transform is easy to implement (https://github.com/storybookjs/storybook/pull/12812/files#diff-83d08076a6b73dab376e42608b87b8720952063d14015e3fbf5d38ac3a23fa94R76) and a good-to-have-feature. Anyone interests?

@shilman
Copy link
Member

shilman commented Oct 28, 2020

Thanks @dallinja and @pocka for pushing this forward 🙏

@zhangjiangqiu
Copy link

This is looking awesome--great work @pocka ! A few minor questions and/or suggestions below.

Mouse_Highlight_Overlay

Any idea why it's :rounded and not :color?` Seems inconsistent but I don't know Vue well enough to say..

I want to know how can I write a story to make this happen!

@MariaNicolosi
Copy link

This looks so much better!
However, the source code isn't displaying named slots.

Screen Shot 2020-12-16 at 9 47 00 AM

Screen Shot 2020-12-16 at 9 44 42 AM

@litan1106
Copy link

I am using vue 3 with storybook 6.2.0-alpha.23 but it didnt render the dynamic code.

image

pocka added a commit that referenced this pull request Feb 25, 2021
#13562

In a Pull Request that adds dynamic source rendering feature for Vue.js
[0], prettier was added as regular dependency. It broke IE11 (ES5)
compatibility because prettier is published in ES2015 or later.

This commit make Storybook's webpack config to transpile prettier down
to ES5.

[0] ... #12812
pocka added a commit that referenced this pull request Feb 25, 2021
#13562

In a Pull Request that adds dynamic source rendering feature for Vue.js
[0], prettier was added as regular dependency. It broke IE11 (ES5)
compatibility because prettier is published in ES2015 or later.

This commit make Storybook's webpack config to transpile prettier down
to ES5.

[0] ... #12812
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.

6 participants