Vue3 ( fix ) implement reactive args + fix many vue app creation issue #20712
Conversation
There was a problem hiding this comment.
The code looks clean! I haven't had a time to try it out yet, I'll do that next week.
Could you add a story that ensures the reactivity works? We can do that by having a stateful story that tests that changing args keeps the state.
So something like a button that increments a counter, and a play function that:
- resets args (so the play function can be run again and again)
- clicks button to increment counter
- asserts counter is incremented
- changes args by emitting an event
- asserts that args have changed, eg. the label on the button has changed
- asserts that the counter is still incremented - thus the state is still there and the component didn't remount.
All the necessary pieces for the above can be seen in this play function, but I'm happy to be more specific if you need me to.
If you add the story to code/renderers/vue3/template/stories it will be picked up by CI.
|
Yeah good idea let me try that I will come back to you @JReinhold if I have some questions |
chakAs3
changed the title
|
Hi, @JReinhold Let's start the week I hope you had a great weekend. I have done what you asked I think it needs some approval on chromatic since there are some new UIs, can you review it and let me know if it is OK for you? |
| // this is the story should be remove it will always fail, because forceReRender Api is removed in V7 | ||
| export const ForceReRender = { | ||
| play: async ({ canvasElement }: PlayFunctionContext<any>) => { | ||
| const channel = globalThis.__STORYBOOK_ADDONS_CHANNEL__; | ||
| const button = await within(canvasElement).findByRole('button'); | ||
| await button.focus(); | ||
| await expect(button).toHaveFocus(); | ||
|
|
||
| // forceRender is not called in V7 |
There was a problem hiding this comment.
Actually this test was a 🤦 . It should be emitting FORCE_REMOUNT. But the bigger issue is making sure it actual fails (chromatic & the test runner) when the assertion fails like this.
There was a problem hiding this comment.
Nice, I got it. let chromatic be happy.
|
When passing some component through args, and that component is rendered using |
Oh yeah, passing a component as prop may cause some issues, as it should be serializable i hope it is a simple functional component, You are right in using shallowReactive to be safe, but we had some issues in case of altering the args with nested object that should be reactive in this case, anyway i will give it another try and see what we can do here to avoid this Thanks for the advice |
Fixes #15567 fixes #13913
issue is not noticed but there is a risk of memory leak and performance issue as the vue component are growing and reactivity is complex
What I did
use reactive args and update them inside of re-creating the whole component and story again and again when the user is interacting
How to test
yarn task --task sandbox --start-from auto --template vue3-vite/default-tsChecklist
MIGRATION.MD
Maintainers
make sure to add the
ci:mergedorci:dailyGH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]