perf(Spinner): replace Web Animations API with CSS animation-delay sync#7550
perf(Spinner): replace Web Animations API with CSS animation-delay sync#7550hectahertz wants to merge 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: b8cc0c2 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 |
|
👋 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. Or, apply the |
3afb5f1 to
7b96dc0
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the Web Animations API-based spinner synchronization mechanism with a CSS animation-delay approach to eliminate a Safari/WebKit performance bottleneck. The previous implementation used multiple Web Animations API calls per Spinner mount (getAnimations(), pause(), element.animate(), and startTime manipulation) along with a global useSyncExternalStore that caused all mounted Spinners to re-render. The new approach computes a negative CSS animation-delay from performance.now() at visibility time, allowing the CSS animation engine to handle synchronization natively with GPU compositing.
Changes:
- Removed the complex Web Animations API synchronization logic including the global
animationTimingStoreanduseSpinnerAnimationhook - Implemented CSS-based animation sync using computed
animation-delaybased onperformance.now() % 1000 - Updated state management to track both visibility and sync delay, computing the delay at the moment the spinner becomes visible (either immediately or after the delay prop timeout)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react/src/Spinner/Spinner.tsx | Replaced ~120 lines of Web Animations API code with a simple 4-line computeSyncDelay function; updated state management to compute sync delay at visibility time; applied computed delay as inline animationDelay style when feature flag is enabled and user has no motion preference |
| .changeset/spinner-css-animation-sync.md | Added patch-level changeset describing the performance improvement |
Closes #
Replaces the Web Animations API-based spinner synchronization with a pure CSS
animation-delayapproach, eliminating a Safari/WebKit performance bottleneck.The previous sync mechanism called three Web Animations API methods per Spinner mount:
element.getAnimations()(3-5x slower in WebKit than Chromium),CSSAnimation.pause()+element.animate()(replaces native CSS animation with a JS-driven one, often falling back to main-thread compositing in WebKit), andAnimation.startTime(forces WebKit to recalculate timing for all document animations). This resulted in 2 animations per SVG and a globaluseSyncExternalStorestore that re-rendered all mounted Spinners when the first one set itsstartTime.The new approach computes a negative CSS
animation-delayfromperformance.now()at visibility time:Since all instances reference the same monotonic clock, they land at the same rotation angle regardless of mount time. The CSS animation engine handles everything natively (GPU-composited), with 1 animation per SVG and zero JS animation overhead. The delay is computed at visibility time (not mount time), so spinners using the
delayprop don't flash at the wrong angle on the first frame.Measurements (5 spinners, staggered mount)
The 4° spread is floating-point rounding from
performance.now(), visually imperceptible.Changelog
New
N/A
Changed
animation-delayinstead of the Web Animations APIRemoved
N/A
Rollout strategy
Testing & Reviewing
primer_react_spinner_synchronize_animationsfeature flag in Storybook toolbarprefers-reduced-motion: reducein OS settings, confirm sync is skippedMerge checklist