maplibre-react-native
maplibre-react-native copied to clipboard
Migrate MapView to react function components
Description
Migrated the MapView component to use function components. Also migrated the functionality of the NativeBridgeComponent to a hook(left other components using the old component untouched for now)
I had to add the following rules in the eslintrc as the current config is to eager on errors and some required rules might result in breaking changes.
'react-hooks/rules-of-hooks': 'error', 'react-hooks/exhaustive-deps': 'warn',
The purpose of this series of PRs is not to make too many changes/fixes to the current codebase just convert the components to function components.
Fixes #50
Checklist
- [x] I have tested this on a device/simulator for each compatible OS
- [x] I formatted JS and TS files with running
yarn lint:fixin the root folder - [x] I have run tests via
yarn testin the root folder - [x] I updated the documentation with running
yarn generatein the root folder - [ ] I mentioned this change in
CHANGELOG.md - [ ] I updated the typings files (
index.d.ts) - [ ] I added/updated a sample (
/example)
Screenshot OR Video
Awesome work 🎉
Yeah, this is really great work, @WingmanImd. What are your thoughts on applying this strategy to the rest of the javascript classes, not just MapView? It would be great to get the javascript modernized before trying to tackle the Maplibre Library updates. Is that something you'd be interested in undertaking?
@tyrauber I have already converted(in the sense that the logic is kept exactly the same but need to be optimized for a function component) the biggest work items "MapView", "Camera", "ShapeSource" and "Style" just didn't push the changes yet. I will do that after I also fix things like the ones @caspg mentioned. The rest of the components will come soon after as they seems to have a lot less logic in them
@WingmanImd How is it going? Do you need extra pair of hands? If yes, I would happily help.
@WingmanImd, Thanks to @sjchristi, we've migrated to the latest Maplibre Native library for iOS and migrated a lot of the class names and import statements from Mapbox to MapLibre. Would you mind rebasing your branch off main and pushing up your additional work? Love to get this merged! Thanks for taking the lead on this.
Hey @tyrauber @caspg I just need a few more days to finish with a few small components and then have another look at the changes to be a little sure that I don't break stuff :)
@tyrauber Late update but an update anyway :) All that remains is to fix the tests as a few of them are broken atm and I'll update the PR. @caspg we'll need your extra pair of eyes to look for possible bugs and/or improvements after that
@tyrauber I can't seem to find time to work on this so I will just disable the tests so I can push my changes into my fork and someone else can pick up from there. Will close the PR after I do that
Totally understand, @WingmanImd. I really appreciate you taking the lead on this and doing what you can.
hey @caspg, do you have any free cycles? Would you be interested in helping me finish up this migration? I'd like to include it in the 10.0.0 release.
@tyrauber yup, I'm up for that. I could start working on this next week.
Hey @WingmanImd, do you have any more commits you want to push up? You had mentioned back in April that you had already converted "MapView", "Camera", "ShapeSource" and "Style". Are those changes pushed? What were the next steps? Any suggestion as to where we should pick up from?
Hey @tyrauber I'll push my changes today. Pretty much all components are done but the tests need to be revised/rewriten, so I would start there. I will be disabling the tests on my fork as I can't push without them running
@tyrauber @caspg I pushed my latest changes. Hope it helps!