[PATCH] [TEST] Element#match should grok selectors with commas

The patch I submitted supports any combination of simple selectors, selectors with commas, and custom selectors. All related tests are also included in the patch.

I created a slightly less complex version Element#match that only supports only a single selector with commas or custom selector (as suggested by Tobie here) and can upload that patch as well if there is enough interest.

In #11206, kangax proposes a different solution to this same problem. I'll upload benchmarks soon to compare the two methods.

Some benchmarks (goes into test/unit)

Good job on benchmarks, but it's clear that your patch does quite a lot of "heavy" lifting: support for multiple arguments ($A, then shift), #each then #detect (which uses #each once again), then split, concat, etc.

Here's what I get with your benchmarks:

testBenchmarkMatchSingleSelectorOriginal   0.36s
testBenchmarkMatchSingleSelectorMetavida   1.031s
testBenchmarkMatchSingleSelectorKangax	   0.394s

testBenchmarkMatchCustomOriginal	   0.06s
testBenchmarkMatchCustomMetavida	   0.259s
testBenchmarkMatchCustomKangax	           0.06s

testBenchmarkMatchMultipleFirstMetavida	   1.363s
testBenchmarkMatchMultipleFirstKangax	   0.628s

testBenchmarkMatchMultipleSecondMetavida   1.687s
testBenchmarkMatchMultipleSecondKangax	   1.142s

It's good to see the performance results being close to original ones.

Agreed, my implementation is definitely a bit heavier/slower in the base case (a single, simple selector). However, on my machine the results on more complicated selectors were quite different from yours. I have a Mac, running OS 10.4. With Firebug & Greasemonkey disabled I got the following results

                                          FF2     Safari  FF3b4
testBenchmarkMatchSingleSelectorOriginal  0.59s   0.321s  0.322s
testBenchmarkMatchSingleSelectorMetavida  0.782s  0.348s  0.362s
testBenchmarkMatchSingleSelectorKangax    0.788s  0.368s  0.391s

testBenchmarkMatchCustomOriginal          0.1s    0.046s  0.048s
testBenchmarkMatchCustomMetavida          0.237s  0.058s  0.063s
testBenchmarkMatchCustomKangax            0.1s    0.046s  0.048s

testBenchmarkMatchMultipleFirstMetavida   0.77s   0.384s  0.368s
testBenchmarkMatchMultipleFirstKangax     1.249s  0.63s   0.664s

testBenchmarkMatchMultipleSecondMetavida  1.376s  0.698s  0.673s
testBenchmarkMatchMultipleSecondKangax    1.79s   0.928s  1.009s

Again, the base cases are slower, but as the selector gets more complex my implementation pulled away. The results were similar on my PC in IE6.

Perhaps a mixed approach would be better. If a simple selector is detected (one argument & no commas) the current implementation is used. Otherwise the more complex implementation is triggered.

I'm sorry, but why multiple arguments?

I'm trying to emulate the behavior of Element#select and $$ as much as possible. For all selectors the following should return an array of true values:

$$(selector).collect(function(element) {
  return element.match(selector);
});

Multiple arguments are allowed for cases like this:

$$('#p a', 'ul#list li').collect(function(element) {
  return element.match('#p a', 'ul#list li');
});

$$('span', { match: function(element) {return true;}}).collect(function(element) {
  return element.match('span', { match: function(element) { return true }});
});

Granted, these are edge cases, but I think it's important that Element#select & Element#match are as similar as possible. Otherwise the differences in behavior can be quiet confusing.

Supporting commas and multiple arguments seems a little redundant, don't you think? Both have the same meaning - check if element matches against *ANY* of the expressions. $$ just happens to support both (I believe "commas" were added later on)

I'd rather remove multiple arguments from #match in favor of speed.

// both return same results
$$('div', 'span');
$$('div, span');

element.match('foo, bar', 'baz, qux, quux'); // <= why do this?
element.match('foo, bar, baz, qux, quux'); // <= if we could simply do this

I agree, multiple arguments does seem a bit redundant, and off the top of my head I'd have a hard time making a really strong case for it. My main concern is that it can be confusing that $$ and Element#match take different arguments. That said, I've got another implementation to propose:

  match: function(element, selector) {
    if (!(element = $(element))) return;
    return Object.isString(selector) 
      ? selector.split(',').detect(function(selector) {
        return (new Selector(selector)).match(element);
      })
      : selector.match(element);
  },

Running it through the benchmarks, it is only slightly slower than the original in testBenchmarkMatchSingleSelectorm runs at the same speed as testBenchmarkMatchCustom, and retains speed with comma-separated selectors tests as well.

The main beef I have with your implementation is that internally Selector.matchElements calls $$. Meanwhile Selector#match only searches the entire DOM tree if it absolutely has to, giving it a potential speed & memory footprint advantage. Additionally, using detect avoids unnecessarily comparing patterns. If the first pattern matches the method returns true without testing further.

Sorry, one more tweak (replace String#split with Selector.split):

match: function(element, selector) {
    if (!(element = $(element))) return;
    return Object.isString(selector) 
      ? Selector.split(selector).detect(function(selector) {
        return (new Selector(selector)).match(element);
      })
      : selector.match(element);
  },

Absolutely agree. I thought about $$ but the solution just seemed elegant and small. Well, now we just need to make sure boolean is always returned (using !!)

match: function(element, selector) {
  if (!(element = $(element))) return;
  return Object.isString(selector) 
    ? !!Selector.split(selector).find(function(selector) {
      return new Selector(selector).match(element);
    })
    : selector.match(element);
}

I was also playing with an "optimized" but a little more "cryptic" version:

match: function(element, selector) {
  if (!(element = $(element))) return;
  if (!Object.isString(selector))
    return selector.match(element);
  for (var i=0, arr = Selector.split(selector), l = arr.length; i<l; i++) {
    return new Selector(arr[i]).match(element);
  }
  return false;
}

Seems to be ~3 times faster : )

Opps, that should have been:

match: function(element, selector) {
  if (!(element = $(element))) return;
  if (!Object.isString(selector))
    return selector.match(element);
  for (var i=0, arr = Selector.split(selector), l = arr.length; i<l; i++) {
    if (new Selector(arr[i]).match(element)) return true;
  }
  return false;
}

I haven't had a chance to run it through benchmarks yet, but that last, "cryptic" implementation of yours looks the most promising to me. Does the 3x speed benefit come using a for loop instead of detect?

Of course. Optimized for(;;) is almost always much faster than any enumerable method (take a look at #map/#detect implementation which in turn rely on #each). Some critical parts of prototype use plain loops to squeeze the most out of performance, while enumerables are used where speed is not much of a bottleneck.

match implementation by kangax - http://dev.rubyonrails.org/ticket/10907#comment:13