danmarshall/deckgl-typings

Trouble with MapView JSX Type Declarations

au-z opened this issue ยท 6 comments

au-z commented

Thanks very much for providing these typings! I've run into a minor problem with the MapView type declaration and figured you may have a pointer or two. I am not so skilled with type declarations but it appears that the MapView class in deck.gl__core may be missing support to be composed as a child component of .

Here's a very simplified version of my component:

import DeckGL from '@deck.gl/react'
import {MapView} from '@deck.gl/core'

export default function Component() {
    return <DeckGL>
        <MapView>
             ...
        </MapView>
    </DeckGL>
}

Here's the accompanying error:

JSX element class does not support attributes because it does not have a 'props' property.ts(2607)
'MapView' cannot be used as a JSX component.
  Its instance type 'MapView' is not a valid JSX element.
    Type 'MapView' is missing the following properties from type 'ElementClass': render, context, setState, forceUpdate, and 3 more.

I believe this syntax (MapView) as a JSX child of should be supported as evidenced in the deck.gl docs here:
https://deck.gl/docs/get-started/using-with-react#using-jsx-with-deckgl-views

Any suggestions?

Hi @auzmartist , thank you for posting this issue. There definitely seems to be something awry when it comes to using the MapView this way. Notice that the DeckGL is declared this way:
https://github.com/danmarshall/deckgl-typings/blob/master/deck.gl__react/index.d.ts#L66

export default class DeckGL extends React.Component<DeckGLProps>

It is explicitly extending a React component. However, the MapView component does not:
https://github.com/visgl/deck.gl/blob/master/modules/core/src/views/map-view.js

Then there seems to be this line which enables the JSX syntax of MapView:
https://github.com/visgl/deck.gl/blob/8.2-release/modules/react/src/deckgl.js#L168

// extract any deck.gl layers masquerading as react elements from props.children
const {layers, views} = this._parseJSX(props);

So it looks like they parse the JSX for you as a "magic" way of declaring your views.

However, since MapView is not really a React component, I don't think we should declare it that way in this package. So you may want to make a local declaration in your project similar to the DeckGL way (above) for MapView. Or you can use the layers & views props with standard syntax instead of JSX:

original

<DeckGL
      initialViewState={INITIAL_VIEW_STATE}
      controller={true}
      layers={layers} >
      <MapView id="map" width="50%" controller={true}>
        <StaticMap mapboxApiAccessToken={MAPBOX_ACCESS_TOKEN} />
      </MapView>
      <FirstPersonView width="50%" x="50%" fovy={50} />
    </DeckGL>

change to:

const staticMap= new StaticMap({mapboxApiAccessToken:MAPBOX_ACCESS_TOKEN});
const mapView = new MapView({staticMap});
const firstPersonView = new FirstPersonView({width:"50%", x:"50%", fovy:50});
<DeckGL
      initialViewState={INITIAL_VIEW_STATE}
      controller={true}
      layers={layers} 
      views={[mapView,  firstPersonView]}
/>

I'm shooting from the hip here, I'm not sure if the above code works but I hope you get the idea.

I will leave this issue open for others to add to the discussion.

au-z commented

Thanks for the insight @danmarshall! I ended up taking the approach you suggested, passing in my views as an attribute instead of as JSX which suits my use case just fine. If I end up needing the JSX syntax, I'll take the custom declaration approach you outlined.

I think this is the case for all layer components? I just hit this with GeoJsonLayer as well. Is that correct? Would it be reasonable to get react-specific extended typings that support this?

mz8i commented

Confirmed that I just got here by hitting the issue with GeoJsonLayer.

I'm running into the same with HexagonLayer, ScatterplotLayer, TextLayer and HeatmapLayer. Not in our storybook implementation of the same maps/data/etc though - if I figure out what's different, I'll share.

@danmarshall Thank you in advance for these super useful typings! I know it must be hard to keep this in sync with deck.gl itself.

It's worth noting that even the simple use case from the deck.gl docs results in page that renders fine in-browser (due to aforementioned magic), but results in a type error:

import React from "react";
import { LineLayer } from "@deck.gl/layers";
import DeckGL from "@deck.gl/react";

// Viewport settings
const INITIAL_VIEW_STATE = {
  longitude: -122.41669,
  latitude: 37.7853,
  zoom: 13,
  pitch: 0,
  bearing: 0
};

function App() {
  return (
    <DeckGL initialViewState={INITIAL_VIEW_STATE} controller={true} >
      <LineLayer id="line-layer" data={data} />
    </DeckGL>
  );
}

Result:

ts(2786)
'LineLayer' cannot be used as a JSX component.
  Its instance type 'LineLayer<unknown, { id: string; data: { sourcePosition: number[]; targetPosition: number[]; }[]; }>' is not a valid JSX element.
    Type 'LineLayer<unknown, { id: string; data: { sourcePosition: number[]; targetPosition: number[]; }[]; }>' is missing the following properties from type 'ElementClass': render, forceUpdate, refs

Some options for moving forward:

  1. Don't support this style and add a note to the readme, so folks know not to go down this path!
  2. Tweak DeckGL types to allow non-ReactElement children (is this even possible?). @danmarshall I think it makes sense that we don't want to type things like MapView or layers like LineLayer as ReactElement, since they technically aren't. However technically DeckGL does support these non-ReactElement child types, but the typing itself doesn't because it only extends from React.Component here.
  3. Write a React wrapper for all pseudo React elements like LineLayer and add an example to the readme. It seems this is possible, but not sure how to do properly.

To me option 2 seems like the right path forward, if it's even technically possible (this is way beyond my TS skills!).