Skip to content

Commit

Permalink
Merge pull request #19207 from storybookjs/shilman/vue2-rendering-imp…
Browse files Browse the repository at this point in the history
…rovements

Vue2: Fix play function `within` & args updating in decorators
  • Loading branch information
shilman authored Oct 2, 2022
2 parents 28b2604 + db85462 commit e562342
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 21 deletions.
12 changes: 6 additions & 6 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ jobs:
executor:
class: medium+
name: sb_node_14_browsers
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand All @@ -497,7 +497,7 @@ jobs:
executor:
class: medium+
name: sb_node_14_browsers
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand All @@ -513,7 +513,7 @@ jobs:
executor:
class: medium+
name: sb_node_14_browsers
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand All @@ -533,7 +533,7 @@ jobs:
executor:
class: medium+
name: sb_node_14_browsers
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand All @@ -549,7 +549,7 @@ jobs:
executor:
class: medium+
name: sb_node_14_browsers
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand All @@ -565,7 +565,7 @@ jobs:
executor:
class: medium+
name: sb_playwright
parallelism: 8
parallelism: 9
steps:
- git-shallow-clone/checkout_advanced:
clone_options: '--depth 1 --verbose'
Expand Down
9 changes: 8 additions & 1 deletion MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
- [Vite builder uses vite config automatically](#vite-builder-uses-vite-config-automatically)
- [Removed docs.getContainer and getPage parameters](#removed-docsgetcontainer-and-getpage-parameters)
- [Icons API changed](#icons-api-changed)
- ['config' preset entry replaced with 'previewAnnotations'](#config-preset-entry-replaced-with-preview-annotations)
- ['config' preset entry replaced with 'previewAnnotations'](#config-preset-entry-replaced-with-previewannotations)
- [Vue2 DOM structure changed](#vue2-dom-structure-changed)
- [Docs Changes](#docs-changes)
- [Standalone docs files](#standalone-docs-files)
- [Referencing stories in docs files](#referencing-stories-in-docs-files)
Expand Down Expand Up @@ -550,6 +551,12 @@ The preset field `'config'` has been replaced with `'previewAnnotations'`. `'con

Additionally, the internal field `'previewEntries'` has been removed. If you need a preview entry, just use a `'previewAnnotations'` file and don't export anything.

#### Vue2 DOM structure changed

In 6.x, `@storybook/vue` would replace the "root" element (formerly `#root`, now `#storybook-root`) with a new node that contains the rendered children. This was problematic because it broke the `play` function, which often starts with `within(canvasElement)` and the old `canvasElement` would get replaced out from under the play function.

In 7.0, `@storybook/vue` now leaves `#storybook-root` alone, and creates a new "dummy node" called `#storybook-vue-root` beneath it. This will break DOM snapshots moving from 6.5 to 7.0, but shouldn't have any other negative effects.

### Docs Changes

The information hierarchy of docs in Storybook has changed in 7.0. The main difference is that each docs is listed in the sidebar as a separate entry, rather than attached to individual stories.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { within, userEvent } from '@storybook/testing-library';
import MyButton from '../Button.vue';

export default {
Expand All @@ -11,7 +12,8 @@ export default {
const Template = (args, { argTypes }) => ({
props: Object.keys(argTypes),
components: { MyButton },
template: '<my-button :color="color" :rounded="rounded">{{label}}</my-button>',
template: `
<my-button :color="color" :rounded="rounded">{{label}}</my-button>`,
});

export const Rounded = Template.bind({});
Expand All @@ -20,6 +22,18 @@ Rounded.args = {
color: '#f00',
label: 'A Button with rounded edges',
};
Rounded.decorators = [
(storyFn, context) => {
return storyFn({ ...context, args: { ...context.args, label: 'Overridden args' } });
},
() => ({
template: '<div style="background: #eee;"><story/></div>',
}),
];
Rounded.play = async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole('button'));
};

export const Square = Template.bind({});
Square.args = {
Expand Down
3 changes: 1 addition & 2 deletions code/lib/cli/src/repro-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ const vueCliTemplates = {
name: 'Vue-CLI (Vue2 JS)',
script:
'npx -p @vue/cli vue create . --default --packageManager=yarn --force --merge --preset=Default\\ (Vue\\ 2)',
// FIXME: https://github.com/storybookjs/storybook/issues/19204
cadence: [] as any,
cadence: ['ci', 'daily', 'weekly'],
expected: {
framework: '@storybook/vue-webpack5',
renderer: '@storybook/vue',
Expand Down
3 changes: 3 additions & 0 deletions code/lib/store/template/stories/hooks.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export const UseState = {
],
play: async ({ canvasElement }: PlayFunctionContext) => {
const button = await within(canvasElement).findByText('Clicked 0 times');
// FIXME: onClick does not properly register in vue2
// https://github.com/storybookjs/storybook/issues/19318
if (globalThis.storybookRenderer === 'vue') return;

await userEvent.click(button);
await within(canvasElement).findByText('Clicked 1 times');
Expand Down
16 changes: 12 additions & 4 deletions code/renderers/vue/src/decorateStory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ export const WRAPS = 'STORYBOOK_WRAPS';

function prepare(
rawStory: StoryFnVueReturnType,
innerStory?: VueConstructor
innerStory?: VueConstructor,
context?: StoryContext<VueFramework>
): VueConstructor | null {
let story: ComponentOptions<Vue> | VueConstructor;

Expand All @@ -37,8 +38,13 @@ function prepare(
return Vue.extend({
// @ts-expect-error // https://github.com/storybookjs/storybook/pull/7578#discussion_r307985279
[WRAPS]: story,
// @ts-expect-error // https://github.com/storybookjs/storybook/pull/7578#discussion_r307984824
[VALUES]: { ...(innerStory ? innerStory.options[VALUES] : {}), ...extractProps(story) },
[VALUES]: {
// @ts-expect-error // https://github.com/storybookjs/storybook/pull/7578#discussion_r307984824
...(innerStory ? innerStory.options[VALUES] : {}),
// @ts-expect-error // https://github.com/storybookjs/storybook/pull/7578#discussion_r307984824
...extractProps(story),
...(context?.args || {}),
},
functional: true,
render(h, { data, parent, children }) {
return h(
Expand Down Expand Up @@ -77,6 +83,8 @@ export function decorateStory(

return prepare(decoratedStory, story as any);
},
(context) => prepare(storyFn(context))
(context) => {
return prepare(storyFn(context), null, context);
}
);
}
32 changes: 25 additions & 7 deletions code/renderers/vue/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ const getRoot = (domElement: Element): Instance => {
return map.get(domElement);
}

// Create a dummy "target" underneath #storybook-root
// that Vue2 will replace on first render with #storybook-vue-root
const target = document.createElement('div');
domElement.appendChild(target);

const instance = new Vue({
beforeDestroy() {
map.delete(domElement);
Expand All @@ -36,17 +41,17 @@ const getRoot = (domElement: Element): Instance => {
[VALUES]: {},
};
},
// @ts-expect-error What's going on here?
render(h) {
map.set(domElement, instance);
const children = this[COMPONENT] ? [h(this[COMPONENT])] : undefined;
return h('div', { attrs: { id: 'storybook-root' } }, children);
return this[COMPONENT] ? [h(this[COMPONENT])] : undefined;
},
});
}) as Instance;

return instance;
};

export const render: ArgsStoryFn<VueFramework> = (props, context) => {
export const render: ArgsStoryFn<VueFramework> = (args, context) => {
const { id, component: Component, argTypes } = context;
const component = Component as VueFramework['component'] & {
__docgenInfo?: { displayName: string };
Expand Down Expand Up @@ -84,7 +89,6 @@ export function renderToDOM(
title,
name,
storyFn,
storyContext: { args },
showMain,
showError,
showException,
Expand All @@ -96,6 +100,20 @@ export function renderToDOM(
Vue.config.errorHandler = showException;
const element = storyFn();

let mountTarget: Element;

// Vue2 mount always replaces the mount target with Vue-generated DOM.
// https://v2.vuejs.org/v2/api/#el:~:text=replaced%20with%20Vue%2Dgenerated%20DOM
// We cannot mount to the domElement directly, because it would be replaced. That would
// break the references to the domElement like canvasElement used in the play function.
// Instead, we mount to a child element of the domElement, creating one if necessary.
if (domElement.hasChildNodes()) {
mountTarget = domElement.firstElementChild;
} else {
mountTarget = document.createElement('div');
domElement.appendChild(mountTarget);
}

if (!element) {
showError({
title: `Expecting a Vue component from the story: "${name}" of "${title}".`,
Expand All @@ -113,10 +131,10 @@ export function renderToDOM(
}

// @ts-expect-error https://github.com/storybookjs/storrybook/pull/7578#discussion_r307986139
root[VALUES] = { ...element.options[VALUES], ...args };
root[VALUES] = { ...element.options[VALUES] };

if (!map.has(domElement)) {
root.$mount(domElement);
root.$mount(mountTarget);
}

showMain();
Expand Down
1 change: 1 addition & 0 deletions code/renderers/vue/template/stories/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Placeholder until we write some render-specific stories

0 comments on commit e562342

Please sign in to comment.