Traversing: use jQuery's push to avoid an unnecessary loop in .find #2370

Closed
timmywil opened this Issue Jun 1, 2015 · 3 comments

Projects

None yet

3 participants

@timmywil
Member
timmywil commented Jun 1, 2015
@timmywil timmywil added the Traversing label Jun 1, 2015
@timmywil timmywil self-assigned this Jun 1, 2015
@timmywil timmywil added this to the 3.0.0 milestone Jun 1, 2015
@gibson042
Member

@timmywil I think we need a jsPerf verifying performance impact before committing to this. I could easily see the improvement from doing less work in pushStack being overwhelmed by using Array methods on non-Arrays, and there's no reason to change back if it actually slows us down.

@timmywil
Member
timmywil commented Jun 2, 2015

Fair enough. I'll do a perf.
On Tue, Jun 2, 2015 at 08:07 Richard Gibson notifications@github.com
wrote:

@timmywil https://github.com/timmywil I think we need a jsPerf
verifying performance impact before committing to this. I could easily see
the improvement from doing less work in pushStack being overwhelmed by
using Array methods on non-Arrays, and there's no reason to change back if
it actually slows us down.


Reply to this email directly or view it on GitHub
#2370 (comment).

@mgol
Member
mgol commented Sep 14, 2015

@timmywil This seems like sth that can be done for 3.0.1. Please revert the milestone change if you disagree.

@mgol mgol modified the milestone: 3.0.1, 3.0.0 Sep 14, 2015
@timmywil timmywil pushed a commit to timmywil/jquery that referenced this issue Jan 19, 2016
Timmy Willison Traversing: restore jQuery push behavior in .find
Fixes gh-2370
439c37f
@timmywil timmywil modified the milestone: 3.0.0, 3.0.1 Jan 19, 2016
@timmywil timmywil pushed a commit that closed this issue Jan 20, 2016
Timmy Willison Traversing: restore jQuery push behavior in .find
Fixes gh-2370
Close gh-2848
4d3050b
@timmywil timmywil closed this in 4d3050b Jan 20, 2016
@mgol mgol removed the Has Pull Request label Mar 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment