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

Compatibility with react-scripts@1.0.1 (Create React App) #1073

Closed
gaearon opened this issue May 19, 2017 · 13 comments
Closed

Compatibility with react-scripts@1.0.1 (Create React App) #1073

gaearon opened this issue May 19, 2017 · 13 comments

Comments

@gaearon
Copy link

gaearon commented May 19, 2017

We recently released 1.0 of react-scripts:
https://facebook.github.io/react/blog/2017/05/18/whats-new-in-create-react-app.html

It includes many changes, and people have been reporting that it’s incompatible with React Storybook as it exists now: facebook/create-react-app#2225.

I imagine there's both some sort of dependency bugs in npm, and just regular incompatibilities (e.g. our Babel preset has updated and we need other tools to catch up with supporting dynamic import()).

For this reason, I have removed React Storybook integration from our docs: facebook/create-react-app#2252. I would be happy to add it back once it is compatible out of the box again. Thanks!

@ndelangen
Copy link
Member

ndelangen commented May 19, 2017

Congrats on the release of the 1.0.0.

We'll be taking a look at it asap, thanks for reporting!

We will create a PR when compatible, which would be the 3.0.0 release.

@ndelangen ndelangen added this to the v3.0.0 milestone May 19, 2017
@ndelangen ndelangen self-assigned this May 19, 2017
@gaearon
Copy link
Author

gaearon commented May 19, 2017

Thanks!

@ndelangen
Copy link
Member

ndelangen commented May 19, 2017

Took a look and concluded that Storybook 3.0.0-alpha.1 IS compatible with react-scripts 1.0.1!

Everything works great, nice work on react-script, it's looking better and better every release.

I don't want to wait too long re-adding storybook to create-react-app, but I'm conflicted on adding it whilst we're in alpha...

  💭
🤔

@ndelangen ndelangen changed the title Compatibility with react-scripts@1.0.0 (Create React App) Compatibility with react-scripts@1.0.1 (Create React App) May 19, 2017
@gaearon
Copy link
Author

gaearon commented May 19, 2017

Pretty cool. Does dynamic import work too?

@ndelangen
Copy link
Member

We use webpack 2, I don't see why not. Will check in the morning 😴

@gaearon
Copy link
Author

gaearon commented May 19, 2017

Ah, I forgot it's native there 😄

We also changed how we handle files: everything gets file-loader by default unless it's one of few whitelisted extensions which are handled by url-loader.

@ndelangen
Copy link
Member

We will have to keep a watch out for the 2 webpack configs acting in very different ways. But we don't have to mimic CRA's webpack config, consumers can change theirs and thus change it to be compatible with any edge-case.

I'm going to mark this issue as resolved, and set a reminder to create a PR when we're out of alpha.

@gaearon
Copy link
Author

gaearon commented May 20, 2017

Do consumers have to create config for CRA integration? I thought it used to work without a config.

@shilman
Copy link
Member

shilman commented May 20, 2017

@gaearon getstorybook works out of the box with zero config, but consumers can override the defaults if they want to.

@oriSomething
Copy link
Contributor

Might related to #1027

But, it doesn't compatible with react-scripts@^1 when it comes to CSS. If you have a CSS files that import other CSS files, it breaks with this error:

ERROR in ./~/css-loader?{"importLoaders":1}!./~/postcss-loader/lib?{"plugins":[null]}!./src/stories/blah.css
Module build failed: TypeError: Cannot read property 'postcss' of null

Code example:

/* a.css */
@import "./b.css";
/* b.css */

:root {
  /* ... */
}
// Story file
import "./a.css";

@shilman
Copy link
Member

shilman commented May 21, 2017

@oriSomething are you using the alpha? if so, would you mind opening another issue for this problem?

@ndelangen
Copy link
Member

ndelangen commented May 21, 2017

This has been fixed in alpha.3

Storybook is compatible with CRA without configuration out of the box, and we absolutely intend to keep it that way. Sorry if my comment before kinda suggested otherwise @gaearon

@oriSomething
Copy link
Contributor

I've checked, and the bug is being fixed

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

No branches or pull requests

5 participants