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

Expression engine extension points #9462

Open
brncsk opened this issue Mar 25, 2020 · 6 comments
Open

Expression engine extension points #9462

brncsk opened this issue Mar 25, 2020 · 6 comments

Comments

@brncsk
Copy link
Contributor

brncsk commented Mar 25, 2020

Motivation

I repeatedly find myself forking and extending mapbox-gl for various projects where the functionality of the readily available expressions feels lacking. This mostly involves adding ad-hoc functions to the second argument of CompoundExpression.register() in definitions/index.js.

Past use cases include:

  • Using some expression that was unavailable at the time (mostly the in expression – this was implemented since)
  • Implementing some performance-critical, project-specific functionality – some of which could have been done using the LISP-like syntax as well, but was more performant using Javascript.

For this reason, I propose a way for end users to be able to extend the list of built-in expression functions by supplying their own Javascript (or C++, in the case of gl-native) implementations. This would allow the user to use the full potential of the underlying Javascript runtime to specify feature styles during render time.

Similar functionality is available in mainstream desktop GIS software as well, see:

...and also in OpenLayers:

(Please note that this is orthogonal to #7010 which proposes a way to define first-class functions inside the expression language.)

Design Alternatives

  • Do nothing: as noted above, forking mapbox-gl is always an option for those who want to do this.

Design

I do realize that you probably consciously chose not to implement this in the past, although I failed to find a write up on the exact reasons (been reading discussions in the old mapbox-gl-function repo etc.).

Mock-Up

This is modelled after how expression functions are defined in definitions/index.js.

Note that I'm using the type names defined in compound_expression.js

map.registerExpression(
  'my-function', // Function name
  StringType, // Type
  [ValueType], // Signature
  (evaluationContext, args) => {
     ...
  } // Evaluate
);

Concepts

Another thing that makes this potentially unfeasible is that this exposes a large number of concepts, classes and types internal to mapbox-gl, such as EvaluationContext, the above mentioned types in compound_expression.js etc.

@ryanhamley
Copy link
Contributor

Thanks for the detailed write up @brncsk! I think my initial reaction to this is that it's likely to make GL JS significantly more complex and lead to quite a few issues as developers attempt to implement their own expressions. Implementing expressions outside of the most simple ones such as * and + can often be a laborious task and there can be subtle bugs introduced. I think it could become very easy for developers to break their map or get unexpected results. We get a lot of feedback already that expressions can be confusing and we need more examples of them to make them clearer.

Additionally, there's a lot of existing infrastructure that may be difficult to tap into with custom expressions. For example, what's the best way to add a custom expression that uses coalesce or coercion?

Using some expression that was unavailable at the time (mostly the in expression – this was implemented since)

I'm curious what other expressions you find yourself frequently wanting to implement. We're always willing to consider external PRs adding new expressions if you're interested in contributing an expression that we don't currently have. It might also be helpful to start crowd-sourcing a list of expressions people need/want. It seems like a decent portion of your use case for custom expressions was fulfilled by implementing the in expression so if we could implement other common expressions to help meet the demand, that might be preferable to adding an interface to an already confusing feature (and we're working on ways to clarify and demystify this area of the library, as well). I think it may be likely that future expressions will tend to be more complex than existing ones as many of the simplest expressions get implemented, which would make custom expressions harder and harder to implement.

All of this isn't to say this can't happen. But I think we'll need to have some discussion on the best way to do this, if we ever decide to go in this direction. For the time being though, I think we need to expend some energy on making expressions as clear to use as possible for developers before we can open up the ability to add custom ones.

@asheemmamoowala
Copy link
Contributor

cc @ericfischer This is similar to what we're discussing separately.

@asheemmamoowala
Copy link
Contributor

#6484 tracks a master list of expressions pending implementation.

@e-n-f
Copy link
Contributor

e-n-f commented Mar 26, 2020

Thanks @asheemmamoowala, yes, this is exactly what I was hoping to do too. The specific custom operators I am wanting to add (as a client of just mapbox-gl-style-spec, not the full mapbox-gl-js) are:

  • Generate a string representation of the feature geometry
  • Hash a string to a number
  • Generate a random number

@stevage
Copy link
Contributor

stevage commented Apr 7, 2020

@brncsk Out of curiosity, do you think it would be possible to write that registerFunction function as a plug-in module? That could be a pretty convenient tool for various edge cases and hacky situations, even if Mapbox never end up officially supporting it.

@DevelWolf
Copy link

A long time ago, and i don't know how far Mapbox and MapLibre have moved away from each other since then.
Nevertheless: I have patched support for custom (native) expressions into the current version of maplibre-gl (not as fork, but directly into maplibre-gl.js).

Maybe someone wants to take the concept and the few lines of code into a fork.

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

6 participants