3

The title of this post reads as webdev-hipster as a skinny flannel scarf at an alleycat race. Sorry.

I'm not great with script runtime optimization, so I'm wondering just how bad computationally the following function call is going to be. I know it wouldn't be practical for a big site, but where I wan to use it, the jQuery call is going to return no more than a half dozen objects, so the volume isn't high.

 Modernizr.load({
    test: Modernizr.borderradius && Modernizr.boxshadow,
    nope: "_/js/polyfills/pie.js",
    complete: function(){
        if(window.PIE){
            $('*').css('box-shadow').each(function(){ PIE.attach(this); });
            $('*').css('border-radius').each(function(){ PIE.attach(this); });
        }
    }
 });

Thanks everyone.

Evan
  • 419
  • 4
  • 14
  • 1
    What are you trying to do? Your code has a bug `.css('box-shadow')` returns a string and you are using it as jQuery object. – ShankarSangoli Feb 15 '12 at 18:11
  • @Shankar You are correct. But it's not exactly a bug: I didn't look closely enough at $.css() - I thought it worked more like $.hasClass(). Now I see that I'll need a more convoluted jQuery statement to filter the results. – Evan Feb 15 '12 at 19:32

2 Answers2

1

Try this.

 Modernizr.load({
    test: Modernizr.borderradius && Modernizr.boxshadow,
    nope: "_/js/polyfills/pie.js",
    complete: function(){
        if(window.PIE){
            $('*').each(function(){
                var $this = $(this);
                //check if box-shadow or border-radius is applied
                if($this.css('box-shadow') || $this.css('border-radius')){
                    PIE.attach(this);
                }
            });
        }
    }
 });
ShankarSangoli
  • 69,612
  • 13
  • 93
  • 124
  • @Evan - Based on your comment I have this solution for you hope it helps you. – ShankarSangoli Feb 15 '12 at 19:36
  • This is perfect! When you submitted it I was actually here typing out essentially the same solution to 'answer' myself, but turns out I can't do that with low rank anyway. Kudos to you friend! – Evan Feb 15 '12 at 19:51
  • Because I am using it at two places so cached it. If you use $(this) also it will work. – ShankarSangoli Feb 15 '12 at 19:54
  • 2
    I would suggest instead of using `*` as root selector if you know specific which part of the page you are targeting use that section or container as root selector. – ShankarSangoli Feb 15 '12 at 19:56
  • I guess so. I know that `$('*')` is a big stick, which prompted the question the first place, but I guess the filtering isn't so bad eh? Naturally `$('body')` will cut down 1/3 of the selection and CSS is only going to be applied with that part of the document. Thanks for your help with this. – Evan Feb 15 '12 at 20:03
  • `$("*")` is almost always a bad idea, *much* better to use a recursive descent function than a huge flat array. Also, note that `css` will return the *computed* value of that property. There's no guarantee that a browser that doesn't support the property will return it from `getComputedStyle` or similar. – T.J. Crowder Feb 15 '12 at 21:41
  • For more on why caching local variables (when targeting IE) is **always** a good idea, see: [IE + JavaScript Performance Recommendations @ MSDN](http://blogs.msdn.com/b/ie/archive/2006/08/28/728654.aspx) – andykram Jun 14 '12 at 22:08
0

...the jQuery call is going to return no more than a half dozen objects...

So half a dozen is six. Four of those will be html, head, script, and body. :-) You only have two other elements on the page?

Seriously, if the number is very low, it doesn't matter. You'd want to limit the $() call to only the elements that really need it, though, rather than $("*") which is a big hammer.

If you really need to run through the whole document, use a simple recursive-descent function:

function applyPie(element) {
    var node;
    for (node = element.firstChild; node; node = node.nextSibling) {
        if (node.nodeType === 1) { // 1 = element
            node.style.boxShadow = /* ...?... there's nothing in your jQuery call */;
            node.style.borderRadius = /* ...?... there's nothing in your jQuery call */;
            PIE.attach(node);
            applyPie(node);
        }
    }
}

applyPie(document.documentElement);

That calls PIE.attach on every element except the documentElement. You might use nodeName (or tagName) so you don't attach PIE to html, head, style, and such. Using a simple recursive-descent function avoids creating large flat arrays in memory, which is what $("*") does.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • There are many more elements in the DOM of these pages. By 'jQuery call' I meant the results as filtered by the .css() function. My logic was that jQuery could select only those elements on which I have applied certain CSS3 rules, allowing me to attach PIE conservatively. WRT your recommendation, it would certainly work, but I feel like there is a more efficient way to let jQuery selectors do the work. – Evan Feb 15 '12 at 20:04
  • @Evan: What you did wouldn't filter the results based on those properties (as you now know from Shankar). Beware your accepted answer, it creates a flat array of **all** elements on your page (`$("*")`) and loops through it; that could be a *very* big array. Also, it then uses `css` to figure out where you've applied those properties, and yet as `css` under-the-covers uses `getComputedStyle` and such, there's no guarantee it will return anything when the browser doesn't support it. At the very least, it would be better to use a recursive-descent function rather than `$("*")`. – T.J. Crowder Feb 15 '12 at 21:39
  • Thanks for your input T.J. I'm going to bet on jQuery's cross-browser performance though. For the ease of maintenance and limited number of objects we're dealing with, I think Shankar's solution is tops. I'll keep your technique in mind though if I have to do this on a bigger site, it does have a performance edge, especially since it's done with core api! Thanks again. – Evan Feb 15 '12 at 21:50