Skip to content

perf(ActionList): replace :has() selector with JS-computed attribute - #7714

Merged
primer[bot] merged 13 commits into
mainfrom
hectahertz/perf-action-list-has-selector
Apr 22, 2026
Merged

perf(ActionList): replace :has() selector with JS-computed attribute#7714
primer[bot] merged 13 commits into
mainfrom
hectahertz/perf-action-list-has-selector

Conversation

@hectahertz

@hectahertzhectahertz commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Closeshttps://github.com/github/github-ui/issues/17359

This CSS selector on the ActionList container forces the browser to walk the entire subtree on every DOM mutation:

&:has([data-has-description='true']):has([data-has-description='false']) {
& .ItemLabel { font-weight:var(--base-text-weight-normal); }
}

With 100 items (~1860 DOM elements), sequential DOM mutations with forced style recalc (the pattern during React reconciliation):

MetricBefore (:has())After (data attr)Improvement
Remove all items426ms10ms42x faster
Add all items415ms27ms15x faster
Total841ms37ms23x faster
Per mutation4.2ms0.18ms23x faster

(Chrome, 100 items, 6x CPU throttle. Safari is worse due to historically quadratic :has() invalidation, causing the 10-20+ second freezes reported in github/github-ui#17223.)

Replaced with a data-mixed-descriptions attribute computed via useLayoutEffect in List.tsx. Two querySelector calls after render replace continuous full-subtree style invalidation from the CSS engine.

Why DOM queries instead of React children/context:

  • Each Item's description is detected via useSlots at render time, so the List can't know which Items have descriptions without duplicating slot detection or deeply inspecting opaque children trees (fragile with Groups, conditional rendering, wrapper components)
  • A context-based approach (Items registering description state) would work but adds registration/unregistration callbacks, a new provider, and re-renders when the count changes. Not worth the complexity for a single derived boolean

Related: #7708 (stylelint guard for container-level :has() selectors), github/github-ui#17223

Changelog

New

N/A

Changed

  • Replaced CSS :has([data-has-description]) container selector with data-mixed-descriptions attribute computed in JS

Removed

N/A

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  1. Open Storybook, navigate to StressTests/Components/ActionList/MixedDescriptions. Click "Start" to run the stress test
  2. Verify labels use font-weight: normal when the list has mixed items (some with descriptions, some without)
  3. Check an ActionList where all items have descriptions, or none do. Labels should use their default font-weight
  4. For perf validation: open Chrome DevTools Performance tab with 6x CPU throttling, run the stress test. Style recalculation per mutation should be <1ms

Merge checklist

@changeset-bot

changeset-botBot commented Mar 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: abb7503

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Mar 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

@hectahertzhectahertz added the skip changeset This change does not need a changelog label Mar 30, 2026
@hectahertz
hectahertz marked this pull request as ready for review March 30, 2026 16:27
@hectahertz
hectahertz requested a review from a team as a code ownerMarch 30, 2026 16:27
@primer

primerBot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve ActionList rendering performance by removing an expensive container-level :has() selector and replacing it with a JS-computed data-mixed-descriptions attribute. However, the PR also introduces a new packages/primer-api workspace plus a new GitHub Actions workflow for a Slack/LLM “Primer Bot”, which significantly expands scope beyond the stated goal.

Changes:

  • ActionList: replace container :has() selector with data-mixed-descriptions computed after render.
  • Add a new @primer/api workspace (HTTP server + GitHub Action entrypoint) for answering Primer questions via LLM + primer.style docs.
  • Add a new primer-bot.yml workflow to run the bot on repository_dispatch / manual dispatch.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 13 comments.

Show a summary per file
FileDescription
packages/react/src/ActionList/List.tsxComputes and sets data-mixed-descriptions on the list element post-render.
packages/react/src/ActionList/ActionList.module.cssSwitches styling condition from :has() to [data-mixed-descriptions='true'].
packages/primer-api/tsconfig.jsonAdds TS config for the new @primer/api workspace.
packages/primer-api/src/prompts.tsDefines the LLM system prompt and user prompt builder.
packages/primer-api/src/llm.tsAdds OpenAI/GitHub Models chat completion integration and context retrieval wiring.
packages/primer-api/src/knowledge.tsRetrieves relevant component docs from primer.style and formats context for the prompt.
packages/primer-api/src/index.tsImplements a small HTTP server with /ask and /health endpoints.
packages/primer-api/src/config.tsLoads configuration from environment variables (tokens, model, port, etc.).
packages/primer-api/src/action.tsGitHub Actions entrypoint to read dispatch payload and post an answer to Slack.
packages/primer-api/package.jsonDeclares the new workspace package, scripts, and dependencies.
packages/primer-api/README.mdDocuments bot setup (Slack + repo dispatch + workflow) and local dev usage.
packages/primer-api/.env.exampleProvides example env vars for local usage.
.github/workflows/primer-bot.ymlAdds workflow to run the Primer bot from dispatch events or manual input.
package-lock.jsonUpdates lockfile for new workspace/dependencies and version bumps.

Comment threadpackages/primer-api/src/prompts.ts Outdated
Comment threadpackages/primer-api/src/index.ts Outdated
Comment threadpackages/primer-api/src/index.ts Outdated
Comment threadpackages/primer-api/src/llm.ts Outdated
Comment thread.github/workflows/primer-bot.yml Outdated
Comment threadpackages/primer-api/src/config.ts Outdated
Comment threadpackages/primer-api/README.md Outdated
Comment threadpackages/react/src/ActionList/List.tsx Outdated
Comment threadpackages/react/src/ActionList/List.tsx
Comment threadpackages/react/src/ActionList/List.tsx
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7714 March 30, 2026 16:36 Inactive
@hectahertz
hectahertzforce-pushed the hectahertz/perf-action-list-has-selector branch from a721664 to c383302CompareMarch 30, 2026 16:36
@github-actions
github-actionsBot requested a deployment to storybook-preview-7714 March 30, 2026 16:41 Abandoned
@primer

primerBot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

🤖 Lint issues have been automatically fixed and committed to this PR.

@hectahertz
hectahertzforce-pushed the hectahertz/perf-action-list-has-selector branch from faeb40f to 0919c3bCompareMarch 30, 2026 16:48
- Use useIsomorphicLayoutEffect for SSR compatibility
- Only update data-mixed-descriptions when value changes
- Remove attribute when not needed instead of setting 'false'
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7714 March 30, 2026 16:51 Inactive
@hectahertz
hectahertz enabled auto-merge March 30, 2026 16:56
@github-actions
github-actionsBot requested a deployment to storybook-preview-7714 March 30, 2026 17:00 Abandoned
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7714 March 30, 2026 17:10 Inactive
@primer
primerBot disabled auto-merge April 15, 2026 14:33
@primer
primerBot enabled auto-merge April 15, 2026 14:33
@primer
primerBot disabled auto-merge April 15, 2026 14:33
@primer
primerBot enabled auto-merge April 15, 2026 14:33
@primer
primerBot disabled auto-merge April 15, 2026 15:08
@primer
primerBot enabled auto-merge April 15, 2026 15:08
@jonrohanjonrohan self-assigned this Apr 21, 2026
@jonrohanjonrohan added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Apr 21, 2026
@primer-integration

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/19042

@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed CI  Passed
Passed VRT  Passed
Passed Projects  Passed

All checks passed!

Replace :has() selector with a JavaScript-computed attribute for performance improvements in ActionList.
@primer
primerBot added this pull request to the merge queueApr 22, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-7714 April 22, 2026 18:17 Inactive
Merged via the queue into main with commit e974d9fApr 22, 2026
53 checks passed
@primer
primerBot deleted the hectahertz/perf-action-list-has-selector branch April 22, 2026 18:22
@primerprimerBot mentioned this pull request Apr 22, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary ReleaseApply this label when you want CI to create a canary release of the current PRintegration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpmintegration-tests: skipped manuallyChanges in this PR do not require an integration test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@hectahertz@jonrohan