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

Update react-native symlink resolving and add support for flow #3306

Merged
merged 5 commits into from
Mar 29, 2018

Conversation

meros
Copy link
Contributor

@meros meros commented Mar 28, 2018

Issue:

The symlink resolving for react-native does not work all the time. react-native tools moved over to a new way of resolving symlinks last year: facebook/react-native@39f83c4#diff-d939fe299a50129184bf6b4208f6d497

What I did

I moved over to the new symlink method according to the react-native PR

How to test

Run storybook on a project depending on external projects in a workspace monorepo. Make sure roots are all there (just like for react-native start) and storybook is the first root.

Is this testable with Jest or Chromatic screenshots?
No

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.

@codecov
Copy link

codecov bot commented Mar 28, 2018

Codecov Report

Merging #3306 into master will decrease coverage by 0.07%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3306      +/-   ##
==========================================
- Coverage   35.69%   35.62%   -0.08%     
==========================================
  Files         472      472              
  Lines       10104    10125      +21     
  Branches      968      965       -3     
==========================================
  Hits         3607     3607              
- Misses       5875     5901      +26     
+ Partials      622      617       -5
Impacted Files Coverage Δ
app/react-native/src/bin/storybook-start.js 0% <0%> (ø) ⬆️
app/react/src/server/utils.js 0% <0%> (-100%) ⬇️
addons/viewport/src/preview/withViewport.js 77.27% <0%> (ø) ⬆️
addons/a11y/src/components/Report/Elements.js 0% <0%> (ø) ⬆️
addons/jest/src/components/Result.js 0% <0%> (ø) ⬆️
addons/storyshots/src/angular/renderTree.js 61.9% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/routed_link.js 22.72% <0%> (ø) ⬆️
...res__/update-addon-info/update-addon-info.input.js 0% <0%> (ø) ⬆️
addons/knobs/src/angular/utils.js 82.14% <0%> (ø) ⬆️
.../src/modules/ui/components/stories_panel/header.js 28.57% <0%> (ø) ⬆️
... and 67 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 9edbb96...5cb3f93. Read the comment docs.

@danielduan danielduan added bug maintenance User-facing maintenance tasks react-native labels Mar 28, 2018
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.

looks reasonable. we're adding flow support as a default here too but that's fine by me.

@meros please fix the lint errors, I think you can ignore the global requires with a // eslint-disable-line global-require

@danielduan danielduan changed the title Update react-native symlink resolving Update react-native symlink resolving and add support for flow Mar 28, 2018
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.

looks reasonable. we're adding flow support as a default here too but that's fine by me.

@meros please fix the lint errors, I think you can ignore the global requires with a // eslint-disable-line global-require

@meros
Copy link
Contributor Author

meros commented Mar 28, 2018 via email

@Hypnosphi Hypnosphi merged commit 01a2fe6 into storybookjs:master Mar 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug maintenance User-facing maintenance tasks react-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants