diff --git a/.changeset/batched-style-injection.md b/.changeset/batched-style-injection.md new file mode 100644 index 000000000..cc3a42de7 --- /dev/null +++ b/.changeset/batched-style-injection.md @@ -0,0 +1,13 @@ +--- +'@cube-dev/ui-kit': minor +--- + +Collapse the kit's stylesheet writes into one style invalidation per 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. + +`` now enables tasty's `batchInjection` and opens a batch window for its own commits. A commit that mounts a portal does not re-render ``, so windows are opened per portal boundary too: `` (tooltips) and `` (popovers, modals and trays — the `Dialog` and `Menu` surfaces). Those are the commits where injection and measurement interleave worst, because react-aria positions the overlay from a layout effect in the same commit that mounts it. + +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. Any commit without a window in it writes straight through exactly as before. + +No API change: no new props, no new setup. SSR is unaffected — styles are collected as text there and the provider is inert without a `document`. diff --git a/src/components/Root.browser.test.tsx b/src/components/Root.browser.test.tsx new file mode 100644 index 000000000..331f5e81e --- /dev/null +++ b/src/components/Root.browser.test.tsx @@ -0,0 +1,217 @@ +import { hasPendingStyleWrites, tasty } from '@tenphi/tasty'; +import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { StrictMode, useLayoutEffect, useRef, useState } from 'react'; + +import { Button } from './actions/Button'; +import { Dialog, DialogTrigger } from './overlays/Dialog'; +import { Portal } from './portal'; +import { Root } from './Root'; + +/** + * The kit's batched-injection wiring. + * + * `configure({ batchInjection: true })` in `Root` only does something inside a + * batch window, so the wiring can silently become a no-op — the flag stays on, + * the provider goes missing, and every render is quietly back to one + * `insertRule()` per component. These tests assert on the queue itself so that + * cannot happen unnoticed. + * + * They also assert on `getBoundingClientRect()` inside a `useLayoutEffect`, + * which is the property that makes batching safe to enable at all: a queued + * write must land before anything can measure. Asserting on CSS text would not + * catch a regression there. + */ + +const WIDTH = 317; + +/** A component that measures itself in a layout effect, like a popover does. */ +function makeMeasured(record: (width: number) => void) { + const Box = tasty({ styles: { width: `${WIDTH}px`, height: '10px' } }); + + return function Measured() { + const ref = useRef(null); + useLayoutEffect(() => { + record(ref.current!.getBoundingClientRect().width); + }, []); + return ; + }; +} + +describe('Root batched injection', () => { + it('batches during the mount commit', () => { + const Box = tasty({ styles: { letterSpacing: '0.013em' } }); + let pendingMidRender: boolean | null = null; + + // Renders after , so anything Box queued is still queued here. + function Probe() { + pendingMidRender = hasPendingStyleWrites(); + return null; + } + + render( + + + + , + ); + + expect(pendingMidRender).toBe(true); + // Root's insertion effect drained the queue before the commit finished. + expect(hasPendingStyleWrites()).toBe(false); + }); + + it('has the rules in the sheet before layout effects run', () => { + let measured = -1; + const Measured = makeMeasured((w) => { + measured = w; + }); + + render( + + + , + ); + + expect(measured).toBe(WIDTH); + }); + + // Root does not re-render when an overlay opens, so its window does not cover + // that commit. Portal opens one of its own — this is what makes dialogs, + // tooltips and menus benefit rather than just the initial mount. + it('batches a portal that mounts without re-rendering Root', () => { + const Box = tasty({ styles: { letterSpacing: '0.029em' } }); + // One observation per render. `Portal` renders its children inline first and + // again once `mountRoot` resolves, and the second pass is a cache hit with + // nothing left to queue — so the question is whether *a* render batched, not + // what the last one saw. + const observed: boolean[] = []; + let open: (value: boolean) => void = () => {}; + + function Probe() { + observed.push(hasPendingStyleWrites()); + return null; + } + + function Host() { + const [isOpen, setOpen] = useState(false); + open = setOpen; + + if (!isOpen) return null; + + return ( + + + + + ); + } + + render( + + + , + ); + + expect(observed).toEqual([]); + + act(() => open(true)); + + expect(observed).toContain(true); + expect(hasPendingStyleWrites()).toBe(false); + }); + + // The claim this change rests on is that *overlays* benefit, and they do not + // go through : Popover, Modal and Tray portal through Overlay's raw + // createPortal, so DialogTrigger and MenuTrigger are covered only by the + // window Overlay opens. Driving a real DialogTrigger is the only way to assert + // that — a test passes whether or not the overlay path is covered. + it('batches the commit a real dialog mounts in', async () => { + const Box = tasty({ styles: { letterSpacing: '0.037em' } }); + const observed: boolean[] = []; + + function Probe() { + observed.push(hasPendingStyleWrites()); + return null; + } + + render( + + + + + + + + + , + ); + + expect(observed).toEqual([]); + + await userEvent.click(screen.getByRole('button', { name: 'Open' })); + await waitFor(() => expect(observed.length).toBeGreaterThan(0)); + + expect(observed).toContain(true); + expect(hasPendingStyleWrites()).toBe(false); + }); + + // Dev runs under StrictMode, which double-invokes render but runs insertion + // effects once. If a batch window survived its commit, the next commit with no + // provider in it would quietly get 'always' semantics and its layout effect + // would measure an unstyled box — a dev-only wrong number that never + // self-corrects. Consumers develop in StrictMode, so this is the common path. + it('keeps measurement correct after a StrictMode commit', () => { + const WIDTH = 211; + const Box = tasty({ styles: { width: `${WIDTH}px`, height: '10px' } }); + let measured = -1; + + function Measured() { + const ref = useRef(null); + useLayoutEffect(() => { + measured = ref.current!.getBoundingClientRect().width; + }, []); + return ; + } + + render( + + + , + ); + + // A separate commit, outside any provider. + render(); + + expect(measured).toBe(WIDTH); + }); + + it('measures correctly inside a portal that mounts later', () => { + let measured = -1; + let open: (value: boolean) => void = () => {}; + const Measured = makeMeasured((w) => { + measured = w; + }); + + function Host() { + const [isOpen, setOpen] = useState(false); + open = setOpen; + + return isOpen ? ( + + + + ) : null; + } + + render( + + + , + ); + + act(() => open(true)); + + expect(measured).toBe(WIDTH); + }); +}); diff --git a/src/components/Root.tsx b/src/components/Root.tsx index ad02d2db2..eb091268a 100644 --- a/src/components/Root.tsx +++ b/src/components/Root.tsx @@ -6,6 +6,7 @@ import { filterBaseProps, setGlobalPredefinedStates, tasty, + TastyBatchProvider, } from '@tenphi/tasty'; import { useEffect, useMemo, useRef, useState } from 'react'; import { ModalProvider } from 'react-aria'; @@ -39,6 +40,15 @@ setGlobalPredefinedStates({ }); configure({ + // Collapse the kit's stylesheet writes into one style invalidation per commit + // instead of one per component. Only takes effect inside a + // `` window, and windows have to be opened per portal + // boundary because a commit that mounts a portal does not re-render ``. + // The kit opens three: here, in `` (tooltips) and in `` + // (popovers, modals, trays — i.e. Dialog and Menu). Writes in any commit + // without a window go straight through exactly as before, so a + // `useLayoutEffect` can never measure an element whose rules have not landed. + batchInjection: true, units: { x: 'var(--gap)', r: 'var(--radius)', @@ -202,42 +212,46 @@ export function Root(allProps: CubeRootProps) { const styles = extractStyles(props, STYLES); return ( - - - - - - - - - - {children} - - - - - - - - + + + + + + + + + + + + {children} + + + + + + + + + + ); } diff --git a/src/components/overlays/Modal/Overlay.tsx b/src/components/overlays/Modal/Overlay.tsx index 7214af074..e02534ddb 100644 --- a/src/components/overlays/Modal/Overlay.tsx +++ b/src/components/overlays/Modal/Overlay.tsx @@ -1,3 +1,4 @@ +import { TastyBatchProvider } from '@tenphi/tasty'; import { Children, cloneElement, @@ -111,7 +112,17 @@ function Overlay(props: CubeOverlayProps, ref) { ); - return createPortal(contents, container || root || document.body); + // Popover, Modal and Tray all portal through here, which makes this the + // overlay path that matters most for batching: a dialog or menu mounts a + // fresh subtree and react-aria positions it from a layout effect in the same + // commit. `` does not re-render for those commits, so open a window + // here — it flushes in `useInsertionEffect`, before any positioning effect + // reads the DOM. Note this is a *raw* `createPortal`, not ``, so the + // window `` opens does not reach these overlays. + return createPortal( + {contents}, + container || root || document.body, + ); } let _Overlay = forwardRef(Overlay); diff --git a/src/components/portal/Portal.tsx b/src/components/portal/Portal.tsx index 0cdeec58e..04a057b95 100644 --- a/src/components/portal/Portal.tsx +++ b/src/components/portal/Portal.tsx @@ -1,3 +1,4 @@ +import { TastyBatchProvider } from '@tenphi/tasty'; import { createPortal } from 'react-dom'; import { PortalProps } from './types'; @@ -29,8 +30,17 @@ import { usePortal } from './usePortal'; export function Portal(props: PortalProps) { const { children, mountRoot, isDisabled } = usePortal(props); - if (isDisabled) return <>{children}; + // A portal mounts a fresh subtree in a commit that did not re-render ``, + // so its window cannot cover this one — open one here, flushing in + // `useInsertionEffect` before any positioning effect reads the DOM. + // + // In the kit this path is tooltips: `TooltipTrigger` is the only component + // that renders ``. Popovers, modals and trays portal through + // ``'s own `createPortal`, which opens its own window. + const content = {children}; + + if (isDisabled) return content; // Render inline until mountRoot is available (fixes timing issues in tests and SSR) - if (!mountRoot) return <>{children}; - return createPortal(children, mountRoot); + if (!mountRoot) return content; + return createPortal(content, mountRoot); }