Skip to content

Commit

Permalink
Portable Stories: Improve Handling of React Updates and Errors
Browse files Browse the repository at this point in the history
Co-authored-by: Yann Braga <[email protected]>
Co-authored-by: Jeppe Reinhold <[email protected]>
  • Loading branch information
3 people committed Sep 4, 2024
1 parent dea51a7 commit 483325a
Show file tree
Hide file tree
Showing 16 changed files with 557 additions and 75 deletions.
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,
} 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

0 comments on commit 483325a

Please sign in to comment.