storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
83.41k stars 9.12k forks source link

framework-preset-react-dom-hack for React versions < 18 isn’t loading without addons declared #18662

Open thibaudcolas opened 2 years ago

thibaudcolas commented 2 years ago

Describe the bug

Follow-up to #18402, specifically for React versions before 18. Switching from v6.4 to v6.5 with Webpack 5, I now get the following error upon building Storybook:

ERR! => Failed to build the preview
ERR! Module not found: Error: Can't resolve 'react-dom/client' in '[…]/node_modules/@storybook/react/dist/esm/client/preview'

This is because of the react-dom/client dynamic import added for React 18 compatibility: https://github.com/storybookjs/storybook/blob/e2673f765722cbb542ef1b5cf8d533c8e746a127/app/react/src/client/preview/render.tsx#L76

The React framework has framework-preset-react-dom-hack set up with an IgnorePlugin to work around this, however this isn’t being loaded for some reason.

When logging Webpack plugins via webpackFinal, I can see the IgnorePlugin isn’t loaded (it should be last):

[
  VirtualModulesPlugin […]
  HtmlWebpackPlugin […]
  DefinePlugin […]
  ProvidePlugin […]
  CaseSensitivePathsPlugin […]
  ProgressPlugin […]
]

This seems to be due to the config’s lack of a addons: [] (see below).

To Reproduce

System

  System:
    OS: macOS 12.4
  Binaries:
    Node: 16.15.1
    npm: 8.11.0
  Browsers: […]

Additional context

This is fixed by adding addons: [] to the config, as shown in this separate config. This is very puzzling. I don’t know how Storybook combines the different framework presets so didn’t investigate further. I can at least see the IgnorePlugin at the end of the list of Webpack plugins as expected, and everything else also seems to be working:

IgnorePlugin {
  options: {
    resourceRegExp: /react-dom\/client$/,
    contextRegExp: /(app\/react|app\\react|@storybook\/react|@storybook\\react)/
  },
  checkIgnore: [Function: bound checkIgnore]
}
philmill commented 1 year ago

We were having issues with our GH action where we have addons array populated but still getting this error. What fixed for us was adding --legacy-peer-deps to npm install command. I noticed while upgrading Storybook to 6.5.13 that it mentioned using this flag with npm 8.

wadefleming-nz commented 1 year ago

I am getting this too.

ibrahim-kurhan commented 1 year ago

I am trying to upgrade from 5.3.18 to 6.5 and getting to same problem

IanVS commented 1 year ago

For those of you having problems with this, could you try upgrading to storybook 7.0 to see if it's still a problem there? You can run npx sb@next upgrade --prerelease.

thibaudcolas commented 1 year ago

@IanVS could you take a look at the repro I provided? Running it + your suggested upgrade step, I get a different error, which I think is due to issues with the upgrade script:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './preset' is not defined by "exports" in /Users/thibaudcolas/Dev/playground/storybook-react-16/node_modules/@storybook/react/package.json

Of all the dependencies in the repro, I see @storybook/manager-webpack5@6.5.9 hasn’t been auto-upgraded which could explain this. Here are the relevant logs for this part of the upgrade script which show this package wasn’t covered:

info  @storybook/addon-a11y            6.5.9  →   7.0.0-beta.16
info  @storybook/addon-actions         6.5.9  →   7.0.0-beta.16
info  @storybook/addon-essentials      6.5.9  →   7.0.0-beta.16
info  @storybook/addon-interactions    6.5.9  →   7.0.0-beta.16
info  @storybook/addon-links           6.5.9  →   7.0.0-beta.16
info  @storybook/builder-webpack5      6.5.9  →   7.0.0-beta.16
info  @storybook/react                 6.5.9  →   7.0.0-beta.16
info  @storybook/testing-library     ^0.0.13  →  ^0.0.14-next.1

[…]
WARN Found 1 outdated packages (relative to '@storybook/addon-a11y@7.0.0-beta.16')
WARN Please make sure your packages are updated to ensure a consistent experience.
WARN - @storybook/manager-webpack5@6.5.9
IanVS commented 1 year ago

That package should have been removed, not sure why it wasn't. But you can delete it as it is no longer required.

thibaudcolas commented 1 year ago

I’m still getting the same error as before even after having manually removed that dependency. Here is the code that throws this error: https://github.com/storybookjs/storybook/blob/a8617fd870d5c645b9239ccdfe8c6c89bf83ae09/code/lib/core-common/src/presets.ts#L109

There is no preset.js file inside node_modules/@storybook/react v7.0.0-beta.16, nor ./preset entry in the package.json exports. I don’t see why this would be called on @storybook/react but my knowledge of the internals stops there.

I’ll let you or someone else take it from there with the repro I provided. It’s a pretty simple Storybook v6 + React 16 project, so both the original issue and this upgrade issue are very straightforward to double check.

IanVS commented 1 year ago

You don't have a .storybook/main.js, so the automigrations weren't able to run completely. In this case, you'd need to either change one of your storybook config folders to .storybook, and re-run npx sb@next automigrate, or perform the migration steps from https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#from-version-65x-to-700 manually.

AvnerCohen commented 3 months ago

FYI This solves this for me as a post install step:

cp ./node_modules/@storybook/react-dom-shim/dist/react-16.js ./node_modules/@storybook/react-dom-shim/dist/react-18.js
cp ./node_modules/@storybook/react-dom-shim/dist/react-16.mjs ./node_modules/@storybook/react-dom-shim/dist/react-18.mjs

This is when using react 16 and storybook 8.

I tried to see if there is a way for me to indicate react-16 to be used here but I see the default (react-18) is always being used.

unional commented 2 months ago

It seems like only 16 and 18 is supported. Is it possible to add 17?

https://github.com/storybookjs/storybook/tree/next/code/lib/react-dom-shim/src

TenetMax commented 1 month ago

It seems like only 16 and 18 is supported. Is it possible to add 17?

https://github.com/storybookjs/storybook/tree/next/code/lib/react-dom-shim/src

I see this problem on React 17, and @AvnerCohen 's post install step resolves this issue for me. I suspect it works because the behavior of reactDom between 16 and 17 is identical or at least compatible.

vgoreev commented 3 weeks ago

I've solved this issue for me. We've got React 17 in our project but @storybook requires React 18 for its manager plugin. So I came up with this workaround. So essentially Storybook uses a package called @storybook/react-dom-shim to create a unified way of mounting and unmounting components. We need to explicitly tell storybook's webpack to use react-16 version of a shim. Here is a code that worked for me.

// main.ts
     ...

    webpackFinal(config) {
        config.resolve = {
            extensions: ['.ts', '.tsx', '.js', '.jsx'],
            alias: {
                react: getAbsolutePath('react'),
                'react-dom': getAbsolutePath('react-dom'),
                '@storybook/react-dom-shim': '@storybook/react-dom-shim/dist/react-16',
            },
        };

        return config;
    },