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

Evaluate an expression, outside of rendering #7670

Open
peterkhayes opened this issue Dec 5, 2018 · 22 comments
Open

Evaluate an expression, outside of rendering #7670

peterkhayes opened this issue Dec 5, 2018 · 22 comments

Comments

@peterkhayes
Copy link

peterkhayes commented Dec 5, 2018

Motivation

MapboxGL expressions allow computing things based on feature properties. For example, at my work, we use an expression to determine which color various point features should be rendered as.

However, sometimes it is useful to be able to reuse logic between Mapbox code and other code. In this case, we would like a marker tooltip attached to a point to have the same color as the point.

Design Alternatives

Currently, if a value is needed both during rendering, and outside of rendering, it looks like you need to implement it once as an expression, and once in Javascript. This is possible (and is what we're doing now), but has some downsides:

  • If the expression is somewhat complicated, it's possible that there will be differences between the expression version and the Javascript version.
  • It's not possible to write unit tests for expressions at present, so it's also not possible to test that the implementations match.

Design

The Map instance could include a method to evaluate expressions. It would take an expression and a feature, and return the result of evaluating that expression on that feature. Any global properties (such as zoom) would come from the map.

Alternatively, the evaluation could be a pure function, independent of a map. However, this would make it more difficult to handle properties like zoom, and doesn't really add much value in the case described under "Motivation".

Mock-Up

Here's a simplified example:

const map = new mapboxgl.Map();

const colorExpr = ['case', ['get', 'isRed'], 'red', 'green'];

const redFeature = {
  type: "Feature",
  geometry: { type: "Point", coordinates: [0, 0] },
  properties: { isRed: true },
}

const notRedFeature = {
  type: "Feature",
  geometry: { type: "Point", coordinates: [0, 1] },
  properties: { isRed: false },
}

map.evaluateExpression(colorExpr, redFeature); // 'red'
map.evaluateExpression(colorExpr, notRedFeature); // 'green'

Concepts

Adding an entry under the list of methods here should be sufficient, I think.

Implementation

I'm currently not very familiar with the internals of MapboxGL JS. However, a quick look inside the source, under style-spec/expression/index.js, shows code that compiles and executes expressions, so I would hope that it wouldn't require too much work to hook into this code. With a little direction, I'd be happy to take a stab at this.

@asheemmamoowala
Copy link
Contributor

@peterkhayes Thank you for the detailed issue description.

If the evaluateExpression method is expected to match the exact behavior of an expression used in the style, it needs to know what property the expression is being used for. This allows validating enum type values, coercing the correct result, and ensuring that expressions are nested appropriately.

You can implement map#evaluateExpression with logic similar to style-spec/expression/index.js#createExpression

  • Use a ParsingContext with an appropriate (or null) value for expectedType
  • Call evaluate() on the expression
    • Populate the globalProperties param with values from the current map.
    • If needed, provide feature and featureState parameters. You can use Map#queryRenderedFeatures and Map#getFeatureState to retrieve values from the map.

@htrex
Copy link

htrex commented Feb 8, 2019

This would be a very useful method.@peterkhayes did you get a chance to take a stab?

@peterkhayes
Copy link
Author

I unfortunately have not. If you'd like to, I would appreciate it for sure!

@tilgovi
Copy link

tilgovi commented May 29, 2019

I'm now experimenting with something like this:

// Expression evaluation
// NOTE: This implementation does not accept a StylePropertySpecification. As a consequence of this
// decision, it will not check the type of the evaluation result and throw runtime evaluation errors.
// The caller should be aware that the result is untyped and wrap the expression in a type assertion
// if runtime type checking is desired.
export function evaluate(
  expression: mixed,
  globals: GlobalProperties,
  feature?: Feature,
  featureState?: FeatureState,
): any {
  const parseResult = createExpression(expression);

  if (parseResult.result === 'success') {
    return parseResult.value.evaluate(globals, feature, featureState);
  }

  throw parseResult.value[0];
}

@SamuelDev
Copy link

Has there been any progress on making this a feature in Mapbox? There isn't really a way to unit test expressions right now, and I'm kind of surprised more people aren't worried about this.

@ryanhamley
Copy link
Contributor

@SamuelDev Expression tests are part of our integration test-suite. You can see all the tests here and they can be run with yarn run test-expressions

@tilgovi
Copy link

tilgovi commented Aug 29, 2019

Took me a couple minutes to dig for how those expression tests are evaluated, so I'll link it here: https://github.com/mapbox/mapbox-gl-js/blob/master/test/expression.test.js

@SamuelDev
Copy link

@ryanhamley Thanks for such a quick reply Ryan. I don't think that really helps us in our case though. We are consuming Mapbox through the npm package, and need to test an expression we have that has business logic inside of it. I can see if it is possible to extract some code from your integration test code base, but something like map.evaluateExpression would be super handy.

@ryanhamley
Copy link
Contributor

@SamuelDev Thanks for letting us know your use case. I misunderstood and was trying to assuage any worries that expressions just weren't tested by the library.

This request is something that's come up a few times. Right now, it's not on our immediate roadmap, but we do of course welcome pull requests if anyone wants to take a stab at it. Asheem laid out the general idea in #7670 (comment)

cc @chloekraw There's some interest in a feature like this

@danvk
Copy link

danvk commented Oct 28, 2019

I wound up creating a TypeScript wrapper for Expression evaluation. If it's helpful to anyone, code is in this gist. One nice thing is that you get proper TypeScript types out if you specify an expected type for the expression:

const r1 = Expression.parse(['+', 1, 2]).evaluate(null!);  // type is any
const r2 = Expression.parse(['+', 1, 2], 'number').evaluate(null!);  // type is number
const r3 = Expression.parse(['+', 1, 2], 'string').evaluate(null!);  // throws

@marlag
Copy link

marlag commented Jan 17, 2020

@danvk I would be interested to use your solution within Angular, but I cannot omit compilation error:

Could not find a declaration file for module 'mapbox-gl/dist/style-spec'. 
'[...]/node_modules/mapbox-gl/dist/style-spec/index.js' implicitly has an 'any' type. 
If the 'mapbox-gl' package actually exposes this module, consider sending a pull request to amend 
'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/mapbox-gl`

Seems having @types/mapbox-gl installed it interferes.

@danvk
Copy link

danvk commented Jan 17, 2020

@marlag that error makes it sound like you haven't put style-spec.d.ts from the gist in a place where tsc can find it.

@marlag
Copy link

marlag commented Jan 18, 2020

@danvk Hmm, what I can say... blindness (as I wanted use only parsing part I even haven't looked on other files). Working fine now, thx a lot.

@rognstad
Copy link

rognstad commented Apr 2, 2020

I think a feature like this would be a huge boon to developers. I'm honestly surprised that more users of mapbox-gl haven't pushed harder for debugging or dev tooling around this. As a new user of Mapbox, the expressions are powerful but really hard to read. I find myself doing a lot of trial an error (requiring a re-render of the whole app to test) many permutations of rules until I reach a result that works or conclude it is impossible. When somethings fails, it is often difficult to know what caused the error. Seeing the result of expression evaluation against a particular feature/layer combo would be much nicer.

I'll give some of the suggestions earlier in this thread a try.

@stevage
Copy link
Contributor

stevage commented Mar 17, 2021

@danvk I have packaged up your Gist and released it as an NPM package mapbox-expression. Thanks so much for sharing that.

So you can do:

import Expression from 'mapbox-expression';

const feature = {
    type: 'Feature',
    properties: {
        name: 'Jan'
    },
    geometry: null
};

Expression.parse(['concat', 'Hello, ', ['get', 'name']]).evaluate(feature);
// 'Hello, Jan'

Expression.parse(['interpolate', ['linear'], ['zoom'], 10, 3, 15, 8]).evaluate(feature, { zoom: 12 })
// 5

@froghilda
Copy link

@stevage Many thanks for creating this package, it seems to be exactly the thing we need to present a dynamic legend for our point layer radiuses (which are rendered differently at different zoom levels via an expression that linearly interpolates between max- and minzoom). We are trying to import and use the package in a Vue.js app, but we're faced with this error from npm when compiling: "export 'default' (imported as 'Expression') was not found in 'mapbox-expression". Perhaps it is something very simple that we have overlooked, but any help to get this to work would be greatly appreciated.

@rreusser
Copy link
Contributor

@froghilda I think perhaps the required syntax is import { Expression } from '...' (note the curly brackets). Be aware that the module also has a peer dependency in mapbox-gl ^1.0.0, which may or may not match your needs. You can also see @mourner's standalone expression evaluation example in this observable notebook.

@froghilda
Copy link

froghilda commented Aug 19, 2021

@rreusser Thanks for your reply! It was the syntax for how we import it, curly brackets seemed to have fixed the error when compiling with npm, big thanks!!

@danvk
Copy link

danvk commented Aug 19, 2021

@froghilda With a recent Mapbox GL update (I forget exactly which one) I had to update my import to use the CommonJS module, maybe this will help

import {expression} from 'mapbox-gl/dist/style-spec/index.cjs';
const expr = expression.createExpression(...);

@radoslavirha
Copy link

Hi,

is there a method which decides if expression is a Data expression? It must check for operators like 'get',... from docs.

I don't know whole syntax and depth of expression, so I don't want to create such a method myself.

something like:

isDataExpression(expression): boolean => {}

Thanks

@rbrundritt
Copy link

Hi,

is there a method which decides if expression is a Data expression? It must check for operators like 'get',... from docs.

I don't know whole syntax and depth of expression, so I don't want to create such a method myself.

something like:

isDataExpression(expression): boolean => {}

Thanks

Generally, data driven styles are arrays, so you can just use Array.isArray(styleValue);. The few styles that use arrays, like line dash array, don't support data driven styles.

@radoslavirha
Copy link

Yeah, I know it's simple to determine an expression, but I want to determine type of an expression. If it's using operators like get, has,... from docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests