-
Notifications
You must be signed in to change notification settings - Fork 8
perf(Root,Portal,Overlay): batch stylesheet writes into one invalidation per commit #1344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2eb6c28
perf(Root,Portal): batch stylesheet writes into one invalidation per …
tenphi d4c4913
test(Root): cover StrictMode, repin tasty to the post-fix snapshot
tenphi 37deb99
fix(Overlay): open a batch window where the overlays actually portal
tenphi 57c5710
chore(deps): update @tenphi/tasty to 3.2.0
tenphi d0b76d6
Merge origin/main into claude/test-batch-injection
tenphi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| --- | ||
| '@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, and `<Portal>` opens one for every overlay that mounts, which is | ||
| where injection and measurement interleave worst. 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`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| import { hasPendingStyleWrites, tasty } from '@tenphi/tasty'; | ||
| import { act, render } from '@testing-library/react'; | ||
| import { useLayoutEffect, useRef, useState } from 'react'; | ||
|
|
||
| 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); | ||
| }); | ||
|
|
||
| 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); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.