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

Storybook 4 not compatible with react 15 #4191

Closed
Sujimoshi opened this issue Sep 17, 2018 · 44 comments
Closed

Storybook 4 not compatible with react 15 #4191

Sujimoshi opened this issue Sep 17, 2018 · 44 comments
Assignees
Labels
Milestone

Comments

@Sujimoshi
Copy link

Sujimoshi commented Sep 17, 2018

Bug

Expected: Min version of react is set to react >= 15.0.0 , but real minimal version of react set by @emotion/core which need react >= 16.3.0
Using react 15 produce error on loading storybook
image

Please specify which version of Storybook and optionally any affected addons that you're running

  • @storybook/react 4.0.0-alpha.21
  • react 15

Screenshots / Screencast / Code Snippets (Optional)

// @storybook/core/package.json
"dependencies": {
    "@emotion/core": "0.13.0",
},
"peerDependencies": {
    "react": ">=15.0.0",
    "react-dom": ">=15.0.0"
},
// @emotion/core/package.json
"peerDependencies": {
    "react": ">=16.3.0"
},
@pksunkara pksunkara added this to the v4.0.0 milestone Sep 18, 2018
@Sujimoshi
Copy link
Author

It's frustrating, since I can not use the react 15 and webpack 4 in my application if it use storybook

@pksunkara pksunkara removed this from the v4.0.0 milestone Sep 18, 2018
@ndelangen
Copy link
Member

ndelangen commented Sep 18, 2018

Hey @Sujimoshi thanks for the issue.

We want to help you as much as wel can. I hope you understand we're doing the best we can to stay backwards compatible AND allow users to use the latest and greatest AND have a nice experience doing it ourselves.

Right now these are my thoughts on the matter:
We're moving forward with "React 16 compatibility"-only for the moment. We didn't want to exclude React 15, it just happened with updating to the modern tools/libs.

This may sounds strange, but React is the only lib that has this problem because of how we're using webpack & set up the package dependencies. We want to change this.

Mid term, we want to split the manager & preview more and this would allow you to run any version of react, and us to run our own.

Short term there are a couple of options:

  • Keep using the alpha.20
  • Upgrade to React 16
  • Find some compatibility lib
  • Assist us with achieving the above faster. I'd be super happy to have a chat about it in detail.
  • Assist us in bringing back compatibility using some other method

edit:
Seems we might be able to change react peerDependency to a regular dependency and resolve this issue that way

I understand this is not exactly the news you were hoping to get, Let's help each other!

@Sujimoshi
Copy link
Author

Isn't alpha.20 have the same dependencies?

@Sujimoshi Sujimoshi reopened this Sep 18, 2018
@Sujimoshi
Copy link
Author

I am not familiar with storybook source code, but why not just remove react from peerDeps, and allow storybook webpack bundle two versions of react

@brochington
Copy link

I am in the process of updating a React 15 application to use Webpack 4 and Babel 7. An upgrade to React 16 is in the horizon, but our app is quite large, and will take quite a long time.

Of the options that @ndelangen posted above, I chose to pin the version of Storybook.

Just a note that I needed to pin to version 4.0.0-alpha.18 to get things to work.

@Hypnosphi
Copy link
Member

@Sujimoshi how does your .storybook/webpack.config.js look? There's a chance we could find out how to make it work with [email protected]

@Sujimoshi
Copy link
Author

Sujimoshi commented Sep 18, 2018

Just a note that I needed to pin to version 4.0.0-alpha.18 to get things to work.

Thank for you reply, my project is huge, so i can't just update to react 16. But pin to 4.0.0-alpha.18 make sense.

@Sujimoshi
Copy link
Author

Sujimoshi commented Sep 18, 2018

@Sujimoshi how does your .storybook/webpack.config.js look? There's a chance we could find out how to make it work with [email protected]

I'm in no hurry to switch to webpack 4. I just would like to solve the problem with backward compatibility, and help Storybook. Anyway here is my webpack 4 config:

const path = require('path')

module.exports = (unused, args) => {
  const env = args.mode || 'production'
  const isProd = env === 'production'

  const config = {}
  config.module = { rules: [] }
  config.plugins = []

  config.mode = isProd ? 'production' : 'development'

  config.entry = path.resolve(__dirname, 'src/index.js')

  config.output = {
    path: path.resolve(__dirname, 'dist'),
    filename: 'index.js',
  }

  config.module.rules.push({
    test: /\.js$/,
    exclude: /(node_modules|bower_components)/,
    use: {
      loader: 'babel-loader',
    },
  })

  config.module.rules.push({
    test: /\.scss$/,
    use: [
      { loader: 'style-loader' },
      { loader: 'css-loader' },
      { loader: 'sass-loader' }],
  })

  config.module.rules.push({
    test: /\.(png|svg|jpg|gif)$/,
    use: [
      'file-loader',
    ],
  })

  return config
}

.storybook/webpack.config.js

// Storybook specific webpack configuration

// Require project-wide webpack configuration
const mainConfig = require('../webpack.config')(null, { mode: 'development' })

module.exports = mainConfig

@Sujimoshi
Copy link
Author

Sujimoshi commented Sep 18, 2018

I just managed to launch Storybook 4 with webpack 4 and react 15. As I mentioned above, I just set react and react-dom 16.4.1 as direct dependencies of Storybook/core, Storybook/react and Storybook/ui. So after the start-storybook, there were two versions of the react inside the bundle, React 15 for my project, and React 16 for storybook components. And it works

@Sujimoshi
Copy link
Author

Why react is listed as peerDependency, if Storybook generally use react for its ui. I'm surprised that this problem did not arise before

@ndelangen
Copy link
Member

I just set react and react-dom 16.4.1 as forward dependencies of Storybook/core, Storybook/react and Storybook/ui. So after the start-storybook, there were two versions of the react inside the bundle, React 15 for my project, and React 16 for storybook components. And it works

Can you please help me understand what you mean with this?
I'm unfamiliar with "forward dependencies".

@Sujimoshi
Copy link
Author

Direct dependency (just dependencies in package.json), sorry for my english

@ndelangen
Copy link
Member

No problem, I think I understand now.

I guess if we set react & react-dom as regular dependencies with 16 or higher, this will fix this issue too.

Is what you're saying..?


Yeah that sounds like something we could do, to preserve React <16 compatibility!

@Hypnosphi is our peerDependency master, do you see any issues with this?

The version of react Storybook needs, is whatever emotion 10 needs.

@Sujimoshi
Copy link
Author

Exactly

@Hypnosphi
Copy link
Member

@Sujimoshi you have no plugins, so there are good chances that your setup will work with Storybook 3.4 and webpack 4 as is

@Hypnosphi
Copy link
Member

Hypnosphi commented Sep 19, 2018

@norbert We use React & React DOM in preview iframe as well: https://github.com/storybooks/storybook/blob/master/app/react/src/client/preview/render.js

We can actually use resolve.alias to enforce our version. But this means that components that use some APIs removed in 16 (like React.PropTypes) will break. Which can in turn be fixed by user-provided resolve.alias in custom webpack config =)

@Sujimoshi
Copy link
Author

@norbert We use React & React DOM in preview iframe as well: https://github.com/storybooks/storybook/blob/master/app/react/src/client/preview/render.js

Is this a problem?

We can actually use resolve.alias to enforce our version. But this means that components that use some APIs removed in 16 (like React.PropTypes) will break. Which can in turn be fixed by user-provided resolve.alias in custom webpack config =)

Not the best solution, as for me

@Sujimoshi you have no plugins, so there are good chances that your setup will work with Storybook 3.4 and webpack 4 as is

Not an option, I have other configurations that require plug-ins

@Hypnosphi
Copy link
Member

Hypnosphi commented Sep 19, 2018

Is this a problem?

That means that we can render your components (created with React 15) using React DOM 16, which can become a problem in some cases

@Hypnosphi
Copy link
Member

I have other configurations that require plug-ins

those will require some duplication between webpack.config.js and .storybook/webpack.config.js

@Sujimoshi
Copy link
Author

Is this a problem?

That means that we can render your components (created with React 15) using React DOM 16, which can become a problem in some cases

What if pass specific version of ReactDOM as option to storybook

@Hypnosphi
Copy link
Member

I'd say that resolve.alias is such an option =)

@ndelangen
Copy link
Member

As long as we keep the react in the iframe compatible with react15 we're good on react15 compatibility, IF we make sure iframe and manager can have a separate versions of react.

I think it's natural for there to be 2 versions of react there, otherwise react-users are locked to use the same version of react as we are.

@Hypnosphi
Copy link
Member

IF we make sure iframe and manager can have a separate versions of react.

A good question is, how do we do that. Does webpack allow different require.resolve for different entry points? Or we need two separate webpack builds?

@Hypnosphi
Copy link
Member

Hypnosphi commented Sep 23, 2018

I think we can just prebuild manager and bundle our react 16 into dist

@Hypnosphi
Copy link
Member

Hypnosphi commented Sep 23, 2018

addons.js makes it a bit tricky but still possible. We just need to prebuild manager part for all the addons that have it as well

