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

Revise the sample in README for ReactNative #4780

Merged
merged 2 commits into from
Nov 23, 2018
Merged

Conversation

niku
Copy link
Contributor

@niku niku commented Nov 13, 2018

With this change, the sample works without any errors.

What I did

$ cat package.json
{
  "name": "empty-project-template",
  "main": "node_modules/expo/AppEntry.js",
  "private": true,
  "scripts": {
    "start": "expo start",
    "android": "expo start --android",
    "ios": "expo start --ios",
    "eject": "expo eject"
  },
  "dependencies": {
    "expo": "^31.0.2",
    "react": "16.5.0",
    "react-native": "https://github.com/expo/react-native/archive/sdk-31.0.0.tar.gz"
  },
  "devDependencies": {
    "@storybook/react-native": "^4.0.4",
    "babel-preset-expo": "^5.0.0"
  }
}

Before change the code of entry point(App.js), I ran into follwing error:

[11:44:21] Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: %s.%s%s, object,  You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Check your code at registerRootComponent.js:17.,
    in ExpoRootComponent (at renderApplication.js:34)
    in RCTView (at View.js:44)
    in RCTView (at View.js:44)
    in AppContainer (at renderApplication.js:33)
- node_modules/react/cjs/react.development.js:217:39 in warningWithoutStack
- ... 22 more stack frames from framework internals

After change it, I ran into following error:

[11:46:38] ReferenceError: Can't find variable: React

This error is located at:
    in StoryView (created by OnDeviceUI)
    in RCTView (at View.js:44)
    in AnimatedComponent (at TouchableOpacity.js:256)
    in TouchableOpacity (created by OnDeviceUI)
    in RCTView (at View.js:44)
    in AnimatedComponent (created by OnDeviceUI)
    in RCTView (at View.js:44)
    in AnimatedComponent (created by OnDeviceUI)
    in RCTView (at View.js:44)
    in RCTView (at View.js:44)
    in AbsolutePositionedKeyboardAwareView (created by OnDeviceUI)
    in RCTSafeAreaView (at SafeAreaView.ios.js:35)
    in SafeAreaView (created by OnDeviceUI)
    in OnDeviceUI (created by StorybookRoot)
    in StorybookRoot (at registerRootComponent.js:17)
    in RootErrorBoundary (at registerRootComponent.js:16)
    in ExpoRootComponent (at renderApplication.js:34)
    in RCTView (at View.js:44)
    in RCTView (at View.js:44)
    in AppContainer (at renderApplication.js:33)
* storybook/stories/index.js:20:4 in <unknown>
- node_modules/@storybook/core/dist/client/preview/client_api.js:58:22 in withSubscriptionTracking
- node_modules/@storybook/react-native/dist/preview/components/StoryView/index.js:122:16 in render
- ... 19 more stack frames from framework internals

Then I have added the line to storybook/stories/index.js, it works without errors.

EDIT: I add a commit describe below:

I also ran into a warning that described at #4680, So fixing it same way.

[13:06:17] Missing 'module' parameter for story with a kind of 'CenteredView'. It will break your HMR
- node_modules/expo/build/environment/logging.js:25:23 in warn
- node_modules/@storybook/core/dist/client/preview/client_api.js:93:32 in <unknown>
* storybook/stories/index.js:18:10 in <unknown>
- node_modules/metro/src/lib/polyfills/require.js:292:12 in loadModuleImplementation
* storybook/index.js:8:9 in <unknown>
- node_modules/@storybook/react-native/dist/preview/index.js:91:17 in configure
* storybook/index.js:7:10 in <unknown>
- node_modules/metro/src/lib/polyfills/require.js:292:12 in loadModuleImplementation
* null:null in <unknown>
- node_modules/metro/src/lib/polyfills/require.js:292:12 in loadModuleImplementation
- node_modules/expo/AppEntry.js:2:0 in <unknown>
- node_modules/metro/src/lib/polyfills/require.js:292:12 in loadModuleImplementation
- node_modules/metro/src/lib/polyfills/require.js:179:45 in guardedLoadModule
* null:null in global code

With this change, the sample works without any errors.
With this change, the sample works without any warnings.
@codecov
Copy link

codecov bot commented Nov 13, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4780   +/-   ##
=======================================
  Coverage   35.48%   35.48%           
=======================================
  Files         557      557           
  Lines        6747     6747           
  Branches      892      892           
=======================================
  Hits         2394     2394           
  Misses       3887     3887           
  Partials      466      466

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 3c1a3d2...c8898a1. Read the comment docs.

@storybook-safe-bot
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS

@Gongreg
Copy link
Member

Gongreg commented Nov 23, 2018

Hey, thanks for the PR. I think somebody has fixed this already before, but it is missing. Let's hope it won't disappear anymore.

@Gongreg Gongreg merged commit 086e4a9 into storybookjs:master Nov 23, 2018
@niku niku deleted the patch-1 branch November 24, 2018 22:25
@shilman shilman added documentation patch:yes Bugfix & documentation PR that need to be picked to main branch labels Nov 25, 2018
@shilman
Copy link
Member

shilman commented Nov 25, 2018

@Gongreg please label PRs when you merge -- we have danger set up to block the merge for a reason 😘

@Gongreg
Copy link
Member

Gongreg commented Nov 25, 2018

Oh, saw you commented on this and assumed it has everything already. My bad, my bad

@igor-dv
Copy link
Member

igor-dv commented Nov 25, 2018

It is merged to master btw...

@shilman shilman added patch:done Patch/release PRs already cherry-picked to main/release branch and removed patch:done Patch/release PRs already cherry-picked to main/release branch labels Nov 25, 2018
@shilman
Copy link
Member

shilman commented Nov 25, 2018

@igor-dv @Gongreg yeah we should merge to next in general. In this case, I just cherry-picked it in the opposite direction (from master to next), so no need to do anything more, but for future merges... 😀

shilman pushed a commit that referenced this pull request Nov 26, 2018
Revise the sample in README for ReactNative
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants