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

viewState `padding` expected to be optional

Open chrisgervang opened this issue 2 years ago • 4 comments

Currently I'm experiencing a Property 'padding' is missing in type ... but required in type 'ViewState'. error. I'd expect this type to be optional based on this implementation: https://github.com/visgl/react-map-gl/blob/109c334bdffaa5b2016c50bf6aaa119ce56082ce/src/mapbox/mapbox.ts#L544-L546

I believe the issue is in this type definition, which should define padding as optional.

https://github.com/visgl/react-map-gl/blob/109c334bdffaa5b2016c50bf6aaa119ce56082ce/src/types/external.ts#L30

I can open a PR to fix this if it should be optional.

chrisgervang avatar Mar 02 '23 20:03 chrisgervang

What is the use case? You are not supposed to supply viewState manually.

Pessimistress avatar Mar 02 '23 22:03 Pessimistress

I'm making a component that wraps Map and passes along any map props, so I thought to type this prop with MapProps like this:

import Map, {MapProps} from 'react-map-gl';

function Wrapper({mapProps}: {mapProps: MapProps}) {
  return (
    <Map
      {...mapProps}
    />
  )
}

I didn't realize that wasn't the intended usage since viewState is on the MapProps type, so TS doesn't hint this isn't the way to do it. I just looked and saw the examples spread viewState on as individual props, and I see that in the type definition as well. I'll do that.

Should viewState be removed from the MapProps type? i.e. MapProps = Omit<MapboxProps, 'viewState'> ...

References:

https://github.com/visgl/react-map-gl/blob/109c334bdffaa5b2016c50bf6aaa119ce56082ce/src/components/map.tsx#L27

https://github.com/visgl/react-map-gl/blob/109c334bdffaa5b2016c50bf6aaa119ce56082ce/src/mapbox/mapbox.ts#L246

chrisgervang avatar Mar 02 '23 23:03 chrisgervang

Yeah that's probably a good idea. I don't think deck.gl depends on react-map-gl types so it should be ok there.

Pessimistress avatar Mar 02 '23 23:03 Pessimistress

Do you know where this external controller use case is in use? I thought this was removed in v7 so that the map could only control deck, rather than the other way around. https://github.com/visgl/react-map-gl/blob/109c334bdffaa5b2016c50bf6aaa119ce56082ce/src/mapbox/mapbox.ts#L245-L249

chrisgervang avatar Mar 06 '23 20:03 chrisgervang