@Sujimoshi
Copy link
Author

Sujimoshi commented Sep 23, 2018

IF we make sure iframe and manager can have a separate versions of react.

A good question is, how do we do that. Does webpack allow different require.resolve for different entry points? Or we need two separate webpack builds?

As I know webpack doesn't have such option. So you need two configs

@Hypnosphi
Copy link
Member

And we probably can't use emotion in info addon unless we do #1501

@ndelangen
Copy link
Member

@Hypnosphi docs-view will solve this. I'd say we're not doing #1501 and we'll assist people migrating to docs-view instead. (which will not have the react in preview)

@ndelangen
Copy link
Member

As I know webpack doesn't have such option. So you need two configs

Working that that exactly, will not make it into 4.0.0

But we'll likely introduce this in some 4.x.y

@shilman
Copy link
Member

shilman commented Nov 22, 2018

Released a first version of compatibility in 4.1.0-alpha.4: https://github.com/storybooks/storybook/releases/tag/v4.1.0-alpha.4

Still needs more testing and documentation

@shilman
Copy link
Member

shilman commented Nov 24, 2018

cc @ranyitz @Sujimoshi @caitlin615 @arkadiusm @Buggytheclown @stereodenis

If you'd like to test out the alpha on your projects and let us know if it works for you, that would be really awesome! Just try out @storybook/react@next which should get you the latest alpha.

@ndelangen is there anything else they need to do or will it work out of the box?

@ndelangen
Copy link
Member

We changed so the preview and manager can use different versions.
With the last next release, using any version of react should just work.

@teksrc
Copy link

teksrc commented Nov 28, 2018

@ndelangen How soon will the last next release occur? I'm dealing with "react": "^15.6.1", and would like to setup Storybook. We may be able to migrate to version 16+, but not instantly. Thanks!

@shilman
Copy link
Member

shilman commented Nov 28, 2018

@Frankcarvajal it's released as alpha in @storybook/react@next. Please check it out! We expect to do a stable release in the next few weeks.

@shlomitc
Copy link

shlomitc commented Nov 28, 2018

@shilman - I've tried the new version on our project wix-style-react but building the storybook fails as Webpack fails to bundle the manager and hits undefined (no error checking on build-static.js).
Trying to log the webpack errors shows it couldn't resolve react from react-dom

You can check out the PR branch that was created https://github.com/wix/wix-style-react/pull/2494 and our discussion.

@ranyitz
Copy link
Contributor

ranyitz commented Nov 29, 2018

Thanks @shlomitc!

@shilman I've found the bug with the resolution of react/react-dom.

TL;DR The manager gets the user version of react and react-dom instead of the version specified in @storybook/react. The issue explained in details in this comment.

@ndelangen ndelangen added this to the next milestone Nov 30, 2018
@ndelangen
Copy link
Member

@shilman Do we close this, as it's fixed now in "next"?

@shilman
Copy link
Member

shilman commented Dec 2, 2018

Yep, closing!

@shilman shilman closed this as completed Dec 2, 2018
@issue-sh issue-sh bot removed the in progress label Dec 2, 2018
@shlomitc
Copy link

shlomitc commented Dec 7, 2018

I keep getting errors with the latest alpha build (v4.1.0-alpha.11) when trying to run build-stroybook

ERR! Module not found: Error: Can't resolve 'react/lib/ReactCurrentOwner' in '/Users/shlomitc/WebstormProjects/wsr-storybook-new/node_modules/react-dom/lib'
(node:73176) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): ModuleNotFoundError: Module not found: Error: Can't resolve 'react/lib/ReactCurrentOwner' in '/Users/shlomitc/WebstormProjects/wsr-storybook-new/node_modules/react-dom/lib'
(node:73176) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@shilman
Copy link
Member

shilman commented Jan 9, 2019

Ta-da!! I just released https://github.com/storybooks/storybook/releases/tag/v4.1.6 containing PR #5148 that references this issue. Upgrade today to try it out!

@alasdairhurst
Copy link

alasdairhurst commented Jan 17, 2019

I'm still getting unmet peer dependencies in 4.1.7 using react 15:

npm WARN @emotion/[email protected] requires a peer of react@>=16.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN @emotion/[email protected] requires a peer of react@>=16.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN @emotion/[email protected] requires a peer of react@>=16.3.0 but none is installed. You must install peer dependencies yourself.```

@ndelangen
Copy link
Member

Does storybook run when ignoring these warnings?

@alasdairhurst
Copy link

alasdairhurst commented Jan 19, 2019 via email

@ndelangen
Copy link
Member

V5 should resolve these peerDependencies warnings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants