Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow 0.69 #6594

Merged
merged 3 commits into from
May 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
.*/test/integration/render-tests/.*

[version]
0.66.0
0.69.0
1 change: 1 addition & 0 deletions flow-typed/vector-tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ declare interface VectorTile {
}

declare interface VectorTileLayer {
version?: number;
name: string;
extent: number;
length: number;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"eslint-plugin-node": "^5.1.1",
"eslint-plugin-react": "^7.3.0",
"execcommand-copy": "^1.1.0",
"flow-bin": "^0.66.0",
"flow-bin": "^0.69.0",
"flow-coverage-report": "^0.3.0",
"github-slugger": "^1.1.1",
"gl": "^4.0.1",
Expand Down
21 changes: 11 additions & 10 deletions src/data/program_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type VertexBuffer from '../gl/vertex_buffer';
import type Program from '../render/program';
import type {
Feature,
FeatureState,
GlobalProperties,
SourceExpression,
CompositeExpression
Expand Down Expand Up @@ -64,7 +65,7 @@ interface Binder<T> {
statistics: { max: number };

populatePaintArray(length: number, feature: Feature): void;
updatePaintArray(start: number, length: number, feature: Feature): void;
updatePaintArray(start: number, length: number, feature: Feature, featureState: FeatureState): void;
upload(Context): void;
destroy(): void;

Expand Down Expand Up @@ -163,9 +164,9 @@ class SourceExpressionBinder<T> implements Binder<T> {
}
}

updatePaintArray(start: number, end: number, feature: Feature) {
updatePaintArray(start: number, end: number, feature: Feature, featureState: FeatureState) {
const paintArray = this.paintVertexArray;
const value = this.expression.evaluate({zoom: 0}, feature);
const value = this.expression.evaluate({zoom: 0}, feature, featureState);

if (this.type === 'color') {
const color = packColor(value);
Expand Down Expand Up @@ -255,11 +256,11 @@ class CompositeExpressionBinder<T> implements Binder<T> {
}
}

updatePaintArray(start: number, end: number, feature: Feature) {
updatePaintArray(start: number, end: number, feature: Feature, featureState: FeatureState) {
const paintArray = this.paintVertexArray;

const min = this.expression.evaluate({zoom: this.zoom }, feature);
const max = this.expression.evaluate({zoom: this.zoom + 1}, feature);
const min = this.expression.evaluate({zoom: this.zoom }, feature, featureState);
const max = this.expression.evaluate({zoom: this.zoom + 1}, feature, featureState);

if (this.type === 'color') {
const minColor = packColor(min);
Expand Down Expand Up @@ -394,18 +395,18 @@ export default class ProgramConfiguration {
const posArray = this._idMap[id];
if (!posArray) continue;

const featureState = featureStates[id];
for (const pos of posArray) {
const feature: any = vtLayer.feature(pos.index);
feature.state = featureStates[id];
const feature = vtLayer.feature(pos.index);

for (const property in this.binders) {
const binder: Binder<any> = this.binders[property];
const binder = this.binders[property];
if (binder instanceof ConstantBinder) continue;
if ((binder: any).expression.isStateDependent === true) {
//AHM: Remove after https://github.com/mapbox/mapbox-gl-js/issues/6255
const value = layer.paint.get(property);
(binder: any).expression = value.value;
binder.updatePaintArray(pos.start, pos.end, feature);
binder.updatePaintArray(pos.start, pos.end, feature, featureState);
dirty = true;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/source/geojson_worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type LoadGeoJSONParameters = {
request?: RequestParameters,
data?: string,
source: string,
cluster: boolean,
superclusterOptions?: Object,
geojsonVtOptions?: Object
};
Expand Down
24 changes: 15 additions & 9 deletions src/source/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type Map from '../ui/map';
import type Tile from './tile';
import type {OverscaledTileID} from './tile_id';
import type {Callback} from '../types/callback';
import {CanonicalTileID} from './tile_id';

/**
* The `Source` interface must be implemented by each source type, including "core" types (`vector`, `raster`,
Expand All @@ -34,15 +35,6 @@ import type {Callback} from '../types/callback';
* if they are floor-ed to the nearest integer.
*/
export interface Source {
/**
* An optional URL to a script which, when run by a Worker, registers a {@link WorkerSource}
* implementation for this Source type by calling `self.registerWorkerSource(workerSource: WorkerSource)`.
* @private
*/
// Static interface properties are not supported in flow as of 0.62.0.
// https://github.com/facebook/flow/issues/5590
// static workerSourceURL?: URL;

+type: string;
id: string;
minzoom: number,
Expand All @@ -51,6 +43,9 @@ export interface Source {
attribution?: string,

roundZoom?: boolean,
isTileClipped?: boolean,
mapbox_logo?: boolean,
tileID?: CanonicalTileID;
reparseOverscaled?: boolean,
vectorLayerIds?: Array<string>,

Expand All @@ -77,6 +72,17 @@ export interface Source {
+prepare?: () => void;
}

type SourceStatics = {
/**
* An optional URL to a script which, when run by a Worker, registers a {@link WorkerSource}
* implementation for this Source type by calling `self.registerWorkerSource(workerSource: WorkerSource)`.
* @private
*/
workerSourceURL?: URL;
};

export type SourceClass = Class<Source> & SourceStatics;

import vector from '../source/vector_tile_source';
import raster from '../source/raster_tile_source';
import rasterDem from '../source/raster_dem_tile_source';
Expand Down
4 changes: 2 additions & 2 deletions src/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class SourceCache extends Evented {
_tileLoaded(tile: Tile, id: string | number, previousState: TileState, err: ?Error) {
if (err) {
tile.state = 'errored';
if (err.status !== 404) this._source.fire(new ErrorEvent(err, {tile}));
if ((err: any).status !== 404) this._source.fire(new ErrorEvent(err, {tile}));
// continue to try loading parent/children tiles if a tile doesn't exist (404)
else this.update(this.transform);
return;
Expand Down Expand Up @@ -461,7 +461,7 @@ class SourceCache extends Evented {
if (!this.used) {
idealTileIDs = [];
} else if (this._source.tileID) {
idealTileIDs = transform.getVisibleUnwrappedCoordinates((this._source.tileID: any))
idealTileIDs = transform.getVisibleUnwrappedCoordinates(this._source.tileID)
.map((unwrapped) => new OverscaledTileID(unwrapped.canonical.z, unwrapped.wrap, unwrapped.canonical.z, unwrapped.canonical.x, unwrapped.canonical.y));
} else {
idealTileIDs = transform.coveringTiles({
Expand Down
6 changes: 4 additions & 2 deletions src/source/source_state.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// @flow

import { extend } from '../util/util';
import Tile from './tile';
import type {FeatureState} from '../style-spec/expression';

export type FeatureStates = {[feature_id: string]: {[key: string]: any }};
export type FeatureStates = {[feature_id: string]: FeatureState};
export type LayerFeatureStates = {[layer: string]: FeatureStates};

/**
/**
* SourceFeatureState manages the state and state changes
* to features in a source, separated by source layer.
*
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/expression/definitions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ CompoundExpression.register(expressions, {
'feature-state': [
ValueType,
[StringType],
(ctx, [key]) => get(key.evaluate(ctx), ctx.state())
(ctx, [key]) => get(key.evaluate(ctx), ctx.featureState || {})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a check for feature when using featureState? Feature state is only valid if there is a current feature. As it stands, it's possible to call Expression#evaluate with a null feature, but non-null featureState.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to fix it this way:

diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
index 1310db3f2..269f718ec 100644
--- a/src/style-spec/expression/index.js
+++ b/src/style-spec/expression/index.js
@@ -179,18 +179,18 @@ export class ZoomDependentExpression<Kind> {
 
 export type ConstantExpression = {
     kind: 'constant',
-    +evaluate: (globals: GlobalProperties, feature?: Feature) => any,
+    +evaluate: (globals: GlobalProperties) => any,
 }
 
 export type SourceExpression = {
     kind: 'source',
     isStateDependent: boolean,
-    +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
+    +evaluate: (globals: GlobalProperties, feature: Feature, featureState?: FeatureState) => any,
 };
 
 export type CameraExpression = {
     kind: 'camera',
-    +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
+    +evaluate: (globals: GlobalProperties, feature: Feature, featureState?: FeatureState) => any,
     +interpolationFactor: (input: number, lower: number, upper: number) => number,
     zoomStops: Array<number>
 };
@@ -198,7 +198,7 @@ export type CameraExpression = {
 export type CompositeExpression = {
     kind: 'composite',
     isStateDependent: boolean,
-    +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
+    +evaluate: (globals: GlobalProperties, feature: Feature, featureState?: FeatureState) => any,
     +interpolationFactor: (input: number, lower: number, upper: number) => number,
     zoomStops: Array<number>
 };

But this requires #6255 (comment), so I'd like to defer it.

However, in looking at this, I noticed that I'd failed to update ZoomDependentExpression#evaluate[WithoutErrorHandling] -- I've added additional render tests to cover that case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. #6255 is needed soon, and I don't see an actual code path at this time where this issue would come up.

],
'properties': [
ObjectType,
Expand Down
7 changes: 2 additions & 5 deletions src/style-spec/expression/evaluation_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

import { Color } from './values';

import type { Feature, GlobalProperties } from './index';
import type { GlobalProperties, Feature, FeatureState } from './index';

const geometryTypes = ['Unknown', 'Point', 'LineString', 'Polygon'];

class EvaluationContext {
globals: GlobalProperties;
feature: ?Feature;
featureState: ?FeatureState;

_parseColorCache: {[string]: ?Color};

Expand All @@ -28,10 +29,6 @@ class EvaluationContext {
return this.feature && this.feature.properties || {};
}

state() {
return this.feature && this.feature.state || {};
}

parseColor(input: string): ?Color {
let cached = this._parseColorCache[input];
if (!cached) {
Expand Down
30 changes: 17 additions & 13 deletions src/style-spec/expression/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export type Feature = {
+properties: {[string]: any}
};

export type FeatureState = {[string]: any};

export type GlobalProperties = $ReadOnly<{
zoom: number,
heatmapDensity?: number,
Expand All @@ -53,24 +55,26 @@ export class StyleExpression {
}
}

evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature): any {
evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
if (!this._evaluator) {
this._evaluator = new EvaluationContext();
}

this._evaluator.globals = globals;
this._evaluator.feature = feature;
this._evaluator.featureState = featureState;

return this.expression.evaluate(this._evaluator);
}

evaluate(globals: GlobalProperties, feature?: Feature): any {
evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
if (!this._evaluator) {
this._evaluator = new EvaluationContext();
}

this._evaluator.globals = globals;
this._evaluator.feature = feature;
this._evaluator.featureState = featureState;

try {
const val = this.expression.evaluate(this._evaluator);
Expand Down Expand Up @@ -129,12 +133,12 @@ export class ZoomConstantExpression<Kind> {
this.isStateDependent = kind !== 'constant' && !isConstant.isStateConstant(expression.expression);
}

evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature): any {
return this._styleExpression.evaluateWithoutErrorHandling(globals, feature);
evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState);
}

evaluate(globals: GlobalProperties, feature?: Feature): any {
return this._styleExpression.evaluate(globals, feature);
evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
return this._styleExpression.evaluate(globals, feature, featureState);
}
}

Expand All @@ -156,12 +160,12 @@ export class ZoomDependentExpression<Kind> {
}
}

evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature): any {
return this._styleExpression.evaluateWithoutErrorHandling(globals, feature);
evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState);
}

evaluate(globals: GlobalProperties, feature?: Feature): any {
return this._styleExpression.evaluate(globals, feature);
evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
return this._styleExpression.evaluate(globals, feature, featureState);
}

interpolationFactor(input: number, lower: number, upper: number): number {
Expand All @@ -181,20 +185,20 @@ export type ConstantExpression = {
export type SourceExpression = {
kind: 'source',
isStateDependent: boolean,
+evaluate: (globals: GlobalProperties, feature?: Feature) => any,
+evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
};

export type CameraExpression = {
kind: 'camera',
+evaluate: (globals: GlobalProperties, feature?: Feature) => any,
+evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
+interpolationFactor: (input: number, lower: number, upper: number) => number,
zoomStops: Array<number>
};

export type CompositeExpression = {
kind: 'composite',
isStateDependent: boolean,
+evaluate: (globals: GlobalProperties, feature?: Feature) => any,
+evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any,
+interpolationFactor: (input: number, lower: number, upper: number) => number,
zoomStops: Array<number>
};
Expand Down
5 changes: 3 additions & 2 deletions src/style-spec/feature_filter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ function isExpressionFilter(filter: any) {
const filterSpec = {
'type': 'boolean',
'default': false,
'function': true,
'function': 'piecewise-constant',
'property-function': true,
'zoom-function': true
'zoom-function': true,
'transition': false
};

/**
Expand Down
21 changes: 14 additions & 7 deletions src/style-spec/style-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,57 @@

export type StylePropertySpecification = {
type: 'number',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
transition: boolean,
default?: number
} | {
type: 'string',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
default?: string,
transition: boolean,
tokens?: boolean
} | {
type: 'boolean',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
transition: boolean,
default?: boolean
} | {
type: 'enum',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
values: {[string]: {}},
transition: boolean,
default?: string
} | {
type: 'color',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
transition: boolean,
default?: string
} | {
type: 'array',
value: 'number',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
transition: boolean,
length?: number,
default?: Array<number>
} | {
type: 'array',
value: 'string',
'function': boolean,
'function': 'interpolated' | 'piecewise-constant',
'property-function': boolean,
'zoom-function': boolean,
transition: boolean,
length?: number,
default?: Array<string>
};
Expand Down
Loading