Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/batched-style-injection.md
Original file line number Diff line number Diff line change
@@ -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.

`<Root>` now enables tasty's `batchInjection` and opens a batch window for its own commits. A commit that mounts a portal does not re-render `<Root>`, so windows are opened per portal boundary too: `<Portal>` (tooltips) and `<Overlay>` (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`.
217 changes: 217 additions & 0 deletions src/components/Root.browser.test.tsx
Original file line number Diff line number Diff line change
@@ -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<HTMLDivElement>(null);
useLayoutEffect(() => {
record(ref.current!.getBoundingClientRect().width);
}, []);
return <Box ref={ref} />;
};
}

describe('Root batched injection', () => {
it('batches during the mount commit', () => {
const Box = tasty({ styles: { letterSpacing: '0.013em' } });
let pendingMidRender: boolean | null = null;

// Renders after <Box/>, so anything Box queued is still queued here.
function Probe() {
pendingMidRender = hasPendingStyleWrites();
return null;
}

render(
<Root>
<Box />
<Probe />
</Root>,
);

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(
<Root>
<Measured />
</Root>,
);

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 (
<Portal>
<Box />
<Probe />
</Portal>
);
}

render(
<Root>
<Host />
</Root>,
);

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 <Portal>: 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 <Portal> 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(
<Root>
<DialogTrigger type="popover">
<Button>Open</Button>
<Dialog>
<Box />
<Probe />
</Dialog>
</DialogTrigger>
</Root>,
);

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<HTMLDivElement>(null);
useLayoutEffect(() => {
measured = ref.current!.getBoundingClientRect().width;
}, []);
return <Box ref={ref} />;
}

render(
<StrictMode>
<Root />
</StrictMode>,
);

// A separate commit, outside any provider.
render(<Measured />);

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 ? (
<Portal>
<Measured />
</Portal>
) : null;
}

render(
<Root>
<Host />
</Root>,
);

act(() => open(true));

expect(measured).toBe(WIDTH);
});
});
88 changes: 51 additions & 37 deletions src/components/Root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
filterBaseProps,
setGlobalPredefinedStates,
tasty,
TastyBatchProvider,
} from '@tenphi/tasty';
import { useEffect, useMemo, useRef, useState } from 'react';
import { ModalProvider } from 'react-aria';
Expand Down Expand Up @@ -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
// `<TastyBatchProvider>` window, and windows have to be opened per portal
// boundary because a commit that mounts a portal does not re-render `<Root>`.
// The kit opens three: here, in `<Portal>` (tooltips) and in `<Overlay>`
// (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)',
Expand Down Expand Up @@ -202,42 +212,46 @@ export function Root(allProps: CubeRootProps) {
const styles = extractStyles(props, STYLES);

return (
<I18nProvider i18n={i18n} locale={locale}>
<Provider navigation={navigation} root={rootRef}>
<TrackingProvider event={tracking?.event}>
<RootElement
ref={ref}
data-uikit={VERSION}
data-tasty={TASTY_VERSION}
data-font-display={fontDisplay}
{...filterBaseProps(props, { eventProps: true })}
styles={styles}
style={{
'--pointer': cursorStrategy === 'web' ? 'pointer' : 'default',
...style,
}}
tokens={tokens}
>
<GlobalStyles
bodyStyles={bodyStyles}
publicUrl={publicUrl}
fonts={fonts}
font={font}
monospaceFont={monospaceFont}
fontDisplay={fontDisplay}
/>
<ModalProvider>
<PortalProvider value={ref}>
<EventBusProvider>
<OverlayProvider>
<AlertDialogApiProvider>{children}</AlertDialogApiProvider>
</OverlayProvider>
</EventBusProvider>
</PortalProvider>
</ModalProvider>
</RootElement>
</TrackingProvider>
</Provider>
</I18nProvider>
<TastyBatchProvider>
<I18nProvider i18n={i18n} locale={locale}>
<Provider navigation={navigation} root={rootRef}>
<TrackingProvider event={tracking?.event}>
<RootElement
ref={ref}
data-uikit={VERSION}
data-tasty={TASTY_VERSION}
data-font-display={fontDisplay}
{...filterBaseProps(props, { eventProps: true })}
styles={styles}
style={{
'--pointer': cursorStrategy === 'web' ? 'pointer' : 'default',
...style,
}}
tokens={tokens}
>
<GlobalStyles
bodyStyles={bodyStyles}
publicUrl={publicUrl}
fonts={fonts}
font={font}
monospaceFont={monospaceFont}
fontDisplay={fontDisplay}
/>
<ModalProvider>
<PortalProvider value={ref}>
<EventBusProvider>
<OverlayProvider>
<AlertDialogApiProvider>
{children}
</AlertDialogApiProvider>
</OverlayProvider>
</EventBusProvider>
</PortalProvider>
</ModalProvider>
</RootElement>
</TrackingProvider>
</Provider>
</I18nProvider>
</TastyBatchProvider>
);
}
13 changes: 12 additions & 1 deletion src/components/overlays/Modal/Overlay.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TastyBatchProvider } from '@tenphi/tasty';
import {
Children,
cloneElement,
Expand Down Expand Up @@ -111,7 +112,17 @@ function Overlay(props: CubeOverlayProps, ref) {
</Provider>
);

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. `<Root>` 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 `<Portal>`, so the
// window `<Portal>` opens does not reach these overlays.
return createPortal(
<TastyBatchProvider>{contents}</TastyBatchProvider>,
container || root || document.body,
);
}

let _Overlay = forwardRef(Overlay);
Expand Down
16 changes: 13 additions & 3 deletions src/components/portal/Portal.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TastyBatchProvider } from '@tenphi/tasty';
import { createPortal } from 'react-dom';

import { PortalProps } from './types';
Expand Down Expand Up @@ -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 `<Root>`,
// 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 `<Portal>`. Popovers, modals and trays portal through
// `<Overlay>`'s own `createPortal`, which opens its own window.
const content = <TastyBatchProvider>{children}</TastyBatchProvider>;

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);
Comment thread
cursor[bot] marked this conversation as resolved.
}
Loading