7b72d5a18ab92129692e97a76a153fe0

This function returns a jQuery object containing all descendent text nodes (and <br/> line breaks) within the context node. I'm looking to make this function as fast as possible, since I need to call it very frequently.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
$.fn.textNodes = function() {
  var ret = [];
  this.each( function() {
    var fn = arguments.callee;
    $(this).contents().each( function() {
      if ( this.nodeType == 3 || $.nodeName(this, "br") ) 
        ret.push( this );
      else fn.apply( $(this) );
    });
  });
  return $(ret);
}

// example: wrap all text nodes in a span
$("body").textNodes().wrap("<span/>");

Refactorings

No refactoring yet !

Ee251c8e33a6ee0920d6bf43c5d0f541

jp.stacey

June 29, 2008, June 29, 2008 21:32, permalink

1 rating. Login to rate!

From what you say, you only need the descendants, not the starting node(s). So you can get it a bit smaller (and a little bit quicker) by removing one of your two function() blocks. How does this look to you?

You might get even quicker by using this.childNodes (which is what jQuery's contents() function uses internally), as long as you don't need to worry about iframes.

1
2
3
4
5
6
7
8
9
10
$.fn.textNodes = function() {
  var ret = [];
  this.contents().each( function() {
    var fn = arguments.callee;
      if ( this.nodeType == 3 || $.nodeName(this, "br") ) 
        ret.push( this );
      else $(this).contents().each(fn);
  });
  return $(ret);
}
5a00a3a98dcf6f9cd717440fd2b606e5

Eineki

June 30, 2008, June 30, 2008 10:43, permalink

1 rating. Login to rate!

Let me say that I'm rather new to jquery and my suggestions are probably naif.
Why don't you use something like this?
It is too slow?

1
2
3
$.fn.textNodes = function() {
   return this.find(*).filter(function(index) { return (this.nodeType==3)||(this.nodeName="br")}).andSelf();
}
7b72d5a18ab92129692e97a76a153fe0

jed

July 8, 2008, July 08, 2008 16:27, permalink

No rating. Login to rate!

@jp.stacey

Good catch. I did some informal profiling, and here's what I got on average:

(1) my original code: ~4100ms
(2) your refactor: ~2800ms
(3) your refactor w/ childNodes and no arguments.callee variable: ~2600ms

Not too shabby, thanks for your help!

@Eineki

Unfortunately, this.find(*) does not return text nodes.

1
2
3
4
5
6
7
8
9
$.fn.textNodes = function() {
  var ret = [];
  $.each(this[0].childNodes, function() {
      if ( this.nodeType == 3 || $.nodeName(this, "br") ) 
        ret.push( this );
      else $.each(this.childNodes, arguments.callee);
  });
  return $(ret);
}
915f797ee0f6ca0cf98c6b8565aa5fc7

Artur Honzawa

August 25, 2008, August 25, 2008 16:52, permalink

No rating. Login to rate!

Care to profile this version? I've skipped the jQuery library altogether, should be faster.

1
2
3
4
5
6
7
8
9
10
11
12
13
$.fn.textNodes = function() {
    var ret = [];

    (function(el){
        if (!el) return;
        if ((el.nodeType == 3)||(el.nodeName =="BR"))
            ret.push(el);
        else
            for (var i=0; i < el.childNodes.length; ++i)
                arguments.callee(el.childNodes[i]);
    })(this[0]);
    return $(ret);
}
Avatar

Artur Honzawa

August 25, 2008, August 25, 2008 17:18, permalink

No rating. Login to rate!

Oops. The "if (!el) return;" is not necessary.

Your refactoring





Format Copy from initial code

or Cancel