Addon Docs: Remove react peer dependency

[JReinhold]

Works on #25179

This PR is an investigation into the possibility of removing React as a peer dependency of addon-docs, originally proposed at #24823.

Objective

Remove react and react-dom as a peer dependency of addon-docs - or alternatively turn into an optional peer dependency.

We want to enable non-React users to use addon-docs without needing to install React themselves and “bloating” their packages. React will no doubt remain in node_modules as a transitive dependency, but that still feels nicer.

Questions we want to answer

• Can we supply a default instance of React with addon-docs, while still allowing React users to use their own instance of React?

👆is important, because React users need to be able to define which version of React they want to use for rendering their components and stories.

Findings

We’ve found a strategy to supply react and react-dom by default with addon-docs (via a direct dependency) while still allowing users to supply their own instance and version by simply installing the packages in the root project if they want to. We’ve proved that this strategy works in almost¹ all situations.

This way, if the user does not have react installed, react@18.2.0 from addon-docs will be used.

The strategy is to attempt to get the path to react using require.resolve('react').

1. We try to resolve it in @storybook/core-server and @storybook/react presets, which will either get the user’s root react dependency, or fail. It will never resolve to the react in @storybook/addon-docs
2. If we get any paths from above, we store them as a resolvedReact property on presets.
3. Then in addon-docs we get that resolvedReact preset and if it’s empty (because the user didn’t have react in the root) we do the same resolution which will always resolve to the react that addon-docs has.
4. Then in addon-docs webpackFinal and viteFinal we alias all imports of react, react-dom and @mdx-js/react² to the paths defined in resolvedReact.
5. Now all imports of react are either from the root or from addon-docs.

Remaining Work

This PR is just a spike that proves this can work, there's more work to be done for this to be releasable.

See #25179

• Find out why the unit test is failing, maybe the React import needs aliased as well
• Update documentation to not say React is necessary - if we do that
• Update documentation to note that users can specify their own React version for docs if they need to
• Automigration to remove React as a dependency if possible - there might be a lot of edge cases here making this "if possible" detection very hard
• Stop adding React dependencies on init
• Test in all permutations of builders package managers and operating systems
• We might want to make it possible to opt-out of this, because there could be unknown unknowns where this strategy brakes users' setups

E2E Test

I propose that we add an E2E test that verifies that React is available in MDX.

1. Create an MDX file that imports version from react and displays it
2. Create a React component that does the same, and include it in the MD file
3. Add an E2E test to addon docs that visits this MDX doc and asserts the React version is rendered as expected.

Manual Test

Before merging this we should manually test it in multiple project permutations:

Renderer	Builder	Package manager	OS	Result
React	Vite	npm	macOS	✅
React	Vite	Yarn node_modules	macOS	✅
React	Vite	Yarn PnP	macOS	✅
React	Vite	pnpm	macOS	✅
React	Webpack	npm	macOS	✅
React	Webpack	Yarn node_modules	macOS	✅
React	Webpack	Yarn PnP	macOS	✅
React	Webpack	pnpm	macOS	✅
Vue3	Vite	npm	macOS	✅
Vue3	Vite	Yarn node_modules	macOS	✅
Vue3	Vite	Yarn PnP	macOS	✅
Vue3	Vite	pnpm	macOS	✅
Vue3	Webpack	npm	macOS	✅
Vue3	Webpack	Yarn node_modules	macOS	✅
Vue3	Webpack	Yarn PnP	macOS	✅
Vue3	Webpack	pnpm	macOS	✅
Vue3+React 17	Vite	npm	macOS	✅
Vue3+React 17	Vite	Yarn node_modules	macOS	✅
Vue3+React 17	Vite	Yarn PnP	macOS	❌
Vue3+React 17	Vite	pnpm	macOS	✅
Vue3+React 17	Webpack	npm	macOS	✅
Vue3+React 17	Webpack	Yarn node_modules	macOS	✅
Vue3+React 17	Webpack	Yarn PnP	macOS	❌
Vue3+React 17	Webpack	pnpm	macOS	✅
React	Vite	npm	Windows	✅
React	Vite	Yarn PnP	Windows	✅
SvelteKit	Vite	npm	Windows	✅
SvelteKit	Vite	Yarn PnP	Windows	❌
React	Webpack	npm	Windows	✅
React	Webpack	Yarn PnP	Windows	✅
Vue3	Webpack	npm	Windows	✅
Vue3	Webpack	Yarn PnP	Windows	✅
Vue3+React 17	Webpack	npm	Windows	✅
Vue3+React 17	Webpack	Yarn PnP	Windows	✅

This pull request has been released as version 0.0.0-pr-24881-sha-6ea23655. Install it by pinning all your Storybook dependencies to that version.

More information

Published version	0.0.0-pr-24[8](https://github.com/storybookjs/storybook/actions/runs/7207572473/job/19634682139#step:10:8)81-sha-6ea23655
Triggered by	@JReinhold
Repository	storybookjs/storybook
Branch	addon-docs-without-react
Commit	6ea23655
Datetime	Thu Dec 14 10:26:44 UTC 2023 (1702549604)
Workflow run	7207572473

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=[24](https://github.com/storybookjs/storybook/actions/runs/7207572473/job/19634682139#step:10:24)881

Footnotes

1. This doesn’t work in the specific combination of Yarn PnP + Non-React framework + user having its own React version in the root package. It doesn’t break, but it will always use react@18.2.0 from addon-docs and ignore the react in the root. We think this is okay because:

   • It’s only a problem in Yarn PnP
   • It’s only a problem in Non-React setups
   • It’s only a problem if the user wants to supply their own version of React

   Bullet 2 and 3 seems highly unlikely to appear at the same time, why would a Svelte user care about which React version their docs are using?

   It has a workaround, the user can set the resolvedReact property in main.ts manually with require.resolve('react') ↩

2. Webpack has a weird behavior with @mdx-js/react and symlinked stories like our template-stories in the monorepo. If the user has a custom React version installed, @mdx-js/react will not be hoisted from addon-docs, and then Webpack can't find it when rendering symlinked MDX files. So we explicitly alias it to fix this - this doesn't seem to be necessary in Vite projects. ↩

[valentinpalkovic]

Well done Jeppe!

[socket-security]

New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@babel/preset-flow	7.22.15	None	+2	30.5 kB	nicolo-ribaudo
next	14.0.3...14.0.4	environment	+10/-11	1.15 GB	vercel-release-bot
@types/react-transition-group	4.4.9...4.4.10	None	+3/-2	399 kB	types
@storybook/theming	7.6.3...7.5.0	None	+0/-0	724 kB	shilman
@storybook/client-logger	7.6.3...7.5.0	None	+0/-0	7.45 kB	shilman
vue	3.3.9...3.3.11	None	+14/-24	11.3 MB	yyx990803
ws	8.14.2...8.15.0	None	+0/-0	140 kB	lpinca
@vue/compiler-sfc	3.3.9...3.3.11	None	+2/-6	2.31 MB	yyx990803
@chromaui/addon-visual-tests	0.0.114...0.0.124	None	+4/-2	8.42 MB	tmeasday
preact	10.19.2...10.19.3	None	+0/-0	1.35 MB	marvinhagemeister
@vitejs/plugin-vue	4.5.1...4.5.2	None	+9/-13	7.03 MB	vitebot

🚮 Removed packages: html-webpack-plugin@5.5.3

[socket-security]

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: json-parse-even-better-errors@3.0.0, ignore-walk@6.0.3, json-stable-stringify@1.0.2, flat-cache@3.1.0

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

[ndelangen]

@SocketSecurity ignore @vitejs/plugin-vue@4.4.0
@SocketSecurity ignore json-parse-even-better-errors@3.0.0
@SocketSecurity ignore ignore-walk@6.0.3
@SocketSecurity ignore json-stable-stringify@1.0.2
@SocketSecurity ignore @babel/preset-modules@0.1.6
@SocketSecurity ignore piscina@4.0.0
@SocketSecurity ignore flat-cache@3.1.0

[ndelangen]

Looks like this work is conflicting with the recent work to support RSC, @shilman

[JReinhold]

Tested this manually across all the project permutations, and everything works as expected. The cases that don't work are already known.

🎉

[github-actions]

	Fails
🚫	PR is marked with "BREAKING CHANGE" label.

Generated by 🚫 dangerJS against 2db44d7

[ndelangen]

@JReinhold merge?