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

(#1286) [WIP] Update package.json to use workspaces in react native examples #2375

Conversation

dangreenisrael
Copy link
Member

@dangreenisrael dangreenisrael commented Nov 25, 2017

Issue:#1286

Why this PR exists

Currently, our build process requires us to take some additional steps for React Native, which slows the development process. The goal here is to get React Native working with yarn workspaces.

Checklist

  • Implement Haul in react native vanilla
  • Implement Haul in crna
  • Update package.json in root directory
  • Update package.json in example apps

What I did

Updated package.json in examples/crna-kitchen-sink and examples/react-native-vanilla

How to test

Is this testable with jest or storyshots?
Yes (existing tests should cover it)

Does this need a new example in the kitchen sink apps?
No

Does this need an update to the documentation?
No

If your answer is yes to any of these, please make sure to include it in your PR.

Screenshots

@dangreenisrael dangreenisrael changed the title (#1286) Update package.json to use workspaces in react native examples (#1286) [WIP] Update package.json to use workspaces in react native examples Nov 25, 2017
@Hypnosphi
Copy link
Member

Hypnosphi commented Nov 25, 2017

Looks like it actually installs the published packages instead of linking, so it doesn't allow to test local code.

You should change those lines to just examples/* so that te RN example packages are actually included in workspaces:
https://github.com/storybooks/storybook/blob/master/package.json#L13-L14

Once things keep working after running yarn bootstrap --reset --core, we should remove the RN-related options from bootstrap command and do some adjustments in CI jobs config

@Hypnosphi
Copy link
Member

Hypnosphi commented Nov 25, 2017

I tried to do this locally and yarn storybook fails in crna-kitchen-sink:

Error: Cannot find module '/Users/jetbrains/IdeaProjects/storybook/examples/crna-kitchen-sink/node_modules/react-native/local-cli/cli.js'
    at Function.Module._resolveFilename (module.js:536:15)
    at Function.Module._load (module.js:466:25)
    at Function.Module.runMain (module.js:676:10)
    at startup (bootstrap_node.js:187:16)
    at bootstrap_node.js:608:3

That is because yarn hoists react-native package to top-level node_modules, and we need it to be present in package level node_modules for some reason

@Hypnosphi
Copy link
Member

We probably need to use something like require.resolve('react-native/local-cli/cli.js') here:
https://github.com/storybooks/storybook/blob/master/app/react-native/src/bin/storybook-start.js#L78

@Hypnosphi
Copy link
Member

Hypnosphi commented Nov 25, 2017

Btw, metro bundler (the default one in RN) still doesn't support symlinks facebook/metro#1

So if we want to move RN examples to workspaces we should switch them to haul bundler

@Hypnosphi Hypnosphi added cleanup Minor cleanup style change that won't show up in release changelog maintenance User-facing maintenance tasks labels Nov 26, 2017
@dangreenisrael
Copy link
Member Author

@Hypnosphi It looks like haul may be a great solution. I'm going to try it out with the react native vanilla app first, then looking to doing it with the crna example (if it is even possible without ejecting).

@Hypnosphi Hypnosphi changed the base branch from release/3.3 to master December 23, 2017 23:57
@dangreenisrael
Copy link
Member Author

Closing this, as I haven't made any progress

@Hypnosphi Hypnosphi deleted the (#1286)-Update-crna-kitchen-sink-to-use-workspaces branch February 17, 2018 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: do not merge cleanup Minor cleanup style change that won't show up in release changelog maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants