perf(css): remove universal * subjects and expand merged :is() selector lists - #4095
Conversation
🦋 Changeset detectedLatest commit: cdcdde5 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 |
* subjects and expand merged :is() selector lists
There was a problem hiding this comment.
Pull request overview
This PR improves CSS style-recalc performance in @primer/view-components by rewriting a handful of selectors that force Blink onto the slow-path (unbucketable universal * subjects and merged :is(...) selector lists produced by nesting pseudo-elements/combinators under selector lists). The changes are scoped to a few high-cost selectors and are designed to preserve selector semantics, with one explicitly called-out behavior shift for autocomplete-item child color overrides.
Changes:
- Remove the
.autocomplete-item &:hover/&.selected … & * { color: inherit !important }universal-descendant recolor rule and rely on inheritedcolorinstead. - Expand Popover caret (
::before/::after) and breadcrumbs::afterrules into explicit per-class selectors to avoid merged:is(...)forms. - Refactor FormControl checkbox/radio labelWrap selectors so each branch has a single-class subject; regenerate
static/classes.jsonand add a patch changeset.
Show a summary per file
| File | Description |
|---|---|
app/components/primer/alpha/auto_complete.pcss |
Drops universal descendant recolor rule; uses inherited color for hover/selected states to avoid slow-path matching. |
app/components/primer/beta/popover.pcss |
Expands caret pseudo-element rules into per-class selectors to prevent merged :is(...) selector lists in compiled output. |
app/components/primer/beta/breadcrumbs.pcss |
Expands ::after suppression rule into explicit selectors to avoid merged :is(...). |
app/components/primer/alpha/text_field.pcss |
Rewrites labelWrap selectors to explicit wrap→labelWrap pairings to keep subjects bucketable. |
static/classes.json |
Surfaces newly explicit Popover caret-direction classes in the class→component mapping. |
.changeset/fast-pumas-juggle.md |
Adds a patch changeset documenting the style-recalc performance improvements. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 0
|
@copilot Fix the stylelint errors |
Fixed in the latest commit. The CI runs stylelint only on changed files, so touching
|
Syncs the fork with primer/view_components up to upstream commit 6733f3c (parent of Release Tracking primer#4098), the last release in this batch reaching v0.51.6. Notable upstream changes pulled in: - classnames.js Set export (primer#4096): static/classnames.{js,cjs,d.ts} plus the package.json exports subpath and files entries. - perf(css): removed universal `*` subjects and expanded merged `:is()` selector lists (primer#4095). Conflicts resolved keeping the fork's identity, scope and its higher dependency versions; upstream's new exports/files additions and the static class-name output were merged in and regenerated cleanly.
Syncs the fork with primer/view_components up to upstream commit 6733f3c (parent of Release Tracking primer#4098), reaching v0.51.6. Upstream changes pulled in: - classnames.js Set export (primer#4096): static/classnames.{js,cjs,d.ts}, the export-css-selectors collection logic, and the package.json exports subpath plus files entries. - perf(css): removed universal `*` subjects and expanded merged `:is()` selector lists (primer#4095). Conflict resolution kept the fork's identity, scope and its higher dependency versions; upstream's new exports were merged in. The recursive tsconfig include is re-applied here so nested component TS keeps experimentalDecorators under Vite 8/rolldown. All generated static files were regenerated from a clean tree.
Syncs the fork with primer/view_components up to upstream commit 6733f3c (parent of Release Tracking primer#4098), reaching v0.51.6. Upstream changes pulled in: - classnames.js Set export (primer#4096): static/classnames.{js,cjs,d.ts}, the export-css-selectors collection logic, and the package.json exports subpath plus files entries. - perf(css): removed universal `*` subjects and expanded merged `:is()` selector lists (primer#4095). Conflict resolution kept the fork's identity, scope and its higher dependency versions; upstream's new exports were merged in. The recursive tsconfig include is re-applied here so nested component TS keeps experimentalDecorators under Vite 8/rolldown. All generated static files were regenerated from a clean tree.
Syncs the fork with primer/view_components up to upstream commit 6733f3c (parent of Release Tracking primer#4098), reaching v0.51.6. Upstream changes pulled in: - classnames.js Set export (primer#4096): static/classnames.{js,cjs,d.ts}, the export-css-selectors collection logic, and the package.json exports subpath plus files entries. - perf(css): removed universal `*` subjects and expanded merged `:is()` selector lists (primer#4095). Conflict resolution kept the fork's identity, scope and its higher dependency versions; upstream's new exports were merged in. The recursive tsconfig include is re-applied here so nested component TS keeps experimentalDecorators under Vite 8/rolldown. All generated static files were regenerated from a clean tree. The upstream css change makes CI lint the whole text_field.pcss, which surfaced a pre-existing false positive: nesting-selector-no-missing- scoping-root fires on the & selectors inside the fork's FormControl-input-width @define-mixin (the & resolves at the @mixin include site). Silenced with a scoped stylelint-disable.
Authors: Please fill out this form carefully and completely.
Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.
What are you trying to accomplish?
On a real github.com trace, several Primer-owned selectors land on the style-recalc slow path (100% non-matching) because their rightmost compound can't be bucketed: a universal
*subject, or a merged:is(...)selector list. This cost is paid on every style invalidation, even on pages that ship none of these components.This addresses the
@primer/view-components-owned offenders in the.pcsssource:autocomplete-item(top offender, ~10.5 ms) — dropped the… *descendant recolor rule;coloris inherited, so setting it on the item recolors descendants without matching every element on the page.Popover-message--*(~6.5 ms) — expanded the nested&::before/&::aftercaret rules under selector lists into per-class selectors (specificity-neutral; eliminates all merged:is()in the compiled output).breadcrumb-item(~0.9 ms) — expanded the::afterrule into per-class selectors.FormControl-checkbox-wrap/radio-wraplabelWrap — pulled thelabelWraprule out of the nested list so each branch has a single-class (bucketable) subject.Regenerated
static/classes.json(expanded Popover selectors surface individual caret-direction classes) and added a patch changeset.Screenshots
No visual change intended. The
autocomplete-itemrecolor now relies on inheritance rather than an!importantuniversal override — descendants that set their owncolorare no longer forced to white (the tradeoff called out in the issue).Integration
No production code updates required; CSS-only refactor. Selector semantics are preserved (Popover/breadcrumb expansions are specificity-neutral; the
checkbox-wrap→checkbox-labelWrappairing is 1:1 in the markup).List the issues that this change affects.
Risk Assessment
What approach did you choose and why?
The root cause is bucketing, not
:is()itself: a subject compound with no class/tag/id/attr (universal*) or a merged multi-class:is(...)can't be fast-rejected, so Blink tests it against nearly every element on every recalc. Even with postcssnoIsPseudoSelector, nesting pseudo-elements/combinators under a selector-list parent still compiles to merged:is(...), so the fix is to author expanded per-class selectors in source.I scoped this to the safe, high-value offenders (≈18 ms of the measured cost) where expansion is specificity-neutral or behavior-equivalent.
ActionListItem--*was intentionally deferred: its:is()branches have differing specificity, so safe expansion needs specificity-preserving restructuring on a core component with visual snapshots — better as a focused follow-up (the "systemic" bucket in the issue).Anything you want to highlight for special attention from reviewers?
autocomplete-itemchange is a deliberate behavior shift: removing the* { color: inherit !important }hammer means descendants with explicit colors keep them. Confirm this is acceptable for current autocomplete item content.static/classes.jsonnow listsPopover-message--bottom/--left/--right/--*-top/--*-bottom, previously hidden inside merged:is(); the CSS-selector preview test passes with these surfaced.Accessibility
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.