fix flow; more robust error handling

debug/cluster.html

@@ -20,7 +20,7 @@
 
 var map = window.map = new mapboxgl.Map({
     container: 'map',
-    zoom: 0,
+    zoom: 1,
     center: [0, 0],
     style: 'mapbox://styles/mapbox/cjf4m44iw0uza2spb3q0a7s41',
     hash: true

src/source/geojson_worker_source.js

@@ -11,6 +11,7 @@ import geojsonvt from 'geojson-vt';
 import assert from 'assert';
 import VectorTileWorkerSource from './vector_tile_worker_source';
 import { createExpression } from '../style-spec/expression';
+import { isGlobalPropertyConstant, isFeatureConstant } from '../style-spec/expression/is_constant';
 
 import type {
     WorkerTileParameters,
@@ -31,7 +32,8 @@ export type LoadGeoJSONParameters = {
     source: string,
     cluster: boolean,
     superclusterOptions?: Object,
-    geojsonVtOptions?: Object
+    geojsonVtOptions?: Object,
+    clusterProperties?: Object
 };
 
 export type LoadGeoJSON = (params: LoadGeoJSONParameters, callback: ResponseCallback<Object>) => void;
@@ -300,15 +302,38 @@ function getSuperclusterOptions({superclusterOptions, clusterProperties}) {
     const initialValues = {};
     const mapExpressions = {};
     const reduceExpressions = {};
-    const globals = {accumulated: null};
+    const globals = {accumulated: null, zoom: 0};
     const feature = {properties: null};
     const propertyNames = Object.keys(clusterProperties);
 
     for (const key of propertyNames) {
         const [operator, initialExpression, mapExpression] = clusterProperties[key];
-        initialValues[key] = createClusterExpression(key, initialExpression).evaluate(globals);
-        mapExpressions[key] = createClusterExpression(key, mapExpression);
-        reduceExpressions[key] = createClusterExpression(key, [operator, ['accumulated'], ['get', key]]);
+
+        const parsed = createExpression(clusterProperties[key]);
+        if (parsed.result === 'error') {
+            const message = parsed.value.map(e => e.message).join('; ');
+            throw new Error(`Error parsing expression for cluster property "${key}": ${message}`);
+
+        } else if (!isGlobalPropertyConstant(parsed.value.expression, ['zoom'])) {
+            throw new Error(`Error parsing expression for cluster property "${key}": zoom expressions not supported.`);
+        }
+
+        const initialExpressionParsed = createExpression(initialExpression);
+        const mapExpressionParsed = createExpression(mapExpression);
+        const reduceExpressionParsed = createExpression([operator, ['accumulated'], ['get', key]]);
+
+        if (initialExpressionParsed.result === 'success') {
+            if (!isFeatureConstant(initialExpressionParsed.value.expression))
+                throw new Error(`Error parsing expression for cluster property "${key}": can't use feature properties in initial expression.`);
+
+            initialValues[key] = initialExpressionParsed.value.evaluate(globals);
+        }
+
+        if (mapExpressionParsed.result === 'success')
+            mapExpressions[key] = mapExpressionParsed.value;
+
+        if (reduceExpressionParsed.result === 'success')
+            reduceExpressions[key] = reduceExpressionParsed.value;
     }
 
     superclusterOptions.initial = () => {
@@ -337,14 +362,4 @@ function getSuperclusterOptions({superclusterOptions, clusterProperties}) {
     return superclusterOptions;
 }
 
-function createClusterExpression(key, expression) {
-    const parsed = createExpression(expression, {});
-    if (parsed.result === 'success') {
-        return parsed.value;
-    }
-    /* eslint no-warning-comments: 0 */
-    // TODO proper error handling
-    console.log(`Error: ${parsed.value.map(e => e.message).join('; ')}`);
-}
-
 export default GeoJSONWorkerSource;

src/style-spec/expression/definitions/index.js

@@ -204,7 +204,7 @@ CompoundExpression.register(expressions, {
     'accumulated': [
         ValueType,
         [],
-        (ctx) => ctx.globals.accumulated
+        (ctx) => ctx.globals.accumulated === undefined ? null : ctx.globals.accumulated
     ],
     '+': [
         NumberType,

src/style-spec/expression/index.js

@@ -38,7 +38,8 @@ export type GlobalProperties = $ReadOnly<{
     zoom: number,
     heatmapDensity?: number,
     lineProgress?: number,
-    isSupportedScript?: (string) => boolean
+    isSupportedScript?: (string) => boolean,
+    accumulated?: Value
 }>;
 
 export class StyleExpression {
@@ -49,12 +50,12 @@ export class StyleExpression {
     _warningHistory: {[key: string]: boolean};
     _enumValues: ?{[string]: any};
 
-    constructor(expression: Expression, propertySpec: StylePropertySpecification) {
+    constructor(expression: Expression, propertySpec: ?StylePropertySpecification) {
         this.expression = expression;
         this._warningHistory = {};
         this._evaluator = new EvaluationContext();
-        this._defaultValue = getDefaultValue(propertySpec);
-        this._enumValues = propertySpec.type === 'enum' ? propertySpec.values : null;
+        this._defaultValue = propertySpec ? getDefaultValue(propertySpec) : null;
+        this._enumValues = propertySpec && propertySpec.type === 'enum' ? propertySpec.values : null;
     }
 
     evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
@@ -105,12 +106,12 @@ export function isExpression(expression: mixed) {
  *
  * @private
  */
-export function createExpression(expression: mixed, propertySpec: StylePropertySpecification): Result<StyleExpression, Array<ParsingError>> {
-    const parser = new ParsingContext(definitions, [], getExpectedType(propertySpec));
+export function createExpression(expression: mixed, propertySpec: ?StylePropertySpecification): Result<StyleExpression, Array<ParsingError>> {
+    const parser = new ParsingContext(definitions, [], propertySpec ? getExpectedType(propertySpec) : undefined);
 
     // For string-valued properties, coerce to string at the top level rather than asserting.
     const parsed = parser.parse(expression, undefined, undefined, undefined,
-        propertySpec.type === 'string' ? {typeAnnotation: 'coerce'} : undefined);
+        propertySpec && propertySpec.type === 'string' ? {typeAnnotation: 'coerce'} : undefined);
 
     if (!parsed) {
         assert(parser.errors.length > 0);