From f1d2c5ff67fb86ce18ad6c4831596364c393d8c6 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Sun, 24 Mar 2024 23:19:27 +0100 Subject: [PATCH] Merge pull request #26557 from storybookjs/jeppe/fix-react-dom-server-resolution Addon-docs: Fix `react-dom/server` imports breaking stories and docs (cherry picked from commit d6eaf19838015f2b8680fe2439ea457f9b256fec) --- code/addons/docs/src/preset.ts | 4 +- .../template/stories/docs2/ResolvedReact.jsx | 28 ++++++++ .../template/stories/docs2/ResolvedReact.mdx | 30 ++++++-- .../stories/docs2/ResolvedReactVersion.jsx | 15 ---- .../stories/docs2/resolved-react.stories.ts | 70 +++++++++++++++++++ code/e2e-tests/addon-docs.spec.ts | 57 ++++++++++++--- code/e2e-tests/util.ts | 9 ++- code/lib/cli/src/sandbox-templates.ts | 6 ++ code/lib/react-dom-shim/src/preset.ts | 8 ++- code/renderers/preact/src/preset.ts | 16 +++++ .../template/stories/{React.js => React.jsx} | 0 ...at.stories.js => react-compat.stories.jsx} | 2 +- 12 files changed, 207 insertions(+), 38 deletions(-) create mode 100644 code/addons/docs/template/stories/docs2/ResolvedReact.jsx delete mode 100644 code/addons/docs/template/stories/docs2/ResolvedReactVersion.jsx create mode 100644 code/addons/docs/template/stories/docs2/resolved-react.stories.ts rename code/renderers/preact/template/stories/{React.js => React.jsx} (100%) rename code/renderers/preact/template/stories/{react-compat.stories.js => react-compat.stories.jsx} (96%) diff --git a/code/addons/docs/src/preset.ts b/code/addons/docs/src/preset.ts index 68c7efb39f8b..31248e7af990 100644 --- a/code/addons/docs/src/preset.ts +++ b/code/addons/docs/src/preset.ts @@ -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'; @@ -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, /** diff --git a/code/addons/docs/template/stories/docs2/ResolvedReact.jsx b/code/addons/docs/template/stories/docs2/ResolvedReact.jsx new file mode 100644 index 000000000000..f16c20f04fce --- /dev/null +++ b/code/addons/docs/template/stories/docs2/ResolvedReact.jsx @@ -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 ( + <> +

+ react:{' '} + + {ReactExport.version ?? 'no version export found'} + +

+

+ react-dom:{' '} + + {ReactDom.version ?? 'no version export found'} + +

+

+ react-dom/server:{' '} + + {ReactDomServer.version ?? 'no version export found'} + +

+ + ); +}; diff --git a/code/addons/docs/template/stories/docs2/ResolvedReact.mdx b/code/addons/docs/template/stories/docs2/ResolvedReact.mdx index 7a5f04ab6bc8..bd3ad01c2e08 100644 --- a/code/addons/docs/template/stories/docs2/ResolvedReact.mdx +++ b/code/addons/docs/template/stories/docs2/ResolvedReact.mdx @@ -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'; + + + +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 -react: {reactVersion} +react: {ReactExport.version ?? 'no version export found'} + +react-dom: {ReactDom.version ?? 'no version export found'} -react-dom: {reactDomVersion} +react-dom/server: {ReactDomServer.version ?? 'no version export found'} -## In `ResolvedReactVersion` component +## In `ResolvedReact` component - + diff --git a/code/addons/docs/template/stories/docs2/ResolvedReactVersion.jsx b/code/addons/docs/template/stories/docs2/ResolvedReactVersion.jsx deleted file mode 100644 index 6e094c1e64d0..000000000000 --- a/code/addons/docs/template/stories/docs2/ResolvedReactVersion.jsx +++ /dev/null @@ -1,15 +0,0 @@ -import React, { version as reactVersion } from 'react'; -import { version as reactDomVersion } from 'react-dom'; - -export const ResolvedReactVersion = () => { - return ( - <> -

- react: {reactVersion} -

-

- react-dom: {reactDomVersion} -

- - ); -}; diff --git a/code/addons/docs/template/stories/docs2/resolved-react.stories.ts b/code/addons/docs/template/stories/docs2/resolved-react.stories.ts new file mode 100644 index 000000000000..91f12041962b --- /dev/null +++ b/code/addons/docs/template/stories/docs2/resolved-react.stories.ts @@ -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: ` +

+ react: ${ + ReactExport.version ?? 'no version export found' + } +

+

+ react-dom: ${ + ReactDom.version ?? 'no version export found' + } +

+

+ react-dom/server: ${ + ReactDomServer.version ?? 'no version export found' + } +

+ `, + }, + 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); + }); + }, +}; diff --git a/code/e2e-tests/addon-docs.spec.ts b/code/e2e-tests/addon-docs.spec.ts index 72470acb62ab..db7b7b7d5e05 100644 --- a/code/e2e-tests/addon-docs.spec.ts +++ b/code/e2e-tests/addon-docs.spec.ts @@ -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 }) => { diff --git a/code/e2e-tests/util.ts b/code/e2e-tests/util.ts index df5ca4ff59a8..f97894075f5b 100644 --- a/code/e2e-tests/util.ts +++ b/code/e2e-tests/util.ts @@ -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); @@ -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'); diff --git a/code/lib/cli/src/sandbox-templates.ts b/code/lib/cli/src/sandbox-templates.ts index b28021be7049..9d8a839bfcce 100644 --- a/code/lib/cli/src/sandbox-templates.ts +++ b/code/lib/cli/src/sandbox-templates.ts @@ -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': { @@ -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': { diff --git a/code/lib/react-dom-shim/src/preset.ts b/code/lib/react-dom-shim/src/preset.ts index 63b2889ba93b..e863a53262b6 100644 --- a/code/lib/react-dom-shim/src/preset.ts +++ b/code/lib/react-dom-shim/src/preset.ts @@ -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'; /** @@ -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'); }; diff --git a/code/renderers/preact/src/preset.ts b/code/renderers/preact/src/preset.ts index 03b11e7e6097..28e5a428af0a 100644 --- a/code/renderers/preact/src/preset.ts +++ b/code/renderers/preact/src/preset.ts @@ -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; + } +}; diff --git a/code/renderers/preact/template/stories/React.js b/code/renderers/preact/template/stories/React.jsx similarity index 100% rename from code/renderers/preact/template/stories/React.js rename to code/renderers/preact/template/stories/React.jsx diff --git a/code/renderers/preact/template/stories/react-compat.stories.js b/code/renderers/preact/template/stories/react-compat.stories.jsx similarity index 96% rename from code/renderers/preact/template/stories/react-compat.stories.js rename to code/renderers/preact/template/stories/react-compat.stories.jsx index 33f1078d3154..b43a0650e748 100644 --- a/code/renderers/preact/template/stories/react-compat.stories.js +++ b/code/renderers/preact/template/stories/react-compat.stories.jsx @@ -1,4 +1,4 @@ -import { ReactFunctionalComponent, ReactClassComponent } from './React'; +import { ReactFunctionalComponent, ReactClassComponent } from './React.jsx'; export default { component: ReactFunctionalComponent,