Skip to content

Commit

Permalink
Merge pull request #26557 from storybookjs/jeppe/fix-react-dom-server…
Browse files Browse the repository at this point in the history
…-resolution

Addon-docs: Fix `react-dom/server` imports breaking stories and docs
(cherry picked from commit d6eaf19)
  • Loading branch information
JReinhold authored and storybook-bot committed Mar 25, 2024
1 parent d384797 commit f229d3e
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 38 deletions.
4 changes: 3 additions & 1 deletion code/addons/docs/src/preset.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dirname, join } from 'path';
import { dirname, join, isAbsolute } from 'path';
import rehypeSlug from 'rehype-slug';
import rehypeExternalLinks from 'rehype-external-links';

Expand Down Expand Up @@ -147,6 +147,8 @@ export const viteFinal = async (config: any, options: Options) => {
resolve: {
alias: {
react,
// Vite doesn't respect export maps when resolving an absolute path, so we need to do that manually here
...(isAbsolute(reactDom) && { 'react-dom/server': `${reactDom}/server.browser.js` }),
'react-dom': reactDom,
'@mdx-js/react': mdx,
/**
Expand Down
28 changes: 28 additions & 0 deletions code/addons/docs/template/stories/docs2/ResolvedReact.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React, * as ReactExport from 'react';
import * as ReactDom from 'react-dom';
import * as ReactDomServer from 'react-dom/server';

export const ResolvedReact = () => {
return (
<>
<p>
<code>react</code>:{' '}
<code data-testid="component-react">
{ReactExport.version ?? 'no version export found'}
</code>
</p>
<p>
<code>react-dom</code>:{' '}
<code data-testid="component-react-dom">
{ReactDom.version ?? 'no version export found'}
</code>
</p>
<p>
<code>react-dom/server</code>:{' '}
<code data-testid="component-react-dom-server">
{ReactDomServer.version ?? 'no version export found'}
</code>
</p>
</>
);
};
30 changes: 23 additions & 7 deletions code/addons/docs/template/stories/docs2/ResolvedReact.mdx
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
import { version as reactVersion } from 'react';
import { version as reactDomVersion } from 'react-dom';
import { ResolvedReactVersion } from './ResolvedReactVersion';
import { Meta } from '@storybook/blocks';
import * as ReactExport from 'react';
import * as ReactDom from 'react-dom';
import * as ReactDomServer from 'react-dom/server';
import { ResolvedReact } from './ResolvedReact';

<Meta title="docs2/ResolvedReact" name="MDX"/>

This doc is used to display the resolved version of React and its related packages.
As long as `@storybook/addon-docs` is installed, `react` and `react-dom` should be available to import from and should resolve to the same version.

The MDX here ensures that it works in an MDX file.

- See the [autodocs](/docs/addons-docs-docs2-resolvedreact--docs) for how it resolves in autodocs.
- See the [Story](/story/addons-docs-docs2-resolvedreact--story) for how it resolves in the actual story.

**Note: There appears to be a bug in the _production_ build of `react-dom`, where it reports version `18.2.0-next-9e3b772b8-20220608` while in fact version `18.2.0` is installed.**

## In MDX

<code>react</code>: <code data-testid="mdx-react">{reactVersion}</code>
<code>react</code>: <code data-testid="mdx-react">{ReactExport.version ?? 'no version export found'}</code>

<code>react-dom</code>: <code data-testid="mdx-react-dom">{ReactDom.version ?? 'no version export found'}</code>

<code>react-dom</code>: <code data-testid="mdx-react-dom">{reactDomVersion}</code>
<code>react-dom/server</code>: <code data-testid="mdx-react-dom-server">{ReactDomServer.version ?? 'no version export found'}</code>

## In `ResolvedReactVersion` component
## In `ResolvedReact` component

<ResolvedReactVersion />
<ResolvedReact />
15 changes: 0 additions & 15 deletions code/addons/docs/template/stories/docs2/ResolvedReactVersion.jsx

This file was deleted.

70 changes: 70 additions & 0 deletions code/addons/docs/template/stories/docs2/resolved-react.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { within, expect } from '@storybook/test';
import * as ReactExport from 'react';
import * as ReactDom from 'react-dom';
import * as ReactDomServer from 'react-dom/server';

/**
* This component is used to display the resolved version of React and its related packages.
* As long as `@storybook/addon-docs` is installed, `react` and `react-dom` should be available to import from and should resolve to the same version.
*
* The autodocs here ensures that it also works in the generated documentation.
*
* - See the [MDX docs](/docs/addons-docs-docs2-resolvedreact--mdx) for how it resolves in MDX.
* - See the [Story](/story/addons-docs-docs2-resolvedreact--story) for how it resolves in the actual story.
*
* **Note: There appears to be a bug in the _production_ build of `react-dom`, where it reports version `18.2.0-next-9e3b772b8-20220608` while in fact version `18.2.0` is installed.**
*/
export default {
title: 'Docs2/ResolvedReact',
component: globalThis.Components.Html,
tags: ['autodocs'],
argTypes: {
content: { table: { disable: true } },
},
args: {
content: `
<p>
<code>react</code>: <code data-testid="react">${
ReactExport.version ?? 'no version export found'
}</code>
</p>
<p>
<code>react-dom</code>: <code data-testid="react-dom">${
ReactDom.version ?? 'no version export found'
}</code>
</p>
<p>
<code>react-dom/server</code>: <code data-testid="react-dom-server">${
ReactDomServer.version ?? 'no version export found'
}</code>
</p>
`,
},
parameters: {
docs: {
name: 'ResolvedReact',
},
},
};

export const Story = {
// This test is more or less the same as the E2E test we have for MDX and autodocs entries in addon-docs.spec.ts
play: async ({ canvasElement, step, parameters }: any) => {
const canvas = await within(canvasElement);

const actualReactVersion = (await canvas.findByTestId('react')).textContent;
const actualReactDomVersion = (await canvas.findByTestId('react-dom')).textContent;
const actualReactDomServerVersion = (await canvas.findByTestId('react-dom-server')).textContent;

step('Expect React packages to all resolve to the same version', () => {
// react-dom has a bug in its production build, reporting version 18.2.0-next-9e3b772b8-20220608 even though version 18.2.0 is installed.
expect(actualReactDomVersion!.startsWith(actualReactVersion!)).toBeTruthy();

if (parameters.renderer === 'preact') {
// the preact/compat alias doesn't have a version export in react-dom/server
return;
}
expect(actualReactDomServerVersion).toBe(actualReactVersion);
});
},
};
57 changes: 49 additions & 8 deletions code/e2e-tests/addon-docs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,30 +185,71 @@ test.describe('addon-docs', () => {
});

test('should resolve react to the correct version', async ({ page }) => {
// Arrange - Navigate to MDX docs
const sbPage = new SbPage(page);
await sbPage.navigateToUnattachedDocs('addons/docs/docs2', 'ResolvedReact');
await sbPage.navigateToStory('addons/docs/docs2/resolvedreact', 'mdx', 'docs');
const root = sbPage.previewRoot();

let expectedReactVersion = /^18/;
// Arrange - Setup expectations
let expectedReactVersionRange = /^18/;
if (
templateName.includes('preact') ||
templateName.includes('react-webpack/17') ||
templateName.includes('react-vite/17')
) {
expectedReactVersion = /^17/;
expectedReactVersionRange = /^17/;
} else if (templateName.includes('react16')) {
expectedReactVersion = /^16/;
expectedReactVersionRange = /^16/;
}

// Arrange - Get the actual versions
const mdxReactVersion = await root.getByTestId('mdx-react');
const mdxReactDomVersion = await root.getByTestId('mdx-react-dom');
const mdxReactDomServerVersion = await root.getByTestId('mdx-react-dom-server');
const componentReactVersion = await root.getByTestId('component-react');
const componentReactDomVersion = await root.getByTestId('component-react-dom');
const componentReactDomServerVersion = await root.getByTestId('component-react-dom-server');

// Assert - The versions are in the expected range
await expect(mdxReactVersion).toHaveText(expectedReactVersionRange);
await expect(componentReactVersion).toHaveText(expectedReactVersionRange);
await expect(mdxReactDomVersion).toHaveText(expectedReactVersionRange);
await expect(componentReactDomVersion).toHaveText(expectedReactVersionRange);
if (!templateName.includes('preact')) {
// preact/compat alias doesn't have a version export in react-dom/server
await expect(mdxReactDomServerVersion).toHaveText(expectedReactVersionRange);
await expect(componentReactDomServerVersion).toHaveText(expectedReactVersionRange);
}

// Arrange - Navigate to autodocs
await sbPage.navigateToStory('addons/docs/docs2/resolvedreact', 'docs');

// Arrange - Get the actual versions
const autodocsReactVersion = await root.getByTestId('react');
const autodocsReactDomVersion = await root.getByTestId('react-dom');
const autodocsReactDomServerVersion = await root.getByTestId('react-dom-server');

// Assert - The versions are in the expected range
await expect(autodocsReactVersion).toHaveText(expectedReactVersionRange);
await expect(autodocsReactDomVersion).toHaveText(expectedReactVersionRange);
if (!templateName.includes('preact')) {
await expect(autodocsReactDomServerVersion).toHaveText(expectedReactVersionRange);
}

// Arrange - Navigate to story
await sbPage.navigateToStory('addons/docs/docs2/resolvedreact', 'story');

await expect(mdxReactVersion).toHaveText(expectedReactVersion);
await expect(mdxReactDomVersion).toHaveText(expectedReactVersion);
await expect(componentReactVersion).toHaveText(expectedReactVersion);
await expect(componentReactDomVersion).toHaveText(expectedReactVersion);
// Arrange - Get the actual versions
const storyReactVersion = await root.getByTestId('react');
const storyReactDomVersion = await root.getByTestId('react-dom');
const storyReactDomServerVersion = await root.getByTestId('react-dom-server');

// Assert - The versions are in the expected range
await expect(storyReactVersion).toHaveText(expectedReactVersionRange);
await expect(storyReactDomVersion).toHaveText(expectedReactVersionRange);
if (!templateName.includes('preact')) {
await expect(storyReactDomServerVersion).toHaveText(expectedReactVersionRange);
}
});

test('should have stories from multiple CSF files in autodocs', async ({ page }) => {
Expand Down
9 changes: 4 additions & 5 deletions code/e2e-tests/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class SbPage {
/**
* Visit a story by selecting it from the sidebar.
*/
async navigateToStory(title: string, name: string) {
async navigateToStory(title: string, name: string, viewMode?: 'docs' | 'story') {
await this.openComponent(title);

const titleId = toId(title);
Expand All @@ -50,11 +50,10 @@ export class SbPage {
const storyLink = this.page.locator('*', { has: this.page.locator(`> ${storyLinkId}`) });
await storyLink.click({ force: true });

// assert url changes
const viewMode = name === 'docs' ? 'docs' : 'story';

await this.page.waitForURL((url) =>
url.search.includes(`path=/${viewMode}/${titleId}--${storyId}`)
url.search.includes(
`path=/${viewMode ?? name === 'docs' ? 'docs' : 'story'}/${titleId}--${storyId}`
)
);

const selected = await storyLink.getAttribute('data-selected');
Expand Down
6 changes: 6 additions & 0 deletions code/lib/cli/src/sandbox-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,9 @@ const baseTemplates = {
renderer: '@storybook/preact',
builder: '@storybook/builder-vite',
},
modifications: {
extraDependencies: ['preact-render-to-string'],
},
skipTasks: ['e2e-tests-dev', 'bench'],
},
'preact-vite/default-ts': {
Expand All @@ -439,6 +442,9 @@ const baseTemplates = {
renderer: '@storybook/preact',
builder: '@storybook/builder-vite',
},
modifications: {
extraDependencies: ['preact-render-to-string'],
},
skipTasks: ['e2e-tests-dev', 'bench'],
},
'qwik-vite/default-ts': {
Expand Down
8 changes: 7 additions & 1 deletion code/lib/react-dom-shim/src/preset.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Options } from '@storybook/types';
import { join, dirname } from 'path';
import { join, dirname, isAbsolute } from 'path';
import { readFile } from 'fs/promises';

/**
Expand All @@ -17,6 +17,12 @@ const getIsReactVersion18 = async (options: Options) => {
const resolvedReact = await options.presets.apply<{ reactDom?: string }>('resolvedReact', {});
const reactDom = resolvedReact.reactDom || dirname(require.resolve('react-dom/package.json'));

if (!isAbsolute(reactDom)) {
// if react-dom is not resolved to a file we can't be sure if the version in package.json is correct or even if package.json exists
// this happens when react-dom is resolved to 'preact/compat' for example
return false;
}

const { version } = JSON.parse(await readFile(join(reactDom, 'package.json'), 'utf-8'));
return version.startsWith('18') || version.startsWith('0.0.0');
};
Expand Down
16 changes: 16 additions & 0 deletions code/renderers/preact/src/preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,19 @@ export const previewAnnotations: PresetProperty<'previewAnnotations'> = async (
.concat([join(__dirname, 'entry-preview.mjs')])
.concat(docsEnabled ? [join(__dirname, 'entry-preview-docs.mjs')] : []);
};

/**
* Alias react and react-dom to preact/compat similar to the preact vite preset
* https://github.com/preactjs/preset-vite/blob/main/src/index.ts#L238-L239
*/
export const resolvedReact = async (existing: any) => {
try {
return {
...existing,
react: 'preact/compat',
reactDom: 'preact/compat',
};
} catch (e) {
return existing;
}
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ReactFunctionalComponent, ReactClassComponent } from './React';
import { ReactFunctionalComponent, ReactClassComponent } from './React.jsx';

export default {
component: ReactFunctionalComponent,
Expand Down

0 comments on commit f229d3e

Please sign in to comment.