Mapbox-gl and Geojson types update

[alex3165]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: https://github.com/mapbox/mapbox-gl-js/blob/master/CHANGELOG.md
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

[typescript-bot]

@alex3165 - It appears Travis did not correctly run on this PR! /cc @RyanCavanaugh to investigate and advise.

[atd-schubert]

The GeometryObject interface should not have an optional property coordinates

[atd-schubert]

A GeometryCollection is also a GeometryObject, but is not allowed to have a coordinates property. At a first glance it seems to be incorrect for me.

Why do you use it here? Maybe we will find another, better solution...

[alex3165]

If the geometry type (GeoJsonGeometryTypes) is Point or Polygon it can have coordinates no? That's why I made it optional, in case it is a GeometryCollection and shouldn't have a coordinates

[atd-schubert]

I think I got the point. But i would not prefer to make it like this. I take a look at an older change especially on the old line 33. In my opinion the GeometryObject should be a type again, like this:

export type GeometryObject = Point | /* other geometries */ | GeometryCollection
// with Geometries like this:

export interface Point extends GeoJsonObject {
    type: "Point";
    coordinates: Position;
}
// and so on...

In short, I think there is no advantage to extends geometries from the GeometryObject instead of GeoJsonObject directly.

What do @Cobster and @JeffJacobson think about that?

[typescript-bot]

@alex3165 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

[alex3165]

Hey @atd-schubert Thanks for your comment, I reverted the changes related to geojson types and created my own type in the end in mapbox-gl instead. could you approve the changes?

[typescript-bot]

🔔 @atd-schubert - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

[typescript-bot]

@alex3165 Thank you for submitting this PR!

🔔 @dobrud @PatrickR - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

[typescript-bot]

We've gotten sign-off from a reviewer 👏. A maintainer will soon review this PR and merge it if there are no issues. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for contributing to DefinitelyTyped!