placemark / check-geojson

a checker for the geojson format. goes beyond a schema, checking semantics and producing character-level warnings.
http://check-geojson.docs.placemark.io/
MIT License
72 stars 2 forks source link

0.1.8 seems to be very different from 0.1.7 #20

Closed Primajin closed 2 years ago

Primajin commented 2 years ago

Hey there, we are using getIssues from this library for validating our geojsons, however after bumping from 0.1.7 to 0.1.8 we have many unit tests breaking as it seems to behave very differently - do you have an upgrade guide we can follow along?

jfgodoy commented 2 years ago

Hi! same issue here. My tests are failing when I check a geojson geometry, which is a valid use case according to the tests. I have trace the problem to the typescript compilation.

If I replace in the types.ts file

export const GEOJSON_TYPES = new Set<GeoJSON['type']>([
  ...GEOJSON_GEOMETRY_TYPES,
  'Feature',
  'FeatureCollection',
]);

for

export const GEOJSON_TYPES = new Set<GeoJSON['type']>([
  'Point',
  'MultiPoint',
  'Polygon',
  'MultiPolygon',
  'LineString',
  'MultiLineString',
  'GeometryCollection',
  'Feature',
  'FeatureCollection',
]);

works perfectly well.

The weird thing is that the tests are defined in TS and seems that aren't affected by this issue.

imylly commented 2 years ago

Hi all, I ran into this issue when using knex-postgis a while back. Solved it by forcing the transitive check-geojson dependency to 0.1.7, but also happened to stumble upon the root cause. As hinted above, it's related to use of spread operator on sets and transpilation - ES6 transpilation does not handle said case well, as is discussed f.ex. here. The javascript target will look something like this

p=new Set(["Point","MultiPoint","Polygon","MultiPolygon","LineString","MultiLineString","GeometryCollection"]);
m=new Set([].concat(p,["Feature","FeatureCollection"]));

which is evidently wrong.

A naive fix is to define the union as

export const GEOJSON_TYPES = new Set<GeoJSON['type']>([
  ...Array.from(GEOJSON_GEOMETRY_TYPES),
  'Feature',
  'FeatureCollection',
]);

which produces the proper flat set in the transpiled javascript.

tmcw commented 2 years ago

Sorry, my notification settings on this repo were borked and I didn't get any notification for this issue.

From what I can tell, this is caused by an upstream transpiler transpiling this module but not handling the 'downlevel iteration' feature. I'll make this code more vanilla in the next release.

Primajin commented 2 years ago

OK I can confirm that 0.1.9 is passing all tests again 👍🏻