Core: Move react and react-dom to peer deps

[gaetanmaisse]

Follow up #12908 (comment)
More or less revert #11628

What I did

As stated by @Hypnosphi:

react and react-dom should always be peer dependencies of packages that provide react components to ensure that there's a single instance of those.

So I moved them to peerDep in some base packages. Plus, all packages depending on a package with react as peerDep should also have it as peerDeps (transitive peer dependencies, for details: https://dev.to/arcanis/implicit-transitive-peer-dependencies-ed0).

Finally, packages in examples/* are "end" packages using Storybook so they must provide react and react-dom in their deps/devDeps. So, I put react and react-dom to ^16.14.0 (latest 16.x available).

How to test

• CI should be 🟢

Questions

• I used 15 || 16 || ^17.0.0 as version range for both react and react-dom when they are peerDep. Does it look good to you?
• Some app packages should maybe have react and react-dom as deps instead of peerDeps, I mean @storybook/angular would find weird to have to add these deps in their project themselves. 🤷🏻
• If we keep react and react-dom as peerDeps in app packages we should update the CLI to detect and add if needed add them automagically when running sb init
• Should we add an example with React 17? We now have one for React 16 and one for React 15

Did I miss something @Hypnosphi? and what's your opinion on that?

[gaetanmaisse]

You're right @Hypnosphi it's just a workaround we discuss this morning to avoid yarn/npm warnings.

Do you have something in mind for this kind of feedbacks:

As a dev on an angular/vue project I don't want to have to add react and react-dom to my project as it's Storybook internal stuff. Storybook should work out of the box with @storybook/angular(/vue) and some addons.

For the @storybook/[app] (non react apps) we can add React as a regular dep because there will be no React version conflict. But what can we do for addons 🤔 They need react but we are sure to have it because addons cannot work without a @storybook/[app], that's why we come up with this optional peerDep workaround 🤯

Other options: we keep react as peerDep everywhere and users need to add react to their own project. But @ndelangen told me it can be a pain to deal with react in a vue project for instance. 🤷🏻

[Hypnosphi]

OK let's make them optional until NPM 7 becomes mainstream (probably in a year)

[shilman]

In #12925 I made them peer deps for everything except the non-React frameworks. So @storybook/vue will install React internally. I think this is the right split.

[gaetanmaisse]

Monthly meeting sum up:
https://github.com/storybookjs/community/blob/master/meetings/2020/monthly-2020-10-02.md#react-as-peerdep-vs-dep

Still one pending question:

• A fixed version of react in all examples?

[shilman]

@gaetanmaisse

A fixed version of react in all examples?

Yes except cra-react15, unless we want to move that to examples-v2, which I'd be OK with.

NOTE: Currently cra-react15 is also where we do our IE11 chromatic test, so we'd need to update that too.

[tmeasday]

Shouldn't our (non-react) examples only install react via the @storybook/X framework package? Or does that lead to peer dep warnings and we don't like that?

[shilman]

Looking great @gaetanmaisse

[shilman]

@tmeasday this PR already implements that (non-react frameworks automatically install react on behalf of users)

[stof]

this PR already implements that (non-react frameworks automatically install react on behalf of users)

that's actually not true for package manager which are strict about peer dependencies (pnpm and yarn 2 for instance). The react dependency of @storybook/html will satisfy a peer dependency of a package being a dependency of @storybook/html, but not of an addon being a dependency of the project and so a sibling of @storybook/html.
And if the project adds a react requirement to satisfy the peer dependency, you end up again with 2 different versions of react.

yarn 1 and npm 6 are more lenient here, because they validate the peer dependencies based on hoisted packages too.

[gaetanmaisse]

@stof Actually it's still a bit more complex than that because there is a webpack alias in SB manager config so all React imports are aliased to the React version that satisfies @storybook/core peer dep. So even if there is another version of React to satisfy an addon peer dep it won't be used by Storybook.
I have created #13194 + comments to sum-up the react dep situation, feel free to add a comment if something is unclear!

[stof]

@gaetanmaisse thanks for the link