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

Addon-docs: Fix react-dom/server imports breaking stories and docs #26557

Merged
merged 19 commits into from
Mar 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
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