Skip to content

perf(Root,Portal,Overlay): batch stylesheet writes into one invalidation per commit - #1344

Open
tenphi wants to merge 4 commits into
mainfrom
claude/test-batch-injection
Open

perf(Root,Portal,Overlay): batch stylesheet writes into one invalidation per commit#1344
tenphi wants to merge 4 commits into
mainfrom
claude/test-batch-injection

Conversation

@tenphi

@tenphi tenphi commented Aug 21, 2026

Copy link
Copy Markdown
Member

Adopts tasty 3.2.0's batched style injection across the kit.

Why

Every insertRule() on a live stylesheet invalidates style for that sheet's scope. Kit components inject during React's render phase, so when anything else reads layout in the same pass — a tooltip positioning itself, TextArea autosizing, a virtualized table measuring rows — the two interleave and the browser is forced to recalculate style between every injection. A Chrome trace of a production app showed 16 forced recalcs totalling 62 ms sandwiched between insertRule calls, including one 475 ms task with 52 injections and 17 recalcs.

What changed

configure({ batchInjection: true }) in Root.tsx, plus a batch window at each portal boundary. Windows have to be per-boundary: a commit that mounts a portal never re-renders <Root>, so Root's window cannot cover it.

Window Covers
<Root> App mount, and any Root re-render
<Portal> Tooltips — TooltipTrigger is its only consumer among the components
<Overlay> Popovers, modals, trays — i.e. Dialog and Menu surfaces

Deliberately not covered: Board's WidgetHost drag clone, LayoutPanel, and use-context-menu's 0×0 anchor. The first two measure only from event handlers, which sit outside any window in true mode and so are written straight through; LayoutPanel never measures geometry at all. A window there would buy nothing but an extra effect per portal.

No API change — no new props, no new setup for consumers.

Why this is safe

The flush happens in useInsertionEffect, which React runs in the mutation phase, before any useLayoutEffect:

provider renders          -> window OPEN
  children render         -> injections queued
provider insertionEffect  -> FLUSH, window CLOSED
layout effects run        -> rules are in the sheet

Any commit with no window in it writes straight through, exactly as before. Turning this on can make injection cheaper; it cannot make a measurement wrong. SSR is unaffected — styles are collected as text there and the provider is inert without a document.

Verification

  • pnpm test — 97 files, 2093 pass (1 pre-existing skip). Every test using renderWithRoot now runs inside a batch window, so the existing suite doubles as the regression test.
  • pnpm test:browser — 11 files, 129 pass, including the measure-heavy suites this targets: TextArea autosize, DialogTrigger, MenuTrigger, Tabs, Board, DataTable.
  • Chromatic UI Tests + UI Review pass — no visual diffs, which matters because <Overlay> wraps every popover, modal and tray.
  • pnpm lint clean, pnpm build succeeds, size +0.15% overall / +0.6% tree-shaken, both within limits.

Root.browser.test.tsx asserts on the write queue itself (hasPendingStyleWrites() mid-render), so the wiring cannot silently degrade into a no-op — the flag staying on while a provider goes missing would otherwise be invisible. Each test was verified to fail with its window removed.

Review history worth knowing

Two bugs were found and fixed here, both by testing claims rather than reading code:

  1. Cursor Bugbot caught that this PR's original central claim was wrong. Popover/Modal/Tray portal through Overlay's raw createPortal, not <Portal>, so DialogTrigger and MenuTrigger — the surfaces named as benefiting most — had no window at all. Fixed by adding the <Overlay> window. The test that now covers it drives a real DialogTrigger; the original <Portal> test passed whether or not the overlay path was covered, so it validated the mechanism while leaving the claim untested.
  2. A StrictMode window leak in tasty itself, found while reviewing that PR: a depth counter ended each StrictMode commit above zero, silently giving the next provider-less commit 'always' semantics and letting its layout effect measure an unstyled box. Fixed upstream before 3.2.0 shipped; the StrictMode test here is the integration-level proof, and it fails against the pre-fix snapshot.

🤖 Generated with Claude Code

…commit

Every insertRule() on a live stylesheet invalidates style for that sheet's
scope. Kit components inject during React's render phase, so when anything else
reads layout in the same pass — a tooltip positioning itself, TextArea
autosizing, a virtualized table measuring rows — the two interleave and the
browser is forced to recalculate style between every injection.

Enable tasty's `batchInjection` and open a batch window in <Root> for its own
commits, plus one in <Portal> for every overlay that mounts. Overlays are where
this interleaving is worst: a dialog or tooltip mounts a fresh subtree and
react-aria positions it from a layout effect in the same commit, and <Root> does
not re-render for those commits so its window cannot cover them.

Writes are queued and applied together, and the flush happens in
useInsertionEffect — before any useLayoutEffect — so nothing can measure an
element whose rules have not landed yet. A commit with no window in it writes
straight through exactly as before, and SSR is unaffected: styles are collected
as text there and the provider is inert without a document.

Root.browser.test.tsx asserts on the write queue itself, so the wiring cannot
silently degrade into a no-op, and on getBoundingClientRect() inside a layout
effect, which is the property that makes batching safe to turn on.

Pins @tenphi/tasty to the PR snapshot that carries the feature. NOT FOR MERGE
until that lands in a release.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 57c5710

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

This PR includes changesets to release 1 package
Name Type
@cube-dev/ui-kit Minor

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

@vercel

vercel Bot commented Aug 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cube-ui-kit Ready Ready Preview Aug 21, 2026 6:11pm

Request Review

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

📦 NPM canary release

Deployed canary version 0.0.0-canary-3fca3d8.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🧪 Storybook is successfully deployed!

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🏋️ Size limit report

Name Size Passed?
All 498.7 KB (+0.15% 🔺) Yes 🎉
Tree shaking (just a Button) 121.84 KB (+0.6% 🔺) Yes 🎉

Compared against main at 4afd730run 32492162650, 2026-08-21T14:26:38Z.

To see which modules changed, download the size-limit-statoscope-report artifact from this run and open report.html.

@tenphi
tenphi marked this pull request as ready for review August 21, 2026 17:27

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2eb6c28. Configure here.

Comment thread src/components/portal/Portal.tsx
The snapshot this branch pinned predated a fix in tenphi/tasty#269: a batch
window opened during render survived its commit under StrictMode, because
StrictMode double-invokes render but runs useInsertionEffect once. The next
commit with no provider in it then got 'always' semantics silently, and its
layout effect measured an unstyled box. Consumers develop in StrictMode, so that
was the common path, not an edge case.

The new test is the integration-level proof: it fails against the old snapshot
and passes against the new one. Worth keeping permanently — nothing else in the
suite renders under StrictMode, so a regression here would otherwise only show
up in a consumer's dev build.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cursor Bugbot caught that the PR's central claim was wrong. Popover, Modal and
Tray portal through Overlay's own raw createPortal, not through <Portal>, so
DialogTrigger and MenuTrigger — the surfaces named as benefiting most — got no
batch window at all. <Portal> turns out to cover tooltips only: TooltipTrigger
is its single consumer among the components.

Open a window in Overlay too. The new DialogTrigger test drives a real popover
and fails without it, which the existing <Portal> test could not do: that one
passes whether or not the overlay path is covered, so it validated the mechanism
while leaving the claim untested.

Windows have to be opened per portal boundary, since a commit that mounts a
portal does not re-render <Root>. Three are now open — Root, Portal, Overlay —
and the comments and changeset say which surfaces each one covers rather than
implying blanket coverage. Still uncovered by design: Board's WidgetHost drag
clone, LayoutPanel, and use-context-menu's anchor, none of which position
themselves off a same-commit measurement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tenphi

tenphi commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

@cursor Good catch, and it invalidated this PR's central claim rather than a detail of it — fixed in 37deb99.

You were right: Popover, Modal and Tray portal through Overlay's own createPortal, so DialogTrigger and MenuTrigger — the surfaces I named as benefiting most — got no batch window at all. Checking the rest of the portal surface, <Portal> turns out to cover tooltips only: TooltipTrigger is its single consumer among the components. So the placement I shipped covered the one overlay family I hadn't argued for and missed the one I had.

What changed:

  • <Overlay> now opens a window of its own, so popovers, modals and trays are covered.
  • A new test drives a real DialogTrigger and asserts writes were queued during the commit that mounts it. It fails without the Overlay window — which the existing <Portal> test could never do, since that one passes whether or not the overlay path is covered. It validated the mechanism while leaving the claim untested.
  • The comments and changeset now name which surfaces each of the three windows covers, instead of implying blanket coverage.

Deliberately still uncovered, and now documented as such: Board's WidgetHost drag clone, LayoutPanel, and use-context-menu's 0×0 anchor. None of them position themselves off a same-commit measurement, so a window buys them nothing but an extra effect. The general constraint is that windows must be opened per portal boundary, because a commit that mounts a portal never re-renders <Root> — worth keeping in mind for future portal sites.

@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

No changes needed — the fix you describe is already committed and pushed as HEAD (37deb99d), and the working tree is clean.

I verified the three batch windows are in place and that each one's comment names its scope:

  • Root.tsx wraps the tree in <TastyBatchProvider>, and the configure() comment states windows must be opened per portal boundary since a portal-mounting commit doesn't re-render <Root>.
  • Portal.tsx notes it covers tooltips only, since TooltipTrigger is its sole consumer.
  • Overlay.tsx opens a window around its raw createPortal, covering popovers, modals, and trays — with a comment explicitly flagging that this is a raw createPortal and not <Portal>, so the <Portal> window doesn't reach it.

Your point about the <Portal> test being unable to catch this is the important one: a test that passes regardless of whether the overlay path is covered validates the mechanism but not the claim, so the DialogTrigger test that fails without the Overlay window is what makes the coverage load-bearing.

Open in Web Open in Cursor 

Batched injection shipped in tasty 3.2.0, so this branch no longer needs a PR
snapshot. Drops the last reason it was a draft.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tenphi tenphi changed the title perf(Root,Portal): batch stylesheet writes into one invalidation per commit perf(Root,Portal,Overlay): batch stylesheet writes into one invalidation per commit Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant