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

Add index.js file for RN 0.49 #2176

Merged
merged 2 commits into from
Oct 29, 2017
Merged

Add index.js file for RN 0.49 #2176

merged 2 commits into from
Oct 29, 2017

Conversation

alj1s
Copy link
Contributor

@alj1s alj1s commented Oct 29, 2017

Issue:
Since RN 0.49, projects now have a single index.js entry point rather than the platform-specific index.android.js and index.ios.js.

What I did

I have added an index.js file to serve as the entry point for RN 0.49 apps. As there is an existing PR for RN 0.49 ( #1971 ) to update the dependencies, I have only made the change to add the file to the generator templates.

How to test

Link the cli project and run getstorybook.

@alj1s
Copy link
Contributor Author

alj1s commented Oct 29, 2017

I felt this was the simplest solution rather than cater for different versions of RN in the generator script. However, I'd be happy to consider alternatives if you prefer.

@codecov
Copy link

codecov bot commented Oct 29, 2017

Codecov Report

Merging #2176 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2176   +/-   ##
======================================
  Coverage    21.4%   21.4%           
======================================
  Files         263     263           
  Lines        5812    5812           
  Branches      696     706   +10     
======================================
  Hits         1244    1244           
+ Misses       4033    3996   -37     
- Partials      535     572   +37
Impacted Files Coverage Δ
.../ui/src/modules/ui/components/layout/dimensions.js 15.62% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/left_panel.js 25.71% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_routing.js 28.04% <0%> (ø) ⬆️
...ponents/left_panel/stories_tree/tree_decorators.js 33.33% <0%> (ø) ⬆️
addons/knobs/src/components/types/Color.js 8.1% <0%> (ø) ⬆️
addons/info/src/components/Props.js 36.36% <0%> (ø) ⬆️
.../src/manager/containers/CommentsPanel/dataStore.js 34.97% <0%> (ø) ⬆️
lib/ui/src/modules/ui/libs/filters.js 47.36% <0%> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 6.25% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 30.98% <0%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec07bb0...9c368a4. Read the comment docs.

@danielduan
Copy link
Member

@ximenean thanks for this!

I'm not sure how to proceed with this PR. I think merging this to master directly would help newcomers. This doesn't benefit an existing user who upgraded to 0.49 so the user has to be knowledgeable enough to figure out to add that index file.

Maybe that's not our responsibility if they upgraded their own dependencies.

@alj1s
Copy link
Contributor Author

alj1s commented Oct 29, 2017

@danielduan - I think that just adding something to the docs for RN 0.49 would suffice. I'm not sure I like any alternatives I can think of to automatically handle the upgrade from the storybook side.

If you think there's a good solution to this, I'm happy to have a go at it.

@danielduan danielduan merged commit cbfe5db into storybookjs:master Oct 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants