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: Fix https://github.com/storybookjs/react-native/issues/120 #190

Conversation

lauriharpf
Copy link
Contributor

Issue: Fix #120 by changing knobs dependencies to ones that don't break Expo's "yarn web"

What I did

Changed dependencies of ondevice-knobs according to the suggested fix at #120 (comment) . react-native-modal-selector did not have a compatible version (there's an open PR from May, peacechen/react-native-modal-selector#162), so changed it to @react-native-picker/picker.

How to test

Smoke testing can be done with https://github.com/lauriharpf/react-native-storybook-expo-knobs-test , which was created based on instructions at #120 . Try this:

  1. Pull https://github.com/lauriharpf/react-native-storybook-expo-knobs-test
  2. npm install -g expo-cli
  3. Pull this branch
  4. yarn dev in the root of this branch
  5. Edit the reference to @storybook/addon-ondevice-knobs in package.json of https://github.com/lauriharpf/react-native-storybook-expo-knobs-test to point to your local ondevice-knobs (pulled and set up in previous two steps)
  6. In the root of https://github.com/lauriharpf/react-native-storybook-expo-knobs-test, run yarn install, followed by yarn start-storybook and then yarn web in another terminal
  7. Once Expo's web has loaded, open the "Knobs Example > with knobs" from the Storybook and use the two knobs (select & datepicker).
  8. Run the Expo project on iOS and Android, repeating the experimenting with "Knobs Example > with knobs"
  9. Compare the results to the ones with running next-6.0 branch instead of this branch.
  • Does this need a new example in examples/native? Yes, we should add examples of the knobs to next-6.0. Tired to do that, but couldn't get any addons to appear. Any help is appreciated!
  • Does this need an update to the documentation? No, not in my opinion

Known issues and limitations

Select issue on Android

For some reason the select values (apple, banana, cherry) are not visible on Android. What I see is this:
Screen Shot 2021-06-27 at 19 09 42

The iOS view looks fine and the values are selectable:
Screen Shot 2021-06-27 at 19 10 18

Datepicker does not work in Expo web

The datepicker does not open when clicking on it. Developer tools shows the following error: react-native-logs.fx.ts:22 DateTimePicker is not supported on: web

Expo issue on iOS

For some reason, the "Knobs Example > with knobs" story breaks for me in Expo on iOS with this error:

Error: React.Children.only expected to receive a single React element child.

This error is located at:
    in TouchableHighlight (at TouchableHighlight.js:383)
    in ForwardRef (at Button/index.js:6)
    in Button (at KnobsExample.stories.js:51)
    in RCTView (at View.js:34)
    in View (created by StoryView)
    in StoryView (created by OnDeviceUI)
    in RCTView (at View.js:34)
    in View (created by Context.Consumer)
    in emotion(View) (created by OnDeviceUI)
    in RCTView (at View.js:34)
    in View (at createAnimatedComponent.js:165)
    in AnimatedComponent (at createAnimatedComponent.js:215)
    in ForwardRef(AnimatedComponentWrapper) (created by OnDeviceUI)
    in RCTView (at View.js:34)
    in View (at createAnimatedComponent.js:165)
    in AnimatedComponent (at createAnimatedComponent.js:215)
    in ForwardRef(AnimatedComponentWrapper) (created by OnDeviceUI)
    in RCTView (at View.js:34)
    in View (created by AbsolutePositionedKeyboardAwareView)
    in RCTView (at View.js:34)
    in View (created by AbsolutePositionedKeyboardAwareView)
    in AbsolutePositionedKeyboardAwareView (created by OnDeviceUI)
    in RCTView (at View.js:34)
    in View (at KeyboardAvoidingView.js:204)
    in KeyboardAvoidingView (created by OnDeviceUI)
    in RCTSafeAreaView (at SafeAreaView.js:51)
    in ForwardRef(SafeAreaView) (created by OnDeviceUI)
    in OnDeviceUI (created by StorybookRoot)
    in ThemeProvider (created by StorybookRoot)
    in StorybookRoot (created by ExpoRoot)
    in ExpoRoot (at renderApplication.js:45)
    in RCTView (at View.js:34)
    in View (at AppContainer.js:106)
    in DevAppContainer (at AppContainer.js:121)
    in RCTView (at View.js:34)
    in View (at AppContainer.js:132)
    in AppContainer (at renderApplication.js:39)
