react-map-gl icon indicating copy to clipboard operation
react-map-gl copied to clipboard

[Feat] Deduplicate common code

Open Tristan-WorkGH opened this issue 1 year ago • 1 comments

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".
Image

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.

Tristan-WorkGH avatar Feb 07 '25 13:02 Tristan-WorkGH

The code base was split so that we no longer have to maintain one copy of code that works with both libraries. They are expected to diverge more and more.

Pessimistress avatar Mar 28 '25 23:03 Pessimistress