Removed global / feature evaluation and property evaluation

[lucaswoj]

With great humility & hindsight, I revisit some of the first code I wrote at Mapbox to simplify the API and improve perf

• remove the global / feature nested evaluation (i.e. f(global)(feature)) in favor of a single evaluation (i.e. f(input))
• remove property object evaluation (i.e. f({$zoom: zoom})) in favor of simple evaluation (i.e. f(zoom))
• remove assert function (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

[jfirebaugh]

• remove the global / feature nested evaluation (i.e. f(global)(feature)) in favor of a single evaluation (i.e. f(input))
• remove property object evaluation (i.e. f({$zoom: zoom})) in favor of simple evaluation (i.e. f(zoom))

So this would drop support for data-driven-style functions? Won't we need to revert these changes once we need DDS support again?

[lucaswoj]

• remove the global / feature nested evaluation (i.e. f(global)(feature)) in favor of a single evaluation (i.e. f(input))

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 property === '$zoom' || property === undefined.

If we want to support for multi-property functions later, we can revisit this interface.

• remove property object evaluation (i.e. f({$zoom: zoom})) in favor of simple evaluation (i.e. f(zoom))

I don't feel strongly about this change. It is one line of code. We can put that line in mapbox-gl-function or in mapbox-gl-js. Given the branching strategy mentioned above, I think putting that line in GL JS would be cleanest.

---

This PR is blocked on updating the style spec for the moment being.

[jfirebaugh]

The double evaluation is a nice conceptual model of how the algorithm works but doesn't jive with the actual code path.

Ok, sounds good then.

Given the branching strategy mentioned above, I think putting that line in GL JS would be cleanest.

If the interface is f(zoom), then it seems like use of mapbox-gl-function requires independent knowledge of the JSON structure for functions, so that the desired feature property or special property can be extracted, and the right value passed to f. That's contrary to one of the design goals of mapbox-gl-function: abstracting the function format, such that mapbox-gl-function is the only code that needs to know about it.

I.e., the interface should be either f(globals)(feature), or f(globals, feature) (not f(extend({}, feature, globals)) because that's inefficient).

Or am I missing something?

[lucaswoj]

abstracting the function format, such that mapbox-gl-function is the only code that needs to know about it.

Right! Sorry. The latest commit

• changes the interface of functions to f(globals, feature) (note: one of the two arguments will always be null)
• restores isGlobalConstant and isFeatureConstant properties to the functions

So the net change of this PR is from f({})({}) to f({}, {})

[jfirebaugh]

👍 😄