-
-
Notifications
You must be signed in to change notification settings - Fork 10k
Svelte: remount when resetting args in controls #21659
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
Changes from all commits
e9b548e
0c9e579
dbc1d81
42b8391
fd7fc99
440f6c0
38de735
6437e13
5188cc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| /* eslint-disable no-param-reassign */ | ||
| import type { RenderContext, ArgsStoryFn } from '@storybook/types'; | ||
| import type { SvelteComponentTyped } from 'svelte'; | ||
| import { RESET_STORY_ARGS } from '@storybook/core-events'; | ||
| // ! DO NOT change this PreviewRender import to a relative path, it will break it. | ||
| // ! A relative import will be compiled at build time, and Svelte will be unable to | ||
| // ! render the component together with the user's Svelte components | ||
| // ! importing from @storybook/svelte will make sure that it is compiled at runtime | ||
| // ! with the same bundle as the user's Svelte components | ||
| // eslint-disable-next-line import/no-extraneous-dependencies | ||
| import PreviewRender from '@storybook/svelte/templates/PreviewRender.svelte'; | ||
| import { addons } from '@storybook/preview-api'; | ||
|
|
||
| import type { SvelteRenderer } from './types'; | ||
|
|
||
|
|
@@ -24,6 +26,20 @@ function teardown(canvasElement: SvelteRenderer['canvasElement']) { | |
| componentsByDomElement.delete(canvasElement); | ||
| } | ||
|
|
||
| /** | ||
| * This is a workaround for the issue that when resetting args, | ||
| * the story needs to be remounted completely to revert to the component's default props. | ||
| * This is because Svelte does not itself revert to defaults when a prop is undefined. | ||
| * See https://github.com/storybookjs/storybook/issues/21470#issuecomment-1467056479 | ||
| * | ||
| * We listen for the RESET_STORY_ARGS event and store the storyId to be reset | ||
| * We then use this in the renderToCanvas function to force remount the story | ||
| */ | ||
| const storyIdsToRemountFromResetArgsEvent = new Set<string>(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure if this Set would ever have more than one entry at a time. It would only occur if I can't think of a scenario where that would happen - in that sense this could be simplified to be a single string instead of a Set. However this feels more bulletproof in a way.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree with all that 💯 |
||
| addons.getChannel().on(RESET_STORY_ARGS, ({ storyId }) => { | ||
| storyIdsToRemountFromResetArgsEvent.add(storyId); | ||
| }); | ||
|
|
||
| export function renderToCanvas( | ||
| { | ||
| storyFn, | ||
|
|
@@ -38,11 +54,18 @@ export function renderToCanvas( | |
| ) { | ||
| const existingComponent = componentsByDomElement.get(canvasElement); | ||
|
|
||
| if (forceRemount) { | ||
| let remount = forceRemount; | ||
|
|
||
| if (storyIdsToRemountFromResetArgsEvent.has(storyContext.id)) { | ||
| remount = true; | ||
| storyIdsToRemountFromResetArgsEvent.delete(storyContext.id); | ||
| } | ||
|
|
||
| if (remount) { | ||
| teardown(canvasElement); | ||
| } | ||
|
|
||
| if (!existingComponent || forceRemount) { | ||
| if (!existingComponent || remount) { | ||
| const createdComponent = new PreviewRender({ | ||
| target: canvasElement, | ||
| props: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import { expect } from '@storybook/jest'; | ||
| import { within, userEvent, waitFor } from '@storybook/testing-library'; | ||
| import { | ||
| UPDATE_STORY_ARGS, | ||
| STORY_ARGS_UPDATED, | ||
| RESET_STORY_ARGS, | ||
| STORY_RENDERED, | ||
| } from '@storybook/core-events'; | ||
| import { addons } from '@storybook/preview-api'; | ||
| import ButtonView from './views/ButtonView.svelte'; | ||
|
|
||
| export default { | ||
| component: ButtonView, | ||
| }; | ||
|
|
||
| export const RemountOnResetStoryArgs = { | ||
| play: async ({ canvasElement, id }) => { | ||
| const canvas = await within(canvasElement); | ||
| const channel = addons.getChannel(); | ||
|
|
||
| // Just to ensure the story is always in a clean state from the beginning, not really part of the test | ||
| await channel.emit(RESET_STORY_ARGS, { storyId: id }); | ||
| await new Promise((resolve) => { | ||
| channel.once(STORY_RENDERED, resolve); | ||
| }); | ||
| const button = await canvas.getByRole('button'); | ||
| await expect(button).toHaveTextContent('You clicked: 0'); | ||
|
|
||
| await userEvent.click(button); | ||
| await expect(button).toHaveTextContent('You clicked: 1'); | ||
|
|
||
| await channel.emit(UPDATE_STORY_ARGS, { storyId: id, updatedArgs: { text: 'Changed' } }); | ||
| await new Promise((resolve) => { | ||
| channel.once(STORY_RENDERED, resolve); | ||
| }); | ||
| await expect(button).toHaveTextContent('Changed: 1'); | ||
|
|
||
| // expect that all state and args are reset after RESET_STORY_ARGS because Svelte needs to remount the component | ||
| // most other renderers would have 'You clicked: 1' here because they don't remount the component | ||
| // if this doesn't fully remount it would be 'undefined: 1' because undefined args are used as is in Svelte, and the state is kept | ||
| await channel.emit(RESET_STORY_ARGS, { storyId: id }); | ||
| await waitFor(async () => | ||
| expect(await within(canvasElement).getByRole('button')).toHaveTextContent('You clicked: 0') | ||
| ); | ||
| }, | ||
| }; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resetting args at the end of this test is unnecessary, because we recently added logic to reset args at the beginning also - both as a way to ensure that the story is in a clean state before executing.