Push the try/catch up to StyleDeclaration#calculate

mapbox · Oct 5, 2017 · 693366b · 693366b
1 parent 74c4898
commit 693366b
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 68 deletions.
diff --git a/bench/benchmarks/expressions.js b/bench/benchmarks/expressions.js
@@ -5,12 +5,8 @@ const accessToken = require('../lib/access_token');
 const spec = require('../../src/style-spec/reference/latest');
 const createFunction = require('../../src/style-spec/function');
 const convertFunction = require('../../src/style-spec/function/convert');
-const {
-    isExpression,
-    createExpressionWithErrorHandling,
-    getExpectedType,
-    getDefaultValue
-} = require('../../src/style-spec/expression');
+const createExpression = require('../../src/style-spec/expression');
+const { isExpression, getExpectedType } = require('../../src/style-spec/expression');
 
 import type {
     StyleExpression,
@@ -40,7 +36,10 @@ class ExpressionBenchmark extends Benchmark {
                     const expressionData = function(rawValue, propertySpec: StylePropertySpecification) {
                         const rawExpression = convertFunction(rawValue, propertySpec);
                         const compiledFunction = createFunction(rawValue, propertySpec);
-                        const compiledExpression = createExpressionWithErrorHandling(rawExpression, getExpectedType(propertySpec), getDefaultValue(propertySpec));
+                        const compiledExpression = createExpression(rawExpression, getExpectedType(propertySpec));
+                        if (compiledExpression.result !== 'success') {
+                            throw new Error(compiledExpression.errors.map(err => `${err.key}: ${err.message}`).join(', '));
+                        }
                         return {
                             propertySpec,
                             rawValue,
@@ -93,7 +92,7 @@ class FunctionConvert extends ExpressionBenchmark {
 class ExpressionCreate extends ExpressionBenchmark {
     bench() {
         for (const {rawExpression, propertySpec} of this.data) {
-            createExpressionWithErrorHandling(rawExpression, getExpectedType(propertySpec), getDefaultValue(propertySpec));
+            createExpression(rawExpression, getExpectedType(propertySpec));
         }
     }
 }

diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
@@ -10,7 +10,6 @@ const Coalesce = require('./definitions/coalesce');
 const Let = require('./definitions/let');
 const definitions = require('./definitions');
 const isConstant = require('./is_constant');
-const {unwrap} = require('./values');
 const extend = require('../util/extend');
 
 import type {Type} from './types';
@@ -107,43 +106,9 @@ function createExpression(expression: mixed, expectedType: Type | null): StyleEx
     };
 }
 
-function createExpressionWithErrorHandling(expression: mixed, expectedType: Type | null, defaultValue: Value | null): StyleExpression {
-    expression = createExpression(expression, expectedType);
-
-    if (expression.result !== 'success') {
-        // this should have been caught in validation
-        throw new Error(expression.errors.map(err => `${err.key}: ${err.message}`).join(', '));
-    }
-
-    const evaluate = expression.evaluate;
-    const warningHistory: {[key: string]: boolean} = {};
-
-    return extend({}, expression, {
-        evaluate(globals, feature) {
-            try {
-                const val = evaluate(globals, feature);
-                if (val === null || val === undefined) {
-                    return unwrap(defaultValue);
-                }
-                return unwrap(val);
-            } catch (e) {
-                if (!warningHistory[e.message]) {
-                    warningHistory[e.message] = true;
-                    if (typeof console !== 'undefined') {
-                        console.warn(e.message);
-                    }
-                }
-                return unwrap(defaultValue);
-            }
-        }
-    });
-}
-
 module.exports = createExpression;
-module.exports.createExpressionWithErrorHandling = createExpressionWithErrorHandling;
 module.exports.isExpression = isExpression;
 module.exports.getExpectedType = getExpectedType;
-module.exports.getDefaultValue = getDefaultValue;
 
 // Zoom-dependent expressions may only use ["zoom"] as the input to a
 // 'top-level' "curve" expression. (The curve may be wrapped in one or more
@@ -240,16 +205,3 @@ function getExpectedType(spec: StylePropertySpecification): Type | null {
 
     return types[spec.type] || null;
 }
-
-const parseColor = require('../util/parse_color');
-const {Color} = require('./values');
-
-function getDefaultValue(spec: StylePropertySpecification): Value | null {
-    const defaultValue = spec.default;
-    if (spec.type === 'color') {
-        const c: [number, number, number, number] = (parseColor((defaultValue: any)): any);
-        assert(Array.isArray(c));
-        return new Color(c[0], c[1], c[2], c[3]);
-    }
-    return defaultValue || null;
-}
diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js
@@ -7,8 +7,7 @@ const unbundle = require('../util/unbundle_jsonlint');
 module.exports = function validateExpression(options) {
     const expression = createExpression(
         deepUnbundle(options.value.expression),
-        getExpectedType(options.valueSpec),
-        getDefaultValue(options.valueSpec));
+        getExpectedType(options.valueSpec));
 
     if (expression.result === 'success') {
         return [];

diff --git a/src/style/style_declaration.js b/src/style/style_declaration.js
@@ -1,11 +1,14 @@
 // @flow
 
 const parseColor = require('../style-spec/util/parse_color');
-const {createExpressionWithErrorHandling, getExpectedType, getDefaultValue} = require('../style-spec/expression');
+const {getExpectedType} = require('../style-spec/expression');
+const createExpression = require('../style-spec/expression');
 const createFunction = require('../style-spec/function');
 const util = require('../util/util');
 const Curve = require('../style-spec/expression/definitions/curve');
+const {unwrap} = require('../style-spec/expression/values');
 
+import type {GlobalProperties} from './style_layer';
 import type {StyleExpression, Feature} from '../style-spec/expression';
 
 function normalizeToExpression(parameters, propertySpec): StyleExpression {
@@ -29,10 +32,12 @@ function normalizeToExpression(parameters, propertySpec): StyleExpression {
     }
 
     if (parameters.expression) {
-        return createExpressionWithErrorHandling(
-            parameters.expression,
-            getExpectedType(propertySpec),
-            getDefaultValue(propertySpec));
+        const expression = createExpression(parameters.expression, getExpectedType(propertySpec));
+        if (expression.result !== 'success') {
+            // this should have been caught in validation
+            throw new Error(expression.errors.map(err => `${err.key}: ${err.message}`).join(', '));
+        }
+        return expression;
     } else {
         return createFunction(parameters, propertySpec);
     }
@@ -47,6 +52,8 @@ class StyleDeclaration {
     json: mixed;
     minimum: number;
     expression: StyleExpression;
+    defaultValue: any;
+    warningHistory: {[key: string]: boolean};
 
     constructor(reference: any, value: any) {
         this.value = util.clone(value);
@@ -55,15 +62,31 @@ class StyleDeclaration {
         this.json = JSON.stringify(this.value);
 
         this.minimum = reference.minimum;
+        this.defaultValue = reference.type === 'color' ? parseColor(reference.default) : reference.default;
+
         this.expression = normalizeToExpression(this.value, reference);
+        this.warningHistory = {};
     }
 
-    calculate(globals: {zoom: number}, feature?: Feature) {
-        const value = this.expression.evaluate(globals, feature);
-        if (this.minimum !== undefined && value < this.minimum) {
-            return this.minimum;
+    calculate(globals: GlobalProperties, feature?: Feature) {
+        try {
+            const val: any = unwrap(this.expression.evaluate(globals, feature));
+            if (val === null || val === undefined) {
+                return this.defaultValue;
+            }
+            if (this.minimum !== undefined && val < this.minimum) {
+                return this.minimum;
+            }
+            return val;
+        } catch (e) {
+            if (!this.warningHistory[e.message]) {
+                this.warningHistory[e.message] = true;
+                if (typeof console !== 'undefined') {
+                    console.warn(e.message);
+                }
+            }
+            return this.defaultValue;
         }
-        return value;
     }
 
     /**