Conversation
So this would drop support for data-driven-style functions? Won't we need to revert these changes once we need DDS support again? |
The double evaluation is a nice conceptual model of how the algorithm works but doesn't jive with the actual code path. We wouldn't iterate over all the features for a "feature constant" function so there's no point in creating a closure for that case. The simplest thing for now is to branch on If we want to support for multi-property functions later, we can revisit this interface.
I don't feel strongly about this change. It is one line of code. We can put that line in This PR is blocked on updating the style spec for the moment being. |
Ok, sounds good then.
If the interface is I.e., the interface should be either Or am I missing something? |
Right! Sorry. The latest commit
So the net change of this PR is from |
|
👍 😄 |
- changes the interface of functions to `f(globals, feature)` - removes the "assert" function - improves perf - pins to an updated style spec - bumps to the v2.1.0
With great humility & hindsight, I revisit some of the first code I wrote at Mapbox to simplify the API and improve perf
f(global)(feature)) in favor of a single evaluation (i.e.f(input))f({$zoom: zoom})) in favor of simple evaluation (i.e.f(zoom))assertfunction (bad for perf and causes mapbox-gl-js tests to fail)These are breaking API changes but nobody is using 2.0 in the wild, so I vote we release this as version 2.1 once the dust settles.
cc @jfirebaugh @ansis