at node_modules/react-native/Libraries/Core/ExceptionsManager.js:104:6 in reportException
at node_modules/react-native/Libraries/Core/ExceptionsManager.js:171:19 in handleException
at node_modules/react-native/Libraries/Core/setUpErrorHandling.js:24:6 in handleError
at node_modules/expo-error-recovery/build/ErrorRecovery.fx.js:12:21 in ErrorUtils.setGlobalHandler$argument_0
at node_modules/core-js/modules/es.promise.js:117:28 in microtask$argument_0
at node_modules/core-js/internals/microtask.js:27:10 in flush
at [native code]:null in flushedQueue
at [native code]:null in invokeCallbackAndReturnFlushedQueue

Screen Shot 2021-06-27 at 19 02 32

This seems to happen when I kill the iOS emulator and Expo, then restart the iOS emulator via Expo. Doing any edit (e.g. just changing comments and saving) to KnobsExample.stories.js fixes the problem, but it always reappears. Don't know what is going on and if that is related to the changes or not.

@dannyhw
Copy link
Member

dannyhw commented Jun 27, 2021

@lauriharpf hey its really great to see the results of your testing here. I've been planning to add web support to the example which would make this type of stuff easier test.

There is a bit of a complication with this PR though. In my controls PR #182 I replace knobs with controls, it will be very similar to knobs (most components are re-used). It might be better to work on these changes once that PR is complete. I'm hoping to get it finished up this week.

I'd say keep this PR and we can work on making the necessary changes to controls based on this.

The addons probably aren't working yet in the example of next-6.0 because I hadn't added the addon support back in after updating the repo. I've been working on the new way of adding them via config as part of the controls PR, if you like I can go through these changes with you at some point to give more context around it.

@lauriharpf
Copy link
Contributor Author

There is a bit of a complication with this PR though. In my controls PR #182 I replace knobs with controls, it will be very similar to knobs (most components are re-used). It might be better to work on these changes once that PR is complete. I'm hoping to get it finished up this week.

I'd say keep this PR and we can work on making the necessary changes to controls based on this.

That sounds like a plan! 👍

Things are in a bit of a flux at the moment with the 6.0 work underway, just felt like probing this ticket a bit 🙂 . Sounds reasonable to leave this open for now, get #182 merged first, then check where we are & what is needed.

The addons probably aren't working yet in the example of next-6.0 because I hadn't added the addon support back in after updating the repo. I've been working on the new way of adding them via config as part of the controls PR, if you like I can go through these changes with you at some point to give more context around it.

Thanks, happy to learn more! I have a lot of free time next week (28th of June - 1st of July), so if you have the time for it then it would be interesting to get a quick deep-dive on what's up e.g. via a Google Meet or something. I have to admit that I haven't read through all of #182 😱 😅 .

@dannyhw
Copy link
Member

dannyhw commented Jun 27, 2021

Well #182 is still very much in progress so it's kind of hard to understand with all the commented code etc 😅. I also had to redo some eslint stuff and some fixes to notes for some testing I was doing so there are really a lot of changes that aren't 100% related to controls.

Main issue I still need to resolve with controls is getting things to update/re-render correctly and then test each type of control then I'll start cleaning it up.

@lauriharpf lauriharpf force-pushed the fix/react-native-web-storybook branch from 673ced3 to 3beba7e Compare July 1, 2021 19:56
@peacechen
Copy link

@lauriharpf Fix published in react-native-modal-selector 2.0.4. Apologies for the delay.

@dannyhw
Copy link
Member

dannyhw commented Jul 6, 2021

@lauriharpf Fix published in react-native-modal-selector 2.0.4. Apologies for the delay.

Thanks @peacechen 🙏

@dannyhw dannyhw added the 6.5 label Jul 13, 2021
@lauriharpf lauriharpf closed this Aug 14, 2021
@lauriharpf lauriharpf deleted the fix/react-native-web-storybook branch August 14, 2021 12:16
@lauriharpf
Copy link
Contributor Author

The work we've done with 6.0 alpha has made this PR obsolete, so closed this PR and opened #251 instead to resolve #120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

react-native-web 0.12.0+ doesn't work with storybook due to deprecated components
3 participants