Skip to content

perf: add .skip(1) to .ancestors() calls in a bunch of places#9330

Merged
dyc3 merged 1 commit intomainfrom
dyc3/perf-skip-ancestors
Mar 4, 2026
Merged

perf: add .skip(1) to .ancestors() calls in a bunch of places#9330
dyc3 merged 1 commit intomainfrom
dyc3/perf-skip-ancestors

Conversation

@dyc3
Copy link
Contributor

@dyc3 dyc3 commented Mar 4, 2026

Summary

There were a bunch of places where we were calling .ancestors() and not skipping the first in the iterator, which is the current node and not a parent.

I think ideally we would provide a separate iterator that doesn't have the current node in the first place.

No changesets because the perf improvement probably won't impact most people.

Test Plan

snapshots

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 4, 2026

⚠️ No Changeset found

Latest commit: 67d018c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS and super languages L-HTML Language: HTML and super languages labels Mar 4, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9270bb9d-dd64-40fa-94a4-c3a9729a9b0b

📥 Commits

Reviewing files that changed from the base of the PR and between 95914f5 and 67d018c.

📒 Files selected for processing (29)
  • crates/biome_css_analyze/src/lint/a11y/use_generic_font_names.rs
  • crates/biome_css_analyze/src/lint/correctness/no_unknown_unit.rs
  • crates/biome_css_analyze/src/lint/correctness/no_unmatchable_anb_selector.rs
  • crates/biome_css_analyze/src/lint/suspicious/no_duplicate_properties.rs
  • crates/biome_html_analyze/src/lint/a11y/no_autofocus.rs
  • crates/biome_html_analyze/src/lint/nursery/use_vue_valid_v_if.rs
  • crates/biome_js_analyze/src/assist/source/use_sorted_attributes.rs
  • crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
  • crates/biome_js_analyze/src/frameworks/mod.rs
  • crates/biome_js_analyze/src/frameworks/vue/vue_component.rs
  • crates/biome_js_analyze/src/lint/a11y/use_aria_activedescendant_with_tabindex.rs
  • crates/biome_js_analyze/src/lint/complexity/no_this_in_static.rs
  • crates/biome_js_analyze/src/lint/complexity/no_useless_constructor.rs
  • crates/biome_js_analyze/src/lint/complexity/no_useless_label.rs
  • crates/biome_js_analyze/src/lint/complexity/no_useless_this_alias.rs
  • crates/biome_js_analyze/src/lint/correctness/no_constant_condition.rs
  • crates/biome_js_analyze/src/lint/correctness/no_constructor_return.rs
  • crates/biome_js_analyze/src/lint/correctness/no_invalid_constructor_super.rs
  • crates/biome_js_analyze/src/lint/correctness/no_nested_component_definitions.rs
  • crates/biome_js_analyze/src/lint/correctness/no_process_global.rs
  • crates/biome_js_analyze/src/lint/correctness/no_setter_return.rs
  • crates/biome_js_analyze/src/lint/correctness/no_solid_destructured_props.rs
  • crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs
  • crates/biome_js_analyze/src/lint/correctness/use_qwik_valid_lexical_scope.rs
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/src/lint/nursery/no_unnecessary_conditions.rs
  • crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rs
  • crates/biome_js_analyze/src/lint/performance/use_top_level_regex.rs
  • crates/biome_js_analyze/src/lint/suspicious/no_var.rs

Walkthrough

This PR systematically adds .skip(1) to ancestor traversal chains across multiple linting rules in CSS, HTML, and JavaScript analyzers. The change causes the immediate parent node to be skipped when searching for specific ancestor types such as at-rules, class declarations, and import statements. No function signatures are modified; only the ancestor iteration scope is adjusted.

Possibly related PRs

Suggested labels

A-Linter, L-JavaScript, L-CSS, L-HTML

Suggested reviewers

  • ematipico
  • arendjr
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarises the primary change—adding .skip(1) to multiple .ancestors() calls across the codebase for performance improvements.
Description check ✅ Passed The description is clearly related to the changeset, explaining the motivation (avoiding iterating over the current node), the implementation approach, and noting that tests use snapshots with no changeset included.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dyc3/perf-skip-ancestors

Comment @coderabbitai help to get the list of available commands and usage tips.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 4, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1


Comparing dyc3/perf-skip-ancestors (67d018c) with main (ed3c753)

Open in CodSpeed

Footnotes

  1. 156 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 4, 2026

Merging this PR will not alter performance

✅ 64 untouched benchmarks
⏩ 152 skipped benchmarks1


Comparing dyc3/perf-skip-ancestors (67d018c) with main (ed3c753)

Open in CodSpeed

Footnotes

  1. 152 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dyc3 dyc3 merged commit c3ec092 into main Mar 4, 2026
23 checks passed
@dyc3 dyc3 deleted the dyc3/perf-skip-ancestors branch March 4, 2026 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-CSS Language: CSS and super languages L-HTML Language: HTML and super languages L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants