'in' Filters with too many elements fail with "Uncaught RangeError"

[peckjon]

Fixes mapbox/mapbox-gl-js#1782

If too many elements (varies by environment, but appx 1-2k) are used for an 'in' filter, it will fail with "Uncaught RangeError: Maximum call stack size exceeded". This is because the logical comparison of many infix comarisons (eg "(p['foo']==='bar') || (p['foo']==='cat') || ...") becomes too long for the v8natives to handle.

Note: this fix does not handle large lists of VectorTileFeatureTypes.

[jfirebaugh]

How about something like:

'[' + Array.prototype.slice.call(arguments, 2).map(function(value) {
  return '(' + operators['=='](_, key, value) + ')';
}).join(',') + '].indexOf(true) >= 0'

That would avoid the large disjunction for all cases. What it sacrifices in order to get that is short-circuit evaluation.

[peckjon]

@jfirebaugh Love it! Passes my local tests (about 2k elements). Committed for Pull.

[jfirebaugh]

👍 Can you add a test that triggers the RangeError prior to this change?

[jfirebaugh]

Thinking about this more, we should preserve short-circuit evaluation too.

'(function(){' + Array.prototype.slice.call(arguments, 2).map(function(value) {
  return 'if (' + operators['=='](_, key, value) + ') return true;';
}).join('') + 'return false;})()';

[peckjon]

Sorry can you elaborate on the range error issue?

On Jan 12, 2016, at 17:30, John Firebaugh notifications@github.com wrote:

Can you add a test that triggers the RangeError prior to this change?

—
Reply to this email directly or view it on GitHub.

[jfirebaugh]

Please add a test for this change, so we can ensure it doesn't regress with future changes.

[peckjon]

Oh, sure. I should be able to get to both of these later tonight.

On Jan 12, 2016, at 17:46, John Firebaugh notifications@github.com wrote:

Please add a test for this change, so we can ensure it doesn't regress with future changes.

—
Reply to this email directly or view it on GitHub.

[peckjon]

@jfirebaugh ready to go

[jfirebaugh]

Thanks! Committed as 6254b4c. I'll make a release and update the dependency in mapbox-gl-js shortly.

[peckjon]

👍