-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Accessing nested properties in flat style definitions #15505
Comments
Hi! In the Maplibre style spec which is the main inspiration for the operators in OpenLayers expressions, the Note that the Would you be willing to provide a PR if that solution fits your needs? I'd be happy to provide guidance the best I can. |
I agree it makes sense to support deeply nested feature property values. I'm not sure that the I think the most natural / guessable syntax would be for the So for a feature with properties like {some: {deeply: {nested: {value: 42}}}} the expression // my ideal syntax
['get', 'some', 'deeply', 'nested', 'value'] would evaluate to If instead we followed the Mapbox style convention and accepted an optional object as the second argument to the // following mapbox
['get', 'value', ['get', 'nested', ['get', 'deeply', ['get', 'some']]]] Both of these approaches conflict with the current signature for I'm not clear on what the proposed If we want to stick close to the Mapbox style spec, then accepting an object as the optional second argument to the The simpler alternative to implement would be the |
Hmm, I still like the Mapbox approach quite a bit, it might be more verbose but it also makes things more explicit. This would also let your read from an array in an object using About the type hint, once type assertions are available in the gpu compiler we can very likely get rid of them, yes! |
I agree with @tschaub that the I really like the variadic API @tschaub suggests. It feels to me like very clean and intuitive API design. I'm not too keen on the verbose nature of I'm not quite sure what the type hint argument does. Is there any documentation I could read? I'd be more than happy to help refactor something to get rid of it! |
The type hint is there because the |
The thing I'm not clear about with the type hint is where it is actually required. I understand how it works currently, but am uncertain where it is required. When parsing, we start with the top-most expression and we can know what output types are expected. For example, when parsing an expression for I know that the current implementation might need the type hints. But I'm still unclear if they are really "required" (if we change the current implementation or if we tighten up the syntax/operators). |
I'm strongly in favor of this simpler alternative and the resulting breaking change. If we do that, we also don't need an {"answer": {"at": {"index": [17, 42]}}} we could get the correct answer with ["get", "answer" , "at", "index", "1"] @jahow Is this something you could live with for the WebGL implementation? If a type hint is needed, wouldn't ["number", ["get", "answer" , "at", "index", "1"]] be a good way to do that? At least that's what Mapbox does. |
Type assertions can definitely replace the type hint of the
I think at some point this ended up being necessary because a property could accept multiple types and just feeding it a "get" expression was not resolving to a single type. In general I think we cannot exclude this to happen, but if we rely on assertions then it's fine and we don't need the type hint anymore. |
Is your feature request related to a problem? Please describe.
We're using OpenLayers extensively in our project. First of all: thank you for this amazing library!
At the moment we're exploring options where our users may be able to customize or define their own styles for layers. We're looking into defining all our available layers, as well as custom data, in a database, and sending flat styles over the wire from our API backend instead of hardcoding
new Style(...)
in code.Things are looking promising, but we have quite a few layers with properties which are actually arbitrarily deeply nested objects. For instance:
Say we want to use
address.province
in afilter
expression, or aget
expression. As far as I can tell, this is currently not supported, and only expressions wherefeature.get('key')
which return a JS literal can be used.Describe the solution you'd like
Hopefully I'm wrong and this is possible. If not, I've been diving into the code in
src/ol/expression.js
, and I propose a newnested
expression. Given the previousaddress
feature, something like:Or a bit more formally:
Op.Nested
, when walked down the JS object path, should return aLiteralExpression
(I think). This should also work out of the box in any other expressions or operators where a literal is expected.Please let me know if this is something you may wish to pursue. I think, with a bit more time spent browsing through the code base in
expression.js
andcpu.js
, I should be able to implement this and file a Pull Request.The text was updated successfully, but these errors were encountered: