Skip to content

/ learn.jquery.com

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

optimize-selectors qSA before Sizzle #694

Open
wants to merge 3 commits into
base: master
from
Open

optimize-selectors qSA before Sizzle #694

wants to merge 3 commits into from

Conversation

@mr21
Copy link

@mr21 mr21 commented Dec 31, 2015

Hi :)

Here a first draft (who probably contain few mistakes) for the issue #666.
@agcolom
Copy link
Member

@agcolom agcolom commented Jan 21, 2016

@dmethvin @timmywil It would be truly wonderful if you could review this PR. Thank you :-)
page/performance/optimize-selectors.md Outdated

Beginning your selector with an ID is always best.
```

This comment has been minimized.

Sign in to view
@AurelioDeRosa

AurelioDeRosa Jan 21, 2016
Member

I think this example might need a small introduction. Even 1-2 sentences should do it.
page/performance/optimize-selectors.md Outdated
$( "#container" ).find( "div.robotarm" );
## Try to segregate the nonstandard parts of your selection

When you select elements, jQuery will call `querySelectorAll` with your selection. If `querySelectorAll` throws an error, jQuery will refer to its Sizzle engine. So, if you are using at least one of these nonstandard pseudo-classes: `:contains()`, `:has`, `:even`, `:submit`, etc. You will not take advantage of the native `querySelectorAll`.

This comment has been minimized.

Sign in to view
@AurelioDeRosa

AurelioDeRosa Jan 21, 2016
Member

I think this sentence

if you are using at least one of these nonstandard pseudo-classes: :contains(), :has,

can be tweaked a bit

if you are using at least one of the nonstandard pseudo-classes such as :contains(), :has,
page/performance/optimize-selectors.md Outdated
```
// A long selection with nonstandard pseudo-classes inside:

This comment has been minimized.

Sign in to view
@AurelioDeRosa

AurelioDeRosa Jan 21, 2016
Member

Maybe we can remove the colon at the end of this comment and the next for consistency. This isn't very important, though.
@AurelioDeRosa
Copy link
Member

@AurelioDeRosa AurelioDeRosa commented Mar 13, 2016

Hi @mr21. Are you still interested in this PR?
@mr21
Copy link
Author

@mr21 mr21 commented Mar 14, 2016

... I'm really sorry, yes I'm still interested.
I'll push ASAP your change
@kswedberg
Copy link
Member

@kswedberg kswedberg commented Mar 14, 2016

@mr21 Make sure you rebase from here before continuing your work. I merged a PR based on another issue and removed/rearranged a bunch of stuff, so you might have conflicts.
@mr21
Copy link
Author

@mr21 mr21 commented Mar 14, 2016

Understood :)
@mr21
optimize-selectors qSA before Sizzle
Loading status checks…
cb2c81b
@mr21 mr21 force-pushed the mr21:optimize-selectors branch from 78fd9d4 to cb2c81b Mar 16, 2016
@mr21
Copy link
Author

@mr21 mr21 commented Mar 16, 2016

Hi :)

So, I have rebased and repush again my commit with the @AurelioDeRosa's modifications.
Except that I didn't add yet a small introduction in place of this removed sentence:
Beginning your selector with an ID is a safe bet.
@AurelioDeRosa
Copy link
Member

@AurelioDeRosa AurelioDeRosa commented Mar 16, 2016

Hi @mr21.

With "yet" you mean that you'll add another commit to update the PR?
@mr21
remove another old sentence
Loading status checks…
c895d3f
@mr21
Copy link
Author

@mr21 mr21 commented Mar 17, 2016

Hi :)

For the moment I have replaced this:

ID-Based Selectors

Beginning your selector with an ID is a safe bet.

// Fast:
$( "#container div.robotarm" );

// Super-fast:
$( "#container" ).find( "div.robotarm" );

With the first approach, jQuery queries the DOM using document.querySelectorAll(). With the second, jQuery uses document.getElementById(), which is faster, although the speed improvement may be diminished by the subsequent call to .find().

by this:

Save calls to querySelectorAll

querySelectorAll is already really fast, if you want maintain this speed try to call it the least possible.

// If in your HTML there are 2 .container with 5 div in each,
// this line will call querySelectorAll 13 times (1 + 2 + 2*5).
$( ".container" ).children( "div" ).find( ".robotarm" );

// Against only 1 call with this:
$( ".container div .robotarm" );

But I don't know what I have to write to introduce my example.
If you have an idea, I will commit it quickly.

Thanks for your patience :)
@mr21
A small introduction for + remove a semi colon
Loading status checks…
1216af9
@JSFOwner JSFOwner removed the CLA: Valid label Nov 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@mr21 @agcolom @AurelioDeRosa @kswedberg @jquerybot @JSFOwner