perf(css): bucket .pagination universal * subject by child tag#3079
Conversation
🦋 Changeset detectedLatest commit: 1c684c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
* subject by child tag
There was a problem hiding this comment.
✅ Ready to approve
The selector changes are narrowly scoped, preserve the intended responsive cascade via specificity/order, and align with the component’s existing a/span/em child styling.
Note: this review does not count toward required approvals for merging.
Pull request overview
This PR improves selector-matching performance in the pagination component by replacing unbucketable universal-child selectors (.pagination > *) with tag-bucketed selectors so Blink can more quickly reject non-matching elements during style recalculation.
Changes:
- Replaced
.pagination > *responsive hide/show rules with> a, > span, > emequivalents insrc/pagination/pagination.scss. - Added a patch changeset documenting the performance-motivated selector change.
File summaries
| File | Description |
|---|---|
src/pagination/pagination.scss |
Replaces universal direct-child selectors with tag-bucketed a/span/em selectors for improved Blink matching performance. |
.changeset/pagination-universal-subject-perf.md |
Adds a patch changeset explaining the selector change and why it improves performance without changing output. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What are you trying to accomplish?
Primer-owned selectors with a universal subject compound (
*/:not([attr])) can't be bucketed by Blink, so they run the full matcher against (nearly) every element on every style recalc — pure wasted work. In@primer/css, the only such offender is.pagination > *(the responsive show/hide rules).What approach did you choose and why?
src/pagination/pagination.scss— replaced the two.pagination > *rules with tag-bucketed selectors so the engine can fast-reject:Output is unchanged: the overriding show-rules (
> :first-child,> .current, …) keep higher specificity(0,2,0)than the new hide rule(0,1,1), so the responsive cascade is preserved.Added a changeset (patch).
Scope notes:
.autocomplete-item,.Popover-message,.breadcrumb-item,.ActionListItem*,.FormControl-*,.prc-*) live in@primer/view-components/@primer/react, not here.:is()list-merge root cause does not apply to@primer/css: SCSS emits expanded per-selector rules, and the compileddist/primer.csscontains zero:is(.What should reviewers focus on?
.paginationdirect children are alwaysa/span/em(consistent with the existinga, span, emstyling block). Any other child type would no longer be hidden by the default rule.> :first-child,> :nth-child(2), …) were intentionally left unchanged — they aren't*/:not([attr])subjects per the issue, and rewriting them is invasive without addressing the called-out root causes.Can these changes ship as is?