[Feat] Deduplicate common code
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 8.5k
- Forks
- 1.4k
- Avg merge
- 5d 17h
- Merged PRs (30d)
- 3
Description
Target Use Case
I'm working on a library for a project that show a Mapbox or MapLibre map depending of a parameter. Since the update to mapbox-gl>=3.5.0 and react-map-gl 8.x, the duplication of functions and components pose us problems.
Here an example of what we're forced to do:
import { MapboxOverlay, type MapboxOverlayProps } from '@deck.gl/mapbox';
import {
Map as MapGL,
type MapProps as MapGlProps,
type MapRef as MapGlRef,
NavigationControl as NavigationControlGl,
useControl as useControlGl,
} from 'react-map-gl/mapbox';
import {
Map as MapLibre,
type MapProps as MapLibreProps,
type MapRef as MapLibreRef,
NavigationControl as NavigationControlLibre,
useControl as useControlLibre,
} from 'react-map-gl/maplibre';
// ...
type DeckGLOverlayProps = MapboxOverlayProps & { typeMap: TypeMap };
const DeckGLOverlay = forwardRef<MapboxOverlay, DeckGLOverlayProps>(({ typeMap, ...props }, ref) => {
let overlay: MapboxOverlay;
/* We can ignore ESlint rule react-hooks/rules-of-hooks here because useControl implementation is the same in the two cases
* https://github.com/visgl/react-map-gl/blob/8.0-release/modules/react-mapbox/src/components/use-control.ts
* https://github.com/visgl/react-map-gl/blob/8.0-release/modules/react-maplibre/src/components/use-control.ts
*/
if (typeMap === TypeMap.GL) {
overlay = useControlGl(() => new MapboxOverlay(props)); // eslint-disable-line react-hooks/rules-of-hooks
} else {
// @ts-expect-error: MapboxOverlay extends mapbox-gl.IControl while useControl ask for maplibre-gl.IControl ...
overlay = useControlLibre(() => new MapboxOverlay(props)); // eslint-disable-line react-hooks/rules-of-hooks
}
overlay.setProps(props);
return null;
});
// ...
return (
<Map>
<DeckGLOverlay typeMap={props.mapLibrary === MAPBOX ? TypeMap.GL : TypeMap.Libre} /*...*/ />
{props.mapLibrary === MAPBOX ? <NavigationControlGl /> : <NavigationControlLibre />}
{/*...*/}
</Map>
);
}
Proposal
After looking quickly in the sources, it seems possible for some functions and components to be moved in an internal module "common".
It is possible to de-duplicate some code like useControl and NavigationControl and move them in an internal module "common".
Then "react-mapbox" and "react-maplibre" modules would have as dependency this common module as a result.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing modules/react-mapbox/src/components/use-control.ts and modules/react-maplibre/src/components/use-control.ts, then inspect the corresponding NavigationControl implementations. Determine which shared functions and components can move into an internal common module without changing either public module's behavior; done means both react-mapbox and react-maplibre consume the shared code without duplication.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100