-
Notifications
You must be signed in to change notification settings - Fork 29
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
Rn 0.57 support #63
Rn 0.57 support #63
Conversation
@@ -6,18 +6,23 @@ | |||
"build": "fable-splitter -c splitter.config.js --define RELEASE", | |||
"start": "fable-splitter -c splitter.config.js -w --define DEBUG", | |||
"compile-for-test": "fable-splitter -c splitter.config.js --define TEST", | |||
"cold-start": "fable-splitter -c splitter.config.js --define DEBUG" | |||
"cold-start": "fable-splitter -c splitter.config.js --define DEBUG", | |||
"reset:packager": "watchman watch-del-all && node_modules/react-native/scripts/packager.sh --reset-cache" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this doing? and how to use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is useful to resolve some metro packager errors. For example, sometimes metro gives you red screen with similar instructions when it can't find files after renaming, moving to different folders or reinstalling dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build.cmd runtests
"appium": "1.7.2", | ||
"babel-core": "6.26.0", | ||
"fable-splitter": "0.1.21" | ||
"babel-core": "6.26.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/react-native-community/react-native-releases/blob/master/CHANGELOG.md#updating-to-this-version says babel-core": "7.0.0-bridge.0" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Babel dependencies are already listed in metro preset: https://github.com/facebook/metro/blob/master/packages/metro-react-native-babel-preset/package.json. Also v6.26.3 had worked for me on several projects with 0.57 so I didn't change this dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to update my app similarly to your changes here. And it looks good on device, but my canopy.mobile tests fail and it looks like toolbar images are missing on the emulator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to run these tests?
@@ -0,0 +1,3 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alfonsogarciacaro is fable using this? or should it go to the splitter config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not Fable itself, but Babel will detect and use it: https://babeljs.io/docs/en/next/options#babelrc
@@ -72,7 +72,7 @@ let update msg model : Model*Cmd<Msg> = | |||
Status = Changed }, Cmd.none | |||
|
|||
| Error e -> | |||
Toast.showShort e.Message | |||
Alert.alert ("Error", e.Message, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Toast doesn't work on iOS
sorry for not coming back to this earlier. Can you please take a look at #66 and apply the things that are still missing? That would help a lot! |
Hi @forki , sorry for late reply. Unfortunately I don't have much time currently to update this PR. |
Checked iOS & Android on fresh install.
fixes #62