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

Integrate crna-kitchen-sink examples app into the monorepo #6125

Merged
merged 7 commits into from
Mar 17, 2019

Conversation

benoitdion
Copy link
Member

@benoitdion benoitdion commented Mar 16, 2019

Issue

Iterating on storybook react-native is currently very painful and hard to test. The first step to improve the situation is tighter integration of the example app into the monorepo.

What I did

How to test

  • yarn bootstrap --reset
  • yarn install
  • yarn dev
  • in example-native/crna-kitchen-sink: yarn start
  • launch the app on android or ios
  • make a change in example-native/crna-kitchen-sink and watch it reload
  • make a change in app/react-native or anywhere in the monorepo and watch it reload

Add a metro config file to ignore undesired packages from the metro bundle. Also avoid hoisting react-native.
@benoitdion benoitdion added the maintenance User-facing maintenance tasks label Mar 16, 2019
@codecov
Copy link

codecov bot commented Mar 16, 2019

Codecov Report

Merging #6125 into next will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #6125      +/-   ##
==========================================
- Coverage   38.07%   38.05%   -0.02%     
==========================================
  Files         643      643              
  Lines        9414     9418       +4     
  Branches     1370     1343      -27     
==========================================
  Hits         3584     3584              
- Misses       5265     5267       +2     
- Partials      565      567       +2
Impacted Files Coverage Δ
lib/core/src/server/build-static.js 0% <ø> (ø) ⬆️
app/react-native/src/preview/index.js 0% <0%> (ø) ⬆️
addons/ondevice-knobs/src/panel.js 0% <0%> (ø) ⬆️
addons/knobs/src/index.js 0% <0%> (ø) ⬆️

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 b7f0e23...0d622bb. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 16, 2019

Codecov Report

Merging #6125 into next will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #6125      +/-   ##
==========================================
- Coverage   38.07%   38.06%   -0.01%     
==========================================
  Files         643      643              
  Lines        9414     9415       +1     
  Branches     1370     1371       +1     
==========================================
  Hits         3584     3584              
  Misses       5265     5265              
- Partials      565      566       +1
Impacted Files Coverage Δ
app/react-native/src/preview/index.js 0% <0%> (ø) ⬆️
addons/ondevice-knobs/src/panel.js 0% <0%> (ø) ⬆️
addons/knobs/src/index.js 0% <0%> (ø) ⬆️

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 b7f0e23...951527c. Read the comment docs.

@shilman
Copy link
Member

shilman commented Mar 17, 2019

@benoitdion This version is working much better!! 🎉

I'm able to:

  • add/remove/edit stories and watch it reload
  • edit @app/react-native and watch it reload
  • select stories from the web UI and watch it update on-device
  • use the on-device UI when the server is not running

However, all hell breaks loose as soon as I open up the on-device nav panel when the device is connected to the server, and I'm back in an infinite loop.

@benoitdion
Copy link
Member Author

@shilman I haven't seen the infinite loop with the latest commit. What steps do you use to repro?

@shilman
Copy link
Member

shilman commented Mar 17, 2019

@benoitdion it looks like the knobs story is causing the problem maybe?

yarn bootstrap --reset --core
cd examples-native/crna-kitchen-sink
yarn storybook
yarn start

Clicking around on the non-knobs story in both UIs seems to be OK, though I didn't stress test. But click on the knobs story on device, and the loop gets triggered

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

@benoitdion I think there are still some issues between the client and server as we've discussed on Discord. But as far as integrating crna-kitchen-sink back into the mono-repo this looks EXCELLENT to me.

Out of curiosity, I ran a short benchmark to see how much overhead it adds to a yarn bootstrap --reset --core vs. next.

Build without CRNA

real	5m54.226s
user	5m10.626s
sys	2m46.306s

Build with CRNA

real	6m31.911s
user	5m30.995s
sys	3m14.872s

Well worth it IMHO. Bravo! 👏

@benoitdion
Copy link
Member Author

the server rendering should be fixed with 0d622bb

@benoitdion benoitdion merged commit d162862 into next Mar 17, 2019
@benoitdion benoitdion deleted the react-native/integrate-monorepo branch March 17, 2019 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks react-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants