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

Fix react native example and bootstrapping #1514

Merged
merged 5 commits into from
Jul 25, 2017

Conversation

shilman
Copy link
Member

@shilman shilman commented Jul 24, 2017

Issue: #1509

What I did

Addresses issues raised in #1509:

  • bootstrapping react native doesn't work
  • error-logging channel breaks RN, so reverting this (cc @tmeasday)

How to test

yarn bootstrap && yarn bootstrap:react-native-vanillla
cd examples/react-native-vanilla
yarn storybook
./node_modules/.bin/react-native run-ios

@shilman shilman requested review from ndelangen and tmeasday July 24, 2017 22:03
@shilman shilman added the bug label Jul 24, 2017
@shilman shilman changed the title 1509 fix react native example Fix react native example and bootstrapping Jul 24, 2017
export class AddonStore {
constructor() {
this.loaders = {};
this.panels = {};
// this.channel should get overwritten by setChannel if package versions are
// correct and AddonStore is a proper singleton. If not, throw an error.
this.channel = { on: channelError, emit: channelError };
this.channel = null;
Copy link
Member

Choose a reason for hiding this comment

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

it was good whilst it lasted

Copy link
Member Author

Choose a reason for hiding this comment

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

It wasn't good! It wasted multiple hours of my time! 😭

@codecov
Copy link

codecov bot commented Jul 24, 2017

Codecov Report

Merging #1514 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1514      +/-   ##
==========================================
+ Coverage    14.6%   14.61%   +<.01%     
==========================================
  Files         202      202              
  Lines        4655     4653       -2     
  Branches      521      522       +1     
==========================================
  Hits          680      680              
+ Misses       3513     3509       -4     
- Partials      462      464       +2
Impacted Files Coverage Δ
lib/addons/src/index.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/api/configs/init_api.js 40.47% <0%> (ø) ⬆️
addons/knobs/src/components/types/Boolean.js 11.62% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/PropField.js 10.86% <0%> (ø) ⬆️
addons/storyshots/src/index.js 0% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-build.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/init_panels.js 25% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/down_panel.js 23.52% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/left_panel.js 20% <0%> (ø) ⬆️
... and 16 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 9b80a99...04a15a8. Read the comment docs.

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Not sure about the bootstrapping bit, but good with removing the check.

@tmeasday
Copy link
Member

An alternative approach: #1515

@ndelangen ndelangen merged commit 0ceac7d into master Jul 25, 2017
@Hypnosphi Hypnosphi deleted the 1509-fix-react-native-example branch August 17, 2017 22:28
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.

3 participants