search

Turfjs / turf

A modular geospatial engine written in JavaScript and TypeScript
https://turfjs.org/
MIT License
9.25k stars 937 forks source link

Add explicit geojson types dependency #2676

Closed mfedderly closed 1 month ago

mfedderly commented 1 month ago

Fixes https://github.com/Turfjs/turf/issues/2617

I struggled a bit trying to figure out what type of dependency this actually represents. I considered defining it as a peer dependency, but in the end it seems like several other packages declare it as a normal dependency (including maplibre-gl).

PR done in two commits, the first one is the manual changes and the second is just running monorepolint and pnpm to get everything in sync with the new spec.

smallsaucepan commented 1 month ago

@mfedderly did we intentionally go from 7946.0.8 back to 7946.0.0 in package.json?

mfedderly commented 1 month ago

@smallsaucepan yes. Internally we're still using the pnpm-lock version of 7946.0.14 within turf itself. Is there some incompatibility across the @types/geojson that I should know about (between 7946.0.0 and .14)?

twelch commented 1 month ago

@mfedderly I don't quite get it, why not use the latest? If we want to communicate that a lower/minimum version is supported, is peerDependencies the way to do that?

What happens if the lockfile gets rebuilt, will it pick up 7946.0.0?

mfedderly commented 1 month ago

If you specified 7946.0.8 then all of the consumers would have to be on .8 or newer. I figured that we didn't want to support 1.x.x versions, but that we wanted to be pretty generous with the 7946.x.x series. I don't think you can specify the same dependency in dependencies and peerDependencies to allow more wiggle room, it has to be in dependencies. If we deleted the pnpm-lock.yaml and re-built it, it should choose the latest version of everything that is compatible with the declarations in the various package.json files.

smallsaucepan commented 1 month ago

@mfedderly yes, looks like there were incompatible changes between .0 and .14. They haven't done us any favours with their versioning scheme.

In Feature interface the geometry and properties fields both became more restrictive:

147c146
<     geometry: G | null;
---
>     geometry: G;

156c155
<     properties: P | null;
---
>     properties: P;
mfedderly commented 1 month ago

@smallsaucepan oy ok. Updated to .10 minimum version and explained some reasoning https://github.com/Turfjs/turf/pull/2688