Skip to content

Commit

Permalink
[master] Fix style-spec isolation for within expression (#9522)
Browse files Browse the repository at this point in the history
* - Add a build-time check for style-spec referencing external files
- Refactor within to NOT import external files

* Address review comments

(cherry picked from commit 2b898c9)
  • Loading branch information
Arindam Bose authored and karimnaaji committed Apr 10, 2020
1 parent f4c148f commit 388424a
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/style-spec/expression/definitions/within.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,30 @@ import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {GeoJSON, GeoJSONPolygon, GeoJSONMultiPolygon} from '@mapbox/geojson-types';
import MercatorCoordinate from '../../../geo/mercator_coordinate';
import EXTENT from '../../../data/extent';
import Point from '@mapbox/point-geometry';
import type {CanonicalTileID} from '../../../source/tile_id';

type GeoJSONPolygons =| GeoJSONPolygon | GeoJSONMultiPolygon;

// minX, minY, maxX, maxY
type BBox = [number, number, number, number];
const EXTENT = 8192;

function updateBBox(bbox: BBox, coord: Point) {
bbox[0] = Math.min(bbox[0], coord[0]);
bbox[1] = Math.min(bbox[1], coord[1]);
bbox[2] = Math.max(bbox[2], coord[0]);
bbox[3] = Math.max(bbox[3], coord[1]);
}

function mercatorXfromLng(lng: number) {
return (180 + lng) / 360;
}

function mercatorYfromLat(lat: number) {
return (180 - (180 / Math.PI * Math.log(Math.tan(Math.PI / 4 + lat * Math.PI / 360)))) / 360;
}

function boxWithinBox(bbox1: BBox, bbox2: BBox) {
if (bbox1[0] <= bbox2[0]) return false;
if (bbox1[2] >= bbox2[2]) return false;
Expand All @@ -32,9 +40,10 @@ function boxWithinBox(bbox1: BBox, bbox2: BBox) {
}

function getTileCoordinates(p, canonical: CanonicalTileID) {
const coord = MercatorCoordinate.fromLngLat({lng: p[0], lat: p[1]}, 0);
const x = mercatorXfromLng(p[0]);
const y = mercatorYfromLat(p[1]);
const tilesAtZoom = Math.pow(2, canonical.z);
return [Math.round(coord.x * tilesAtZoom * EXTENT), Math.round(coord.y * tilesAtZoom * EXTENT)];
return [Math.round(x * tilesAtZoom * EXTENT), Math.round(y * tilesAtZoom * EXTENT)];
}

function onBoundary(p, p1, p2) {
Expand Down
20 changes: 20 additions & 0 deletions src/style-spec/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from 'path';
import replace from 'rollup-plugin-replace';
import buble from 'rollup-plugin-buble';
import resolve from 'rollup-plugin-node-resolve';
Expand All @@ -14,6 +15,8 @@ const transforms = {
modules: esm ? false : undefined
};

const ROOT_DIR = __dirname;

const config = [{
input: `${__dirname}/style-spec.js`,
output: {
Expand All @@ -23,6 +26,23 @@ const config = [{
sourcemap: true
},
plugins: [
{
name: 'dep-checker',
resolveId(source, importer) {
// Some users reference modules within style-spec package directly, instead of the bundle
// This means that files within the style-spec package should NOT import files from the parent mapbox-gl-js tree.
// This check will cause the build to fail on CI allowing these issues to be caught.
if (importer && !importer.includes('node_modules')) {
const resolvedPath = path.join(importer, source);
const fromRoot = path.relative(ROOT_DIR, resolvedPath);
if (fromRoot.length > 2 && fromRoot.slice(0, 2) === '..') {
throw new Error(`Module ${importer} imports ${source} from outside the style-spec package root directory.`);
}
}

return null;
}
},
// https://github.com/zaach/jison/issues/351
replace({
include: /\/jsonlint-lines-primitives\/lib\/jsonlint.js/,
Expand Down

0 comments on commit 388424a

Please sign in to comment.