maplibre-react-native icon indicating copy to clipboard operation
maplibre-react-native copied to clipboard

Migrate MapView to react function components

Open WingmanImd opened this issue 1 year ago • 6 comments

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:fix in the root folder
  • [x] I have run tests via yarn test in the root folder
  • [x] I updated the documentation with running yarn generate in 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

WingmanImd avatar Feb 07 '24 19:02 WingmanImd

Awesome work 🎉

caspg avatar Apr 10 '24 10:04 caspg

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 avatar Apr 10 '24 11:04 tyrauber

@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 avatar Apr 10 '24 12:04 WingmanImd

@WingmanImd How is it going? Do you need extra pair of hands? If yes, I would happily help.

caspg avatar Apr 29 '24 07:04 caspg

@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.

tyrauber avatar May 13 '24 14:05 tyrauber

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 :)

WingmanImd avatar May 14 '24 19:05 WingmanImd

@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

WingmanImd avatar May 21 '24 19:05 WingmanImd

@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

WingmanImd avatar Jun 07 '24 07:06 WingmanImd

Totally understand, @WingmanImd. I really appreciate you taking the lead on this and doing what you can.

tyrauber avatar Jun 07 '24 07:06 tyrauber

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 avatar Jun 12 '24 17:06 tyrauber

@tyrauber yup, I'm up for that. I could start working on this next week.

caspg avatar Jun 13 '24 07:06 caspg

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?

tyrauber avatar Jun 16 '24 14:06 tyrauber

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

WingmanImd avatar Jun 17 '24 06:06 WingmanImd

@tyrauber @caspg I pushed my latest changes. Hope it helps!

WingmanImd avatar Jun 17 '24 17:06 WingmanImd