storybook icon indicating copy to clipboard operation
storybook copied to clipboard

Vue3: fix source decorator + rendering with decorators

Open chakAs3 opened this issue 1 year ago • 15 comments

Closes #20670 Closes #21235 may even Closes #21221 stil have to go through some testing

Improving Reactivity and Rendering in Storybook

In this PR i took a bit time to get things right to minimise issue in future, I'm excited to share some of the enhancements I've made to improve the reactivity and rendering of stories.

Fully Reactive Stories

One of the most significant improvements I've made is to ensure that all stories are fully reactive to related changes, without requiring a remount or re-rendering of the DOM tree. This is achieved by making the top-level components reactive via the props and even the slots, which fixes a lot of issues related to remounting components or the entire Vue app and causing the loss of state. Furthermore, I've added support for reactive stories using multiple decorators. This is particularly helpful for developers who use UI libraries like Chakra or Vuetify to build fast apps or prototypes. These libraries need to wrap their stories to inject their style or configuration. Now, with CSF 2 and 3, we can work in a reactive way with decorators, which greatly simplifies the process. To demonstrate these improvements, I've created a repository that showcases these points. You can find the link check out this video demo

Powerful and Reactive Slots

Another area where I've made significant improvements is in the handling of slots. Slots are now very powerful, as they are reactive and automatically generated if your component has defined them. We support named and scoped slots, and using the args, you can pass slot types such as function, string, VNode (using Vue render function h), or Functional Component. This is incredibly powerful, as it opens up a wide range of possibilities for building custom components and integrating them seamlessly into your storybook. One area where we still have work to do is on the controls side, as we link between components using the controls and inject them as slots to any component. However, with the improvements we've made, we're confident that we can continue to push the boundaries of what's possible with slots in Storybook. To see these improvements in action, check out this video demo.

Source Decorator

Last but not least, I've rewrote my old Source Decorator . This is a fun and important feature that most developers will appreciate, as it makes it easy to pick working code and use it quickly. With the Vue compiler, we can get proper DOM elements to parse and generate the right and proper code. Overall, I'm thrilled to have contributed to the Storybook code base and made these improvements. I believe that they will significantly improve the developer experience and enable developers to build more powerful and flexible components. check out this video demo.

@shilman @jonniebigodes let me know if we can work on documentation, i will be working on short video for vue 3 developers showing new features in V7

Checklist

  • [ ] Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • [ ] Make sure to add/update documentation regarding your changes
  • [ ] If you are deprecating/removing a feature, make sure to update MIGRATION.MD

Maintainers

  • [ ] If this PR should be tested against many or all sandboxes, make sure to add the ci:merged or ci:daily GH label to it.
  • [ ] Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

chakAs3 avatar Feb 27 '23 14:02 chakAs3

@chakAs3 Thanks 🙌 Will look at this monday!

kasperpeulen avatar Mar 10 '23 16:03 kasperpeulen

@chakAs3 Thanks 🙌 Will look at this monday!

sure man thx i know it is quite large PR to review .

chakAs3 avatar Mar 11 '23 12:03 chakAs3

@chakAs3 Thank you very much, these are really good improvements. I'm not experienced with Vue, nor the core rendering logic of storybook, so I'm trying to wrap my head around it now. I hope we can get this in before storybook day.

Do you know what is going on with the tests and build?

@ndelangen @tmeasday If any of you have time, it would be good if you can look at this as well.

kasperpeulen avatar Mar 13 '23 10:03 kasperpeulen

@chakAs3 Thank you very much, these are really good improvements. I'm not experienced with Vue, nor the core rendering logic of storybook, so I'm trying to wrap my head around it now. I hope we can get this in before storybook day.

Do you know what is going on with the tests and build?

@ndelangen @tmeasday If any of you have time, it would be good if you can look at this as well.

yes @kasperpeulen thank you for taking time and review, i will fix the tests i don't know why it is failing

chakAs3 avatar Mar 13 '23 17:03 chakAs3

@chakAs3 can you provide a demo or an explanation of how this works with slots, and what the API looks like from a user perspective? I want to understand if this is useful and possible for other slot-based languages as well, like Svelte.

JReinhold avatar Mar 13 '23 21:03 JReinhold

Maybe we could add some template stories to the renderer demonstrating the new apis/behaviours also

tmeasday avatar Mar 14 '23 08:03 tmeasday

Maybe we could add some template stories to the renderer demonstrating the new apis/behaviours also

i have already 1 repo planing outside of the monorepo, planning run some tests there so i don't impact the monorepo in case other framework, otherwise yes i will add some stories there

chakAs3 avatar Mar 14 '23 10:03 chakAs3

@chakAs3 can you provide a demo or an explanation of how this works with slots, and what the API looks like from a user perspective? I want to understand if this is useful and possible for other slot-based languages as well, like Svelte.

yes sure i already prepare videos and 1 repo

chakAs3 avatar Mar 14 '23 10:03 chakAs3

@chakAs3 if you add story files in the renderer (like these React ones: https://github.com/storybookjs/storybook/blob/01e21c9470f7eeb395405e30252dc0c40321100c/code/renderers/react/template/stories/hooks.stories.tsx) they'll get tested by the test runner + chromatic which gives us much better test coverage of this new behaviour!

tmeasday avatar Mar 14 '23 11:03 tmeasday

@chakAs3 if you add story files in the renderer (like these React ones: https://github.com/storybookjs/storybook/blob/01e21c9470f7eeb395405e30252dc0c40321100c/code/renderers/react/template/stories/hooks.stories.tsx) they'll get tested by the test runner + chromatic which gives us much better test coverage of this new behaviour!

yes tom i have already added the reactiveArgs stories 2 months ago , i will add these some others

chakAs3 avatar Mar 14 '23 11:03 chakAs3

Hi @chakAs3. There is a lot going on in this PR.

Could we split this in more focused PRs. We are currently in a feature freeze for Storybook 7. So we only want to merge bugs now, and release new features shortly after the Storybook 7 release in a SB 7.1 alpha release.

Could we split this PR maybe in 3 different PRs:

  • Reactive stories and decorators
  • Source decorators
  • Reactive slots

I'm also not 100% sure, which parts are fixing bugs, and which are introducing new features.

Thanks for all your work!

kasperpeulen avatar Mar 14 '23 15:03 kasperpeulen

Hi @chakAs3. There is a lot going on in this PR.

Could we split this in more focused PRs. We are currently in a feature freeze for Storybook 7. So we only want to merge bugs now, and release new features shortly after the Storybook 7 release in a SB 7.1 alpha release.

Could we split this PR maybe in 3 different PRs:

  • Reactive stories and decorators
  • Source decorators
  • Reactive slots

I'm also not 100% sure, which parts are fixing bugs, and which are introducing new features.

Thanks for all your work!

hi @kasperpeulen thanks for the quality of your review. you really took time to do that in good manner and helped me changing some code.

However i think spliting the PR in this stage is difficult i can't ship code that i'm sure 100% will have a bug,i know exactly what will happen.

i have added tests story for slots like @tmeasday asked and answered his questions and i provided 3 videos that demo everything that should be enough unless someone else wants to review it

chakAs3 avatar Mar 14 '23 20:03 chakAs3

@chakAs3 I'm trying wrap my head more around the reactivity problems.

Could you maybe add a play function test, that breaks in the next branch and passes in this branch, to make sure we are never regressing back? Maybe also one for decorators?

kasperpeulen avatar Mar 16 '23 12:03 kasperpeulen

@chakAs3 I'm trying wrap my head more around the reactivity problems.

Could you maybe add a play function test, that breaks in the next branch and passes in this branch, to make sure we are never regressing back? Maybe also one for decorators?

i will do it once i get time this evening, but to get an idea it is about how to do things right, in reactive way . otherwise we could always workaround to pass tests, react way of doing things it is working seems reactive but it not the case because react render all component stack again and again.

chakAs3 avatar Mar 16 '23 14:03 chakAs3

@chakAs3 Right I thought maybe about a test like:

  1. Filling in an input element
  2. Changing args
  3. Input state should be preserved

kasperpeulen avatar Mar 16 '23 15:03 kasperpeulen

Right, I noticed that as well. My feeling that the correct way forward would be to rewrite them all without hooks, or at least something that is compatible with signal based architectures. (Initial setup only running once).

@JReinhold

yes I though of having something like preact + signals will realy do great in performance, when i was the hooks i thought these are React's but Tom explained to me these are Storybook hooks, but i guess they work pretty much like React.

in the source decorator i replace them with Vue watch

chakAs3 avatar Mar 21 '23 10:03 chakAs3

Have you seen the Svelte implementation? @chakAs3 We were thinking that the proper way for Vue maybe similar. cc @JReinhold

Well Svelte uses reactivity differently no tracking in runtime but in build which is still fine, cause we don't have dynamic dependencies. Svelte smarter than React but less than Vue

chakAs3 avatar Mar 21 '23 10:03 chakAs3

Fwiw, @chakAs3 has mentioned in discord that this should fix an issue with the templates,

Simply having multiple <div></div> blocks in the template : `` would cause this. (this happens if you have the same html elements within one another).

image

Reproduction here https://discord.com/channels/486522875931656193/1090204816300593162/1091732602802679848

blowsie avatar Apr 04 '23 09:04 blowsie

Hi @blowsie the bug is related to source decorator, check the repo i made for you forked from yours https://github.com/chakAs3/unexpected-closing-tag-issue let me know so i can work on merging this PR

chakAs3 avatar Apr 04 '23 14:04 chakAs3

Hi @blowsie the bug is related to source decorator, check the repo i made for you forked from yours https://github.com/chakAs3/unexpected-closing-tag-issue let me know so i can work on merging this PR

Thanks, this is working in StackBlitz at least, but im unsure how I can test this with my repo?

blowsie avatar Apr 06 '23 16:04 blowsie