Style spec references code from maplibre-gl

[rotu]

The following identifiers are defined in the maplibre-gl package but are referenced from the style spec. Their definitions should be moved to the style spec. The code needs to be reorganized to prevent the style spec from being dependent on the implementation:

• CanonicalTileID
• MercatorCoordinate
• EXTENT

[rotu]

Once resolved, I'd like to enforce this from happening again by enabling the eslint import/no-relative-packages rule

[rotu]

The build scripts for style-spec live in the maplibre-gl package. They should probably also be moved within the style spec subpackage.

[wipfli]

Place in style spec which reference maplibre-gl-js code are:

maplibre-gl-js/src/style-spec/expression/evaluation_context.ts

Line 4 in 862e92b

import type {CanonicalTileID} from '../../source/tile_id';

maplibre-gl-js/src/style-spec/expression/index.ts

Line 26 in 862e92b

import type {CanonicalTileID} from '../../source/tile_id';

maplibre-gl-js/src/style-spec/expression/definitions/within.ts

Line 7 in 862e92b

import type {CanonicalTileID} from '../../../source/tile_id';

maplibre-gl-js/src/style-spec/feature_filter/feature_filter.test.ts

Lines 5 to 7 in 862e92b

	import MercatorCoordinate from '../../geo/mercator_coordinate';
	import EXTENT from '../../data/extent';
	import {CanonicalTileID} from '../../source/tile_id';

maplibre-gl-js/src/style-spec/feature_filter/index.ts

Line 3 in 862e92b

import type {CanonicalTileID} from '../../source/tile_id';

[wipfli]

Removing CanonicalTileID and the others from gl js is probably a breaking change, so we should do it as part of v3, #1716

[HarelM]

Wht are these referenced? Moving the style spec to a different repo will also help solving this.

[wipfli]

Ah the references are just the places where file in the src/style-spec folder import something from outside the src/style-spec folder. So these are the places where the style-spec package depends on maplibre-gl-js.

[wipfli]

Some of these circular dependencies were introduced here: mapbox/mapbox-gl-js#9428 and here: mapbox/mapbox-gl-js#9352

[rotu]

Wht are these referenced? Moving the style spec to a different repo will also help solving this.

1. Moving the style spec to another repo won’t solve this. The other way round. Solving this is required to move the code to another repo.
2. Moving to another repo is also unnecessary to enforce this separation. The eslint import/no-relative-packages is sufficient.
3. I’m not sure that it’s correct to move CanonicalTileID to style-spec. As far as I can tell, this logic belongs in the style implementation, not style-spec.

[rotu]

@wipfli, could you fix the refs in feature_filter.test? I tried and I don't understand the code well enough to disentangle this.

[wipfli]

EXTENT seems to be hardcoded in a bunch of places.

[wipfli]

And the class mercator coordinate defined here:

maplibre-gl-js/src/geo/mercator_coordinate.ts

Line 77 in ff99b07

class MercatorCoordinate {

Might be similar to this point module or how it is called and maybe could be factored out.

[birkskyum]

The style-spec actually compiles without importing anything from maplibre-gl-js at this point.

This is obtained by having MercatorCoordinate is inside of style-spec here:
https://github.com/maplibre/maplibre-gl-style-spec/blob/main/src/coordinates/mercator_coordinate.ts

Only the type of CanonicalTileID is used in style-spec, so there is an interface ICanonicalTileID here, which is imported maplibre-gl-js as well:
https://github.com/maplibre/maplibre-gl-style-spec/blob/main/src/tiles_and_coordinates.ts

Also, the LngLat type is necessary for MercatorCoordinate, so it's also to be found in style-spec here:
https://github.com/maplibre/maplibre-gl-style-spec/blob/main/src/coordinates/lng_lat.ts