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

Upgrade React Native to webpack 2 config #1097

Merged

Conversation

matt-oakes
Copy link
Contributor

Issue: #1061 React Native is using the old webpack config format

What I did

I have upgraded the default webpack config, following the changes made in #1062 by @danielduan (see the diff here.

This is the first time I have used webpack directly, so my changes could be very wrong. Feel free to close this and do it a better way if it's more useful. This is, however, allowing Storybook to work correctly with my React Native project.

How to test

  1. Create a React Native project (react-native init StorybookTest).
  2. Run getstorybook.
  3. Run yarn run storybook.
  4. Run the app (react-native run-ios) and open http://localhost:7007/ to see the preview working.

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

newConfig.module.loaders = [
...newConfig.module.loaders,
{
test: /\.css?$/,
include: includePaths,
loaders: [
use: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! thanks for upgrading this to webpack 2 format!

@@ -3,16 +3,26 @@ import { includePaths } from '../utils';

// Add a default custom config which is similar to what React Create App does.
module.exports = storybookBaseConfig => {
const newConfig = storybookBaseConfig;
const newConfig = { ...storybookBaseConfig };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, can you try just exporting the app/react version of this file directly? It looks like the configs could probably be shared directly.

this is outside the scope of your PR but I see a lot of shared server files for react-native and react as well. we could combine them as much as possible to prevent fixing problems in one while forgetting about the other @ndelangen

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we at some point add a third UI library, this will be refactored.

@matt-oakes matt-oakes force-pushed the issue-1061-react-native-config branch from 7cca6e0 to 43a665c Compare May 22, 2017 21:26
@matt-oakes
Copy link
Contributor Author

I've updated the code to fix the lint issue that bitHound found (missing comma), but CodeFactor is going to fail due to duplicate code as @danielduan mention in his comment (#1097 (comment)).

@ndelangen
Copy link
Member

The code validation tools are here to help us, not to block us!

@ndelangen
Copy link
Member

@danielduan if you think is good to go, just merge!

@danielduan danielduan merged commit 346968d into storybookjs:master May 22, 2017
@matt-oakes matt-oakes deleted the issue-1061-react-native-config branch May 22, 2017 21:56
@matt-oakes
Copy link
Contributor Author

That was amazingly speedy! Thanks @ndelangen and @danielduan!

@shilman shilman added the misc label May 27, 2017
@ndelangen ndelangen changed the title Upgrade React Native webpack config Upgrade React Native to webpack 2 config May 27, 2017
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants