DataView filtering: compileFilter() very sensitive to custom filter function structure

[asavoy]

Note: This is similar to issue #243 which I reported, but I feel it warrants a separate issue.

Background

We're using SlickGrid (it's awesome), and recently updated to latest. We found that after running our build scripts, which compress our JS files using Google Closure Compiler, our grids that used DataView filtering all broke.

Cause

The problem is with the general approach that compileFilter() and compileFilterWithCaching() use; that is, using Function.toString() to fetch original source code for fnTemplate and the user's filter function, to generate a new function.

The code makes some pretty strong expectations about the style/structure of the user's filter function. It's very easy to get it wrong. Worse still, it's easy to get it right, but have it completely break after JS compression.

Here's an example replacement from slick.dataview.js:

...
.replace(/return true;/gi, "{ _cache[_i] = true;_retval[_idx++] = $item$; continue _coreloop; }")
...

Here's my custom filter function:

var myFilter = function(item, args) {
    if (item.completed) {
        return false;
    }

    return true;
}

After compression, compiler optimisations will change the code. I was affected by semicolons being stripped if they are the last line in a code block. (actually, whitespace is also stripped, but not important to show for this case):

var myFilter = function(item, args) {
    if (item.completed) {
        return false
    }

    return true
}

Which no longer match for replacement. At that point, we're in undefined territory and weird errors start coming up.

Another thing, JS compilers might go a further and shorten the above even more:

var myFilter = function(i){return !i.completed}

Also; it's not clear, but because of this dynamic function generation, you can't use a filter function that relies on a closure, since that doesn't persist to the generated function. (Hence the args argument, I suppose)

Workaround

As a fix for the meantime, I rewrote my filter functions using strings, since this prevents them from being modified by a JS compressor. Unfortunately, it's very ugly:

var myFilter = new Function("item", "args", [
    "if (item.completed) { ",
        "return false; ",
    "} ",
    "return true; "
].join(""));

Furthermore, there isn't any workaround offered by Google Closure Compiler; there is no way to ignore or "not-compress" the code, whether at the function level or at the file level.

[slubowsky]

And as I commented on issue #243, same problem apparently occurs with formatters

[mleibman]

thank you for the awesome in-depth bug report!
i wish they all were like that.

[mleibman]

Fixed in 58a1156.
This doesn't address all the valid syntax variations, but will work for this particular issue.

[dacap]

I think that the commit which fixes this issue is this one 5b6e673
Thanks!

[asavoy]

Perhaps this issue and its workaround could be documented somewhere in the Wiki?

I feel like this hard-won knowledge might really help users in the future; as others unaware of this issue could fall into the same trap when they implement their own custom filters (or formatters), even if SlickGrid's own filtering is now fixed.

[mleibman]

I've now made this behavior optional via the new DataView "inlineFilters" option.