withMap HoC removes prop typings from components

[ghost]

NPM package: 4.0.0

The withMap HoC from react-mapbox-gl/src/context is removing the prop typings from the components that it wraps.

To see the problem, import Cluster import { Cluster } from "react-mapbox-gl"; (or any other component wrapped with withMap), and inspect it's type. You'll see something along these lines: const Cluster: <T>(props: T) => JSX.Element

T is the unconstrained type of the props being passed in. Cluster has a required prop of ClusterMarkerFactory, but with the lack of typings it'll give no error no matter what is or isn't passed.

If instead the unwrapped version of Cluster is directly imported: import { Cluster } from "react-mapbox-gl/src/cluster";, then you'll find that you get an error about the missing required prop: Property 'ClusterMarkerFactory' is missing in type '{}'.

So how can this be fixed? By improving the typings on withMap:

type Omit<T, K> = Pick<T, Exclude<keyof T, keyof K>>;
interface HasMapProp { map: MapboxGl.Map | undefined; }

// Typing - Take in a component which must have a "map" prop.
export const withMap = <TProps extends HasMapProp>(Component: React.ComponentType<TProps>):
// Typing - Return a component function (Stateless Functional Component) which has the same props, except without "map".
React.SFC<Omit<TProps, HasMapProp>> =>
// The actual code performing the injection
    (props) => {
        return (
            <MapContext.Consumer>
                {map => <Component map={map} {...props} />}
            </MapContext.Consumer>
        );
    };

This improves the typings in two ways, first it means withMap can only be applied to components which can take a map prop, and second it means the wrapped component (such as Cluster) is returned with the same props that it was passed in with, except for map, which has been injected by the HoC.

Giving it a quick test with const WrappedCluster = withMap(Cluster); <WrappedCluster /> gives us the correct error Property 'ClusterMarkerFactory' is missing in type '{}'., and from the prop list in the typings we can see map has been removed: const WrappedCluster: React.StatelessComponent<Pick<Props, "children" | "ClusterMarkerFactory" | "radius" | "maxZoom" | "minZoom" | "extent" | "nodeSize" | "log" | "zoomOnClick" | "zoomOnClickPadding" | "style" | "className" | "tabIndex">>

I wanted to submit this as a PR, but I had trouble getting the project to build.
I had a look around but couldn't find any info on the tooling that you use.
I tried to use VSCode on Win10, and that allowed me to experiment a bit, but the Demos especially had build errors (e.g. TS2307: Cannot find module '../../../'.) and I didn't feel confident pushing code that I couldn't fully test and show was working.

This post is a result of going through context.tsx due to this issue so if you do fix these typings, it'd be great if you could also action that issue, exposing MapContext and withMap!

This fixes the missing prop types, but my original issue was how to make a custom control that works like ZoomControl and the others.

If I can get some guidance on setting up a development environment then I'd be more than happy to submit this as a PR, with all the due diligence required.

[mklopets]

Hi, thanks for the detailed issue! I'll try and take care of this soon. For future reference, the issue you were seeing with examples is that they depend on the main package being built locally (so npm run-script build in the repo root).

[mklopets]

Ran into some type issues, where I can't guarantee that the argument of withMap indeed supports Map | undefined as the map prop's type. Will try and look more into it tomorrow, but this is a bit nasty, giving a false sense of security while in fact runtime type errors can arise.