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

Portable Stories: Improve Handling of React Updates and Errors #29044

Merged
merged 19 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
483325a
Portable Stories: Improve Handling of React Updates and Errors
valentinpalkovic Sep 4, 2024
8ea5c9e
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 4, 2024
da21bfe
Next.js: Update webpack configuration to support react-dom/test-utils
valentinpalkovic Sep 4, 2024
da2aee4
name decorators for easier debugging
JReinhold Sep 5, 2024
4f1b73d
fix duplicate default annotations
JReinhold Sep 5, 2024
58173b2
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 5, 2024
8a7d8ee
fix decorator type
JReinhold Sep 5, 2024
67f91ec
Merge branch 'valentin/propagate-error-in-testing' of github.com:stor…
JReinhold Sep 5, 2024
6109294
Merge remote-tracking branch 'origin/next' into valentin/propagate-er…
valentinpalkovic Sep 5, 2024
e4697f9
fix composing undefined defaultProjectAnnotations
JReinhold Sep 5, 2024
758aaa1
Next.js-Vite: Update vite-plugin-storybook-nextjs
valentinpalkovic Sep 5, 2024
b00a8ce
Merge branch 'valentin/propagate-error-in-testing' of github.com:stor…
JReinhold Sep 5, 2024
714913d
Next.js: Fix react-dom/test-utils aliasing
valentinpalkovic Sep 5, 2024
ea1a533
Fix Webpack aliasing
valentinpalkovic Sep 6, 2024
c24f221
Remove console.log
valentinpalkovic Sep 6, 2024
fe0843b
Merge branch 'next' of github.com:storybookjs/storybook into valentin…
JReinhold Sep 9, 2024
e672f1f
fix lock-file changes
JReinhold Sep 9, 2024
ceb8387
Add comment explaining asyncWrapper
kasperpeulen Sep 11, 2024
46aa6e0
Fix bug where @storybook/test is not imported but canvas is used dire…
kasperpeulen Sep 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export function setProjectAnnotations<TRenderer extends Renderer = Renderer>(
| NamedOrDefaultProjectAnnotations<TRenderer>[]
): NormalizedProjectAnnotations<TRenderer> {
const annotations = Array.isArray(projectAnnotations) ? projectAnnotations : [projectAnnotations];
if (globalThis.defaultProjectAnnotations) {
annotations.push(globalThis.defaultProjectAnnotations);
}

globalThis.globalProjectAnnotations = composeConfigs(annotations.map(extractAnnotation));

return globalThis.globalProjectAnnotations;
Expand Down
17 changes: 0 additions & 17 deletions code/lib/react-dom-shim/src/preventActChecks.tsx

This file was deleted.

6 changes: 2 additions & 4 deletions code/lib/react-dom-shim/src/react-16.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
import type { ReactElement } from 'react';
import * as ReactDOM from 'react-dom';

import { preventActChecks } from './preventActChecks';

export const renderElement = async (node: ReactElement, el: Element) => {
return new Promise<null>((resolve) => {
preventActChecks(() => void ReactDOM.render(node, el, () => resolve(null)));
ReactDOM.render(node, el, () => resolve(null));
});
};

export const unmountElement = (el: Element) => {
preventActChecks(() => void ReactDOM.unmountComponentAtNode(el));
ReactDOM.unmountComponentAtNode(el);
};
23 changes: 17 additions & 6 deletions code/lib/react-dom-shim/src/react-18.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
/* eslint-disable @typescript-eslint/no-unnecessary-type-constraint */
import type { FC, ReactElement } from 'react';
import type { ReactElement } from 'react';
import * as React from 'react';
import type { Root as ReactRoot, RootOptions } from 'react-dom/client';
import * as ReactDOM from 'react-dom/client';

import { preventActChecks } from './preventActChecks';

// A map of all rendered React 18 nodes
const nodes = new Map<Element, ReactRoot>();

const WithCallback: FC<{ callback: () => void; children: ReactElement }> = ({
declare const globalThis: {
IS_REACT_ACT_ENVIRONMENT: boolean;
};

function getIsReactActEnvironment() {
return globalThis.IS_REACT_ACT_ENVIRONMENT;
}

const WithCallback: React.FC<{ callback: () => void; children: ReactElement }> = ({
callback,
children,
}) => {
Expand Down Expand Up @@ -43,16 +49,21 @@ export const renderElement = async (node: ReactElement, el: Element, rootOptions
// Create Root Element conditionally for new React 18 Root Api
const root = await getReactRoot(el, rootOptions);

if (getIsReactActEnvironment()) {
root.render(node);
return;
}

const { promise, resolve } = Promise.withResolvers<void>();
preventActChecks(() => root.render(<WithCallback callback={resolve}>{node}</WithCallback>));
root.render(<WithCallback callback={resolve}>{node}</WithCallback>);
return promise;
};

export const unmountElement = (el: Element, shouldUseNewRootApi?: boolean) => {
const root = nodes.get(el);

if (root) {
preventActChecks(() => root.unmount());
root.unmount();
nodes.delete(el);
}
};
Expand Down
4 changes: 4 additions & 0 deletions code/renderers/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,16 @@
"require-from-string": "^2.0.2"
},
"peerDependencies": {
"@storybook/test": "workspace:*",
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0-beta",
"react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0-beta",
"storybook": "workspace:^",
"typescript": ">= 4.2.x"
},
"peerDependenciesMeta": {
"@storybook/test": {
"optional": true
},
"typescript": {
"optional": true
}
Expand Down
9 changes: 7 additions & 2 deletions code/renderers/react/src/__test__/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export const HooksStory: CSF3Story = {
);
},
play: async ({ canvasElement, step }) => {
console.log('start of play function');
const canvas = within(canvasElement);
await step('Step label', async () => {
const inputEl = canvas.getByTestId('input');
Expand All @@ -112,8 +111,8 @@ export const HooksStory: CSF3Story = {
await userEvent.type(inputEl, 'Hello world!');

await expect(inputEl).toHaveValue('Hello world!');
await expect(buttonEl).toHaveTextContent('I am clicked');
});
console.log('end of play function');
},
};

Expand Down Expand Up @@ -182,6 +181,12 @@ export const MountInPlayFunction: CSF3Story<{ mockFn: (val: string) => string }>
},
};

export const MountInPlayFunctionThrow: CSF3Story<{ mockFn: (val: string) => string }> = {
play: async () => {
throw new Error('Error thrown in play');
},
};

export const WithActionArg: CSF3Story<{ someActionArg: HandlerFunction }> = {
args: {
someActionArg: action('some-action-arg'),
Expand Down
13 changes: 13 additions & 0 deletions code/renderers/react/src/__test__/ComponentWithError.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import type { Meta, StoryObj } from '..';
import { ComponentWithError } from './ComponentWithError';

const meta = {
title: 'Example/ComponentWithError',
component: ComponentWithError as any,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider removing 'as any' and properly typing the component

} satisfies Meta<typeof ComponentWithError>;

export default meta;

type Story = StoryObj<typeof meta>;

export const ThrowsError: Story = {};
4 changes: 4 additions & 0 deletions code/renderers/react/src/__test__/ComponentWithError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export function ComponentWithError() {
// eslint-disable-next-line local-rules/no-uncategorized-errors
throw new Error('Error in render');
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,40 @@ exports[`Legacy Portable Stories API > Renders Modal story 1`] = `
</body>
`;

exports[`Legacy Portable Stories API > Renders MountInPlayFunction story 1`] = `
<body>
<div>
<div
data-testid="loaded-data"
>
loaded data
</div>
<div
data-testid="spy-data"
>
mockFn return value
</div>
</div>
</body>
`;

exports[`Legacy Portable Stories API > Renders MountInPlayFunctionThrow story 1`] = `
<body>
<div>
<div
data-testid="loaded-data"
>
loaded data
</div>
<div
data-testid="spy-data"
>
mockFn return value
</div>
</div>
</body>
`;

exports[`Legacy Portable Stories API > Renders WithActionArg story 1`] = `
<body>
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ describe('Legacy Portable Stories API', () => {
it.each(testCases)('Renders %s story', async (_storyName, Story) => {
cleanup();

if (_storyName === 'CSF2StoryWithLocale' || _storyName === 'MountInPlayFunction') {
if (
_storyName === 'CSF2StoryWithLocale' ||
_storyName === 'MountInPlayFunction' ||
_storyName === 'MountInPlayFunctionThrow'
) {
return;
}

Expand Down
Loading