-
Notifications
You must be signed in to change notification settings - Fork 5
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
react-native 0.61.1 upgrade #98
Conversation
I had a few false starts getting this set up but eventually got a decent chunk of it done. update dependenciesI realized when setting this up that my computer had upgraded itself to latest xcode since last doing observe work. I'm currently on xcode 11.0. At some point apparently a long time ago people moved from using react-native-cli to @react-native-community/cli but react-native-cli is still referred to in react-native docs. I upgraded:
Latest react-native is 0.61.1 so that's the version react-native-cli used to start the project. I'm using the ios 12.2 iphone X simulator. What I've done so farI renamed and checked in the new android-rncli-init ios-rncli-init directories so we can finish comparing them to the older ios/android directories. Went throught the root level files and the config files in the android and ios directories and merged them as best I could, including updating the pods for ios. Next steps
I can work on this some more tomorrow. |
…e and a lot of things
Thank you for getting this started @sethvincent. Goal is to get Android to build. So I just went through these steps:
Android now builds, but the app crashes. I've not inspected the logcat yet, so that'd be next. And ios doesn't build at all. |
With the latest commit I'm able to run the android build in a simulator. Mostly this came down to futzing with dependencies and slightly revising the java files in the app to have the mapbox access token / http request fixes we did in our previous upgrade attempt. I've commented out the mapboxgl code in the app for now to better separate mapboxgl issues from general app issues. |
@sethvincent confirmed that Android builds. I had to add a cleartext thing in the manifest to enable emulator to fetch the bundle from metro 6579546#diff-020edf1fd6d483b0e2c8f5a5734ea1f2R15 I worked on getting the build to work on ios and now it does. It took me several hours of learning react native links, unlinking and cleaning up old configuration, updating several modules, and then some minor hacks in the Podfile. This now builds and is comparable to Android. I think next step is to remove the init folders and start plugging in newer Mapbox GL code. |
When I pulled latest earlier today I got this error:
To fix this I:
To get the app to boot I also removed some old ios config that seems to no longer be necessary with recent react-native-config versions (though we should double check this) b8040e2#diff-1489ef5d471d1e9ceaaab302212f10e8L2301-R2319 I deleted the global version of @react-native-community/cli on my computer to only use the locally installed version via npm scripts to test that out. Seems fine. The metro bundler is no longer starting automatically if I run But a very basic map now loads on ios and android! From here we need to do refactoring/rewriting of the old map and verify that all functionality still works. @geohacker I noticed we might be using different cocoapods versions. Not sure how much that matters. |
I can reproduce this behaviour 🤷♂ strange. |
I managed to get the symbol layers and icons to work, but the roads layer doesn't seem to. I think something wrong with the way we're using the expression to match highway tag to line color https://github.com/developmentseed/observe/blob/react-native-0.61.1-upgrade/app/style/map.js#L42 Will keep looking, but I think I've run out of ideas. |
Spent a while trying to get a handle on linelayers, styles, and specifically the roads layer. I tried a lot of things and it basically comes down to not finding a way to do a If I just set For a bit I got distracted and wondered if there was a bug in the filters and then I couldn't read the filters very well so I pulled them out into an object and then I realized that no, it was definitely the style that was the problem. Also:
|
With latest changes I'm seeing this error when saving an attribute edit to a point using ios: Going to the contributions screen it looks like the changeset successfully uploaded, so this is a rendering error. And that makes sense given the error message. On android I see this error after saving an attribute edit: After that the app got in a state where I couldn't navigate around without closing the app on the emulator and restarting it. But it looks like the changeset still went through, just like ios. |
iosI was able to download offline tiles/data successfully. When testing offline editing and turning the network back on the app did not detect the network right away. Sidenote: I noticed that the pending/offline icon looks too similar to the completed icon in the contributions screen. I logged out and logged in a few times. Seems to work ok. After testing offline and bringing the network connection back I tried logging in and got Some interesting logs:
The metro bundler now outputs useful logs from ios and android, so that's nice. androidlogging in and out worked ok. There was once where a weird flash of an error notification showed after being redirected back from chrome, but then it rendered the profile correctly. It had logged this error: trying to download offline tiles on android eventually started timing out:
This was for a very small area. Roughly a city block. Tried again and got the same response. It says the tiles got to 50%. |
I've managed to add all the map layers back including edited geojson states. This should now handle all cases for pois, lines and polygons. @sethvincent the error you ran into while editing features should be gone now. |
I can reproduce the failed offline pack messages on Android, but I'm on a really spotty network. The downloads I tried all got to 100%. So I'm assuming it retries and then manages to download ok. I'm not entirely sure and we should inspect this. |
Since we managed to get the app building on both platforms and replicate pretty much all the functionality, we should merge to develop. |
…eed/observe into react-native-0.61.1-upgrade
…eed/observe into react-native-0.61.1-upgrade
Work in progress react-native upgrade pr.