[select] use for loop instead of `push.apply`
Open
Nobody has claimed this yet.
Has a jQuery counterpart
- Dominant language
- JavaScript
- Stars
- 6.3k
- Forks
- 927
- PR merge metrics
- No merged PRs in 30d
Description
Migrated from https://github.com/jquery/jquery/issues/3704
We decided in the meeting to try a for loop instead of push.apply. Check perf to be sure, but for loops are so heavily optimized these days that it might even be better. It has the added advantage of not causing "stack limit exceeded" errors.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Locate the Sizzle code that uses push.apply and review the surrounding selector-engine logic. Compare the proposed for-loop approach for performance, then confirm the relevant path no longer risks stack-limit errors and that existing tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- performance, web-dev
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100