GeoJsonLayerProps: data definition does not accept GeoJson types

[etimberg]

I have some code that is typed using @types/geojson. When integrating types for Deck.gl into my codebase, I get a type warning when creating a GeoJSONLayer.

Reproduce

import { GeoJsonLayer } from 'deck.gl' };
import { Feature, FeatureCollection, Polygon } from 'geojson';

const createGeoJsonLayer = (data: FeatureCollection<Feature<Polygon>>) => {
  return [
    new GeoJsonLayer({
       data,
    });
  ];
};

The error I get is:

Type 'FeatureCollection<Feature<Polygon, GeoJsonProperties>, GeoJsonProperties>' is not assignable to type 'string | DataSet | Promise<DataSet> | undefined'. Type 'FeatureCollection<Feature<Polygon, GeoJsonProperties>, GeoJsonProperties>' is missing the following properties from type 'Promise<DataSet>': then, catch, [Symbol.toStringTag], finally

I think the data property needs to be redefined on the GeoJsonLayerProps rather than using the definition from LayerProps<D>. https://github.com/danmarshall/deckgl-typings/blob/master/deck.gl__core/index.d.ts#L1106

Possible Solutions

I can think of two possible solutions to this:

• Type the GeoJson layer strictly to just GeoJson data
• Expand the type from DataSet<D> | Promise<DataSet<D>> | string; to D | DataSet<D> | Promise<DataSet<D>> | string; for this layer

My Setup

Version: 3.5.0 Deck.gl Version: 7.3.15

[danmarshall]

Is the FeatureCollection iterable? I'm wondering if you can merge it with an ArrayLike interface, then use that new interface instead?

[etimberg]

No, it's an object that looks like what's shown below. The individual Feature objects are the ones that are then drawn on the layer.

{
  type: "FeatureCollection";
  features: Feature[];
}

[danmarshall]

Can you omit the FeatureCollection and just use the Feature<Polygon> ? Either way, your original proposal is fine to me. If you like, you can submit a PR and folks can comment if it works for them.

[etimberg]

That's a good work around. I will look into submitting a PR with the updates 😄

[brajan1984]

hey! Looks like this issue is still open. We can see that types has been changed since, but we still see lack of allowing to pass FeatureCollection to GeoJsonLayerProps as data container. I think it should be possible, because deck.gl allows it and constraints on data in Layer is to strict. Is there any progress on this topic?

The other thing is that we can create PR with an idea to pass any container of data to Layer as third generic option there with fallback to current data type, so we can pass it in GeoJsonLayerProps, but before we start just quick question if this is already resolved and we don't see something.

Really rough idea (it would go through CompositeLayerProps etc.):

export interface LayerProps<D> extends LayerPropsBase<D, DataSet<D> | Promise<DataSet<D>> | string>  { }

export interface LayerPropsBase<D, C> {
  //https://deck.gl/#/documentation/deckgl-api-reference/layers/layer?section=properties
  id?: string;
  data?: C;
  ...
}

[strican]

So I came up with two possible solutions:

• The simple fix, which just adds an extra type to the data attribute that's fully generic. This has the smallest delta but it opens up the type constraints for any other layer type.
• The more complex fix, which does similarly by making the data of type any, which is technically true since layers can add more types to it. However, we then go through and redefine the data parameter in each of the inherited interfaces, which then limit it back. For the GeoJsonLayerProps, we specify a GeoJSON.GeoJSON, type, which should be what we want specifically. This is more specific, but has the downside of each layer needing to specify similar types. I'm also not 100% sure how Typescript handles that. It seems to work fine because we're scoping down the type, but I can't be certain there's no weird interactions.

I figured I'd post here first to get thoughts before I opened a PR for the favored solution.

For the lazy, react 1️⃣ for simple, 2️⃣ for complex

[strican]

@danmarshall Should I just open a PR? I wanted to get some thoughts before I did so since there were two options.

[danmarshall]

Hi @strican - I'm still unsure which approach would be better. I would say go for the simple approach first, and hope it doesn't break anything or reduce the TypeScript features of data for existing users. So that means adding a few unit tests to the PR which still access strong-typed data on a layer.

[strican]

Alright, @danmarshall I opened up #186 with the simple fix. I'm not sure how we go about validating this. Taking a look at the unit test, we're doing the following:

var x = layer1.props.data as P[];
x[0].foo;

but layer1.props.data is of type string | P | DataSet<P> | Promise<DataSet<P>> now. So I'm not sure we can with confidence do the cast we're performing. Or I'm not sure how to test what you want.

[strican]

Although I guess this was never guaranteed before either. It could have been a string or a Promise. What were you thinking for validating this?