Skip to content
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

[Feat] Deduplicate common code #2488

Open
Tristan-WorkGH opened this issue Feb 7, 2025 · 0 comments
Open

[Feat] Deduplicate common code #2488

Tristan-WorkGH opened this issue Feb 7, 2025 · 0 comments
Labels

Comments

@Tristan-WorkGH
Copy link

Tristan-WorkGH commented Feb 7, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant