Repository navigation
perf(Layer): the observer that follows a Tooltip's trigger re-arms from its own report, with no layout read - #10235
Conversation
…no layout read The observer that follows a Tooltip's trigger through a scroll re-arms after every report. Each re-arm read the trigger's box and the viewport's width and height, and a ratio-1 report read the box once more, so every frame of a scroll read layout once or twice. The report already carries both: its target box, and its root box, which is the viewport shrunk by the root margin the observer was armed with. Re-arming now uses them. The box and the viewport are read once, when observing starts. Measured in Chromium over a scripted scroll, a layout shift and a clipped scroller: the same move reports on the same frames, and no layout reads.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| LayerProvider | 4.1 kB (0) · 20 B CSS (0) | — | ✓ · ✓ · ✗ |
| FieldLabel | 43.0 kB (+117 B) · 5.2 kB CSS (0) | — | — · — · — |
| SegmentedControl | 89.1 kB (+114 B) · 6.8 kB CSS (0) | — | — · — · — |
112 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DropdownMenu | 85.8 kB (+113 B) · 16.6 kB CSS (0) | — | — · — · — |
| ChatComposer | 68.3 kB (+112 B) · 11.2 kB CSS (0) | — | — · — · — |
| CheckboxListItem | 66.2 kB (+109 B) · 10.9 kB CSS (0) | — | — · — · — |
| ChatComposerInput | 66.3 kB (+108 B) · 10.6 kB CSS (0) | — | — · — · — |
| ChatComposerTokenElement | 66.3 kB (+107 B) · 10.6 kB CSS (0) | — | — · — · — |
| DateTimeInput | 193.4 kB (+105 B) · 18.0 kB CSS (0) | — | — · — · — |
| Carousel | 58.4 kB (+103 B) · 10.1 kB CSS (0) | — | — · — · — |
| ListItem | 65.2 kB (+103 B) · 10.9 kB CSS (0) | — | — · — · — |
| DateInput | 167.1 kB (+102 B) · 16.1 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 91.0 kB (+102 B) · 16.7 kB CSS (0) | — | — · — · — |
| lab/ListInput | 91.0 kB (+101 B) · 1.6 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 92.4 kB (+101 B) · 1.9 kB CSS (0) | — | — · — · — |
| charts/Chart | 173.7 kB (+100 B) · 494 B CSS (0) | — | — · — · — |
| Pagination | 98.7 kB (+100 B) · 16.9 kB CSS (0) | — | — · — · — |
| Selector | 88.7 kB (+100 B) · 16.5 kB CSS (0) | — | — · — · — |
| SideNavHeading | 66.6 kB (+100 B) · 10.6 kB CSS (0) | — | — · — · — |
| DropdownMenuSubMenu | 64.6 kB (+99 B) · 9.4 kB CSS (0) | — | — · — · — |
| Toast | 89.0 kB (+99 B) · 9.6 kB CSS (0) | — | — · — · — |
| lab/TransferList | 85.4 kB (+99 B) · 1.7 kB CSS (0) | — | — · — · — |
| AvatarGroupOverflow | 44.5 kB (+98 B) · 5.5 kB CSS (0) | — | — · — · — |
| Popover | 60.8 kB (+98 B) · 13.4 kB CSS (0) | — | — · — · — |
| PowerSearch | 264.0 kB (+98 B) · 22.9 kB CSS (0) | — | — · — · — |
| TableCell | 91.4 kB (+98 B) · 16.9 kB CSS (0) | — | — · — · — |
| BaseTypeahead | 62.0 kB (+97 B) · 10.3 kB CSS (0) | — | — · — · — |
| ContextMenu | 90.2 kB (+97 B) · 16.5 kB CSS (0) | — | — · — · — |
| MultiSelector | 88.2 kB (+97 B) · 16.2 kB CSS (0) | — | — · — · — |
| TopNavHeading | 66.1 kB (+96 B) · 10.4 kB CSS (0) | — | — · — · — |
| Typeahead | 69.9 kB (+96 B) · 11.8 kB CSS (0) | — | — · — · — |
| lab/TourStep | 66.9 kB (+96 B) · 590 B CSS (0) | — | — · — · — |
| DateRangeInput | 81.4 kB (+95 B) · 11.8 kB CSS (0) | — | — · — · — |
| RadioListItem | 63.0 kB (+95 B) · 9.7 kB CSS (0) | — | — · — · — |
| Switch | 94.8 kB (+95 B) · 9.5 kB CSS (0) | — | — · — · — |
| TabMenu | 63.0 kB (+95 B) · 10.4 kB CSS (0) | — | — · — · — |
| Tooltip | 25.9 kB (+95 B) · 4.3 kB CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 75.2 kB (+95 B) · 1.1 kB CSS (0) | — | — · — · — |
| InfoTip | 43.1 kB (+94 B) · 5.3 kB CSS (0) | — | — · — · — |
| TimeInput | 144.0 kB (+94 B) · 15.1 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 74.7 kB (+94 B) · 773 B CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 239.6 kB (+94 B) · 454 B CSS (0) | — | — · — · — |
| Link | 48.5 kB (+93 B) · 6.9 kB CSS (0) | — | — · — · — |
| Markdown | 161.4 kB (+93 B) · 19.5 kB CSS (0) | — | — · — · — |
| TopNavMegaMenu | 61.0 kB (+93 B) · 11.2 kB CSS (0) | — | — · — · — |
| CheckboxInput | 60.0 kB (+92 B) · 9.0 kB CSS (0) | — | — · — · — |
| Lightbox | 59.7 kB (+92 B) · 9.7 kB CSS (0) | — | — · — · — |
| Slider | 97.0 kB (+92 B) · 8.3 kB CSS (0) | — | — · — · — |
| lab/Schedule | 98.8 kB (+92 B) · 3.9 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditor | 290.7 kB (+92 B) · 2.6 kB CSS (0) | — | — · — · — |
| charts/ChartTooltip | 151.1 kB (+91 B) · 423 B CSS (0) | — | — · — · — |
| MoreMenu | 86.1 kB (+91 B) · 16.6 kB CSS (0) | — | — · — · — |
| StatusDot | 27.6 kB (+91 B) · 5.3 kB CSS (0) | — | — · — · — |
| TextArea | 53.9 kB (+91 B) · 8.9 kB CSS (0) | — | — · — · — |
| Tokenizer | 80.3 kB (+91 B) · 17.8 kB CSS (0) | — | — · — · — |
| TopNavMenu | 65.9 kB (+91 B) · 10.5 kB CSS (0) | — | — · — · — |
| lab/Comments | 281.8 kB (+91 B) · 2.5 kB CSS (0) | — | — · — · — |
| Avatar | 39.5 kB (+90 B) · 5.5 kB CSS (0) | — | — · — · — |
| SideNav | 81.6 kB (+90 B) · 10.7 kB CSS (0) | — | — · — · — |
| SideNavItem | 61.6 kB (+90 B) · 10.4 kB CSS (0) | — | — · — · — |
| RadioList | 48.3 kB (+89 B) · 6.8 kB CSS (0) | — | — · — · — |
| Table | 99.6 kB (+89 B) · 17.2 kB CSS (0) | — | — · — · — |
| Toolbar | 39.5 kB (+89 B) · 7.9 kB CSS (0) | — | — · — · — |
| AlertDialog | 95.6 kB (+88 B) · 13.7 kB CSS (0) | — | — · — · — |
| MobileNav | 61.5 kB (+88 B) · 9.9 kB CSS (0) | — | — · — · — |
| AppShell | 81.5 kB (+87 B) · 12.8 kB CSS (0) | — | — · — · — |
| ComplexSelector | 62.5 kB (+87 B) · 11.0 kB CSS (0) | — | — · — · — |
| NumberInput | 62.8 kB (+87 B) · 10.7 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 276.0 kB (+87 B) · 2.4 kB CSS (0) | — | — · — · — |
| richtext/RichTextView | 271.0 kB (+87 B) · 2.3 kB CSS (0) | — | — · — · — |
| Field | 46.8 kB (+86 B) · 6.7 kB CSS (0) | — | — · — · — |
| FileInput | 60.3 kB (+86 B) · 10.3 kB CSS (0) | — | — · — · — |
| HoverCard | 24.3 kB (+86 B) · 4.3 kB CSS (0) | — | — · — · — |
| ChatLayout | 54.9 kB (+85 B) · 9.8 kB CSS (0) | — | — · — · — |
| InputGroup | 48.6 kB (+85 B) · 7.3 kB CSS (0) | — | — · — · — |
| TopNav | 63.1 kB (+85 B) · 10.2 kB CSS (0) | — | — · — · — |
| lab/CommentPin | 52.7 kB (+85 B) · 1.2 kB CSS (0) | — | — · — · — |
| TextInput | 59.4 kB (+84 B) · 10.5 kB CSS (0) | — | — · — · — |
| Thumbnail | 56.0 kB (+84 B) · 10.9 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 115.4 kB (+84 B) · 1.9 kB CSS (0) | — | — · — · — |
| CheckboxList | 50.1 kB (+83 B) · 7.5 kB CSS (0) | — | — · — · — |
| TabList | 40.2 kB (+82 B) · 6.4 kB CSS (0) | — | — · — · — |
| ToggleButton | 52.5 kB (+82 B) · 9.4 kB CSS (0) | — | — · — · — |
| lab/SelectableCardGroup | 56.2 kB (+82 B) · 162 B CSS (0) | — | — · — · — |
| BreadcrumbItem | 75.8 kB (+81 B) · 11.5 kB CSS (0) | — | — · — · — |
| SideNavCollapseButton | 52.5 kB (+81 B) · 9.4 kB CSS (0) | — | — · — · — |
| Banner | 53.6 kB (+80 B) · 9.8 kB CSS (0) | — | — · — · — |
| ChatSendButton | 52.1 kB (+80 B) · 9.3 kB CSS (0) | — | — · — · — |
| IconButton | 51.7 kB (+80 B) · 9.3 kB CSS (0) | — | — · — · — |
| MobileNavToggle | 52.0 kB (+80 B) · 9.3 kB CSS (0) | — | — · — · — |
| lab/EmojiPicker | 77.6 kB (+80 B) · 751 B CSS (0) | — | — · — · — |
| Button | 51.6 kB (+79 B) · 9.3 kB CSS (0) | — | — · — · — |
| Calendar | 62.4 kB (+79 B) · 9.9 kB CSS (0) | — | — · — · — |
| ChatDictationButton | 52.2 kB (+79 B) · 9.4 kB CSS (0) | — | — · — · — |
| ChatLayoutScrollButton | 52.1 kB (+79 B) · 9.5 kB CSS (0) | — | — · — · — |
| AvatarGroup | 42.2 kB (+78 B) · 5.6 kB CSS (0) | — | — · — · — |
| DrawerHeader | 54.6 kB (+78 B) · 12.2 kB CSS (0) | — | — · — · — |
| TableSelectionToolbar | 61.2 kB (+78 B) · 12.4 kB CSS (0) | — | — · — · — |
| lab/DiffViewer | 78.0 kB (+78 B) · 2.1 kB CSS (0) | — | — · — · — |
| CodeBlock | 66.2 kB (+75 B) · 10.6 kB CSS (0) | — | — · — · — |
| DialogHeader | 54.4 kB (+74 B) · 10.8 kB CSS (0) | — | — · — · — |
| Stepper | 53.3 kB (+74 B) · 9.5 kB CSS (0) | — | — · — · — |
| Spinner | 20.3 kB (+1 B) · 6.1 kB CSS (0) | — | — · — · — |
| Timer | 9.6 kB (+1 B) · 5.9 kB CSS (0) | — | — · — · — |
| charts/ChartLegend | 35.6 kB (0) · 224 B CSS (0) | — | — · — · — |
| CommandPalette | 59.3 kB (0) · 12.4 kB CSS (0) | — | — · — · — |
| CommandPaletteInput | 27.9 kB (0) · 7.5 kB CSS (0) | — | — · — · — |
| Heading | 8.0 kB (0) · 5.3 kB CSS (0) | — | — · — · — |
| ProgressBar | 11.5 kB (0) · 6.2 kB CSS (0) | — | — · — · — |
| Timestamp | 23.0 kB (0) · 9.6 kB CSS (0) | — | — · — · — |
| ChatMessageList | 23.8 kB (−1 B) · 6.9 kB CSS (0) | — | — · — · — |
| ChatToolCalls | 27.7 kB (−1 B) · 8.3 kB CSS (0) | — | — · — · — |
| Text | 7.5 kB (−2 B) · 5.3 kB CSS (0) | — | — · — · — |
| layer | — | — | — · ✓ · — |
| Layer | — | — | ✓ · ✓ · — |
Package size: @astryxdesign/core 6.69 MB packed (+896 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 19 rows, core 21 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
There was a problem hiding this comment.
Requesting changes. Moves are followed on the same frames as before in Chromium, Firefox and WebKit, so the observer change holds up; two small fixes are needed first.
- The new test's
spy.mock.contexts.filter(el => !el.hasAttribute('popover'))(packages/core/src/Layer/useLayer.test.tsx:3094) fails the core typecheck with TS18046 becauseelisunknown, so cast it, for example!(el as HTMLElement).hasAttribute('popover'). - The changeset says following a Tooltip's trigger through a scroll "no longer reads layout", but each followed move still reads the trigger, the layer and the viewport in the next frame to keep the Tooltip clear of its trigger, so please scope the changeset (and the title) to the observer's re-arm.
Non-blocking: the comment "An engine that reports no root box leaves the size to one read." (useLayer.tsx:658) could read "If the report has no root box, read the viewport's size."
[Automated review]
| const width = vi.spyOn(document.documentElement, 'clientWidth', 'get'); | ||
| const height = vi.spyOn(document.documentElement, 'clientHeight', 'get'); | ||
| const triggerReads = () => | ||
| spy.mock.contexts.filter(el => !el.hasAttribute('popover')).length; |
There was a problem hiding this comment.
tsc --project tsconfig.json --noEmit in packages/core fails here with TS18046 ("'el' is of type 'unknown'"): the spy's mock.contexts is unknown[]. It passes with the base versions of the Layer files. A cast fixes it:
spy.mock.contexts.filter(el => !(el as HTMLElement).hasAttribute('popover')).length;| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [perf] While a Tooltip is open, following its trigger through a scroll no longer reads layout. Each frame of a scroll used to read the trigger's box once or twice and the viewport's size to re-arm the observer that follows the trigger. The box and the viewport now come from the observer's own report. The Tooltip follows its trigger on the same frames as before. |
There was a problem hiding this comment.
This claims more than the change does. Only the observer's re-arm stops reading layout: each followed move still runs keepAnchorClear in the next frame (useLayer.tsx:789-796), which reads the trigger's box, the layer's box and the viewport's height (useLayer.tsx:524-539), as useLayer.test.tsx:3181-3184 asserts. Suggested wording:
[perf] While a Tooltip is open, the observer that follows its trigger through a scroll no longer reads layout to re-arm. It used to read the trigger's box once or twice and the viewport's size on each frame of a scroll. It now takes them from its own report. The Tooltip still checks its placement once per move, on the same frames as before.
| now.width !== rect.width || | ||
| now.height !== rect.height; | ||
| // A report's root box is the viewport, here shrunk by the root margin. | ||
| // An engine that reports no root box leaves the size to one read. |
There was a problem hiding this comment.
Non-blocking (comment style): "leaves the size to one read" is vague. Suggested:
// If the report has no root box, read the viewport's size.There was a problem hiding this comment.
Requesting changes. The typecheck fix in the new test works, and the observer change still follows moves on the same frames as before; one fix remains.
- The changeset says following a Tooltip's trigger through a scroll "no longer reads layout", but each followed move still reads the trigger, the layer and the viewport in the next frame to keep the Tooltip clear of its trigger (
packages/core/src/Layer/useLayer.tsx:774-796), so please scope the changeset (.changeset/layer-move-observer-entry-box.md:5) and the title to the observer's re-arm.
Non-blocking: the comment "An engine that reports no root box leaves the size to one read." (packages/core/src/Layer/useLayer.tsx:658) could read "If the report has no root box, read the viewport's size."
[Automated review]
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet because the pull request is a draft; it runs once the pull request is marked ready for review.
The observer now re-arms from its own reports and follows moves on the same frames as before in Chromium, Firefox and WebKit, and the changeset now describes exactly that change.
[Automated review]
Maintainer impact
While a Tooltip is open, the layer runtime follows its trigger with an IntersectionObserver whose root is the viewport shrunk to the trigger's rectangle; every move re-arms it on the new rectangle. Each re-arm read the trigger's box (
getBoundingClientRect) and the viewport'sclientWidthandclientHeight, and a report at ratio 1 read the box once more to look for a move under a pixel. So every frame of a scroll with a Tooltip open read layout once or twice per trigger, on the main thread, to rebuild an observer.Intended invariant
architecture:react-component-runtimeINV18 (a scroll path reads no layout; pending in docs(react-component-runtime): read layout as little as possible #10219).spec:AST-059FR8 andarchitecture:layer-runtimeINV12: a same-size anchor that moves is learned from an intersection observer, re-armed on each report that differs from its threshold; a clipped anchor waits on a second observer; no timer. These are silent on how the observer is armed and stay true.No behavior change: the observer reports the same moves on the same frames, and the layer is read and placed as before.
Change
An intersection report already carries both values:
boundingClientRect(the target's box) androotBounds(the root's box, here the viewport shrunk by the root margin the observer was armed with, so the viewport's size isrootBounds.right + right marginandrootBounds.bottom + bottom margin). Re-arming and the under-a-pixel check now use them. The box and the viewport are read once, when observing starts. An engine that reports no root box falls back to reading the viewport's size. Each followed move still checks the Tooltip's placement in the next frame (keepAnchorClearreads the trigger, the layer and the viewport's height), as before; only the re-arm stops reading layout.Evidence
Failure before the change: the new
useLayer.test.tsxcase "follows a move from its report alone: no layout read, armed on the reported box" fails onmain(the re-arm reads the trigger's box and the viewport).Success after the change, measured in Chromium (Playwright's headless Chromium) with the shipped
observeMovefrommainand from this branch on two anchors, through a scripted 60-frame scroll, a layout shift, and an inner scroller that clips one anchor and brings it back (170 frames, 91 moves):getBoundingClientRectclientWidth/clientHeightThe old reads did not force layout (the callback runs after paint), so this removes main-thread reads, not forced layouts.
Product/runtime behavior verified unchanged: in a separate run, both observers ran side by side on the same anchors through the same script; they reported the same 151 moves on the same frames (3 of 3 runs; only the order of the two anchors within a frame differs). In Chromium the report's box equals a fresh
getBoundingClientRectand the viewport fromrootBoundsequalsclientWidth/clientHeight, with and without a 15 px classic scrollbar and at device scale 1, 1.5 and 2. The one exception is a report whose target moved again between the observation and its callback; the new observer's first report then sees that move, as the comment describes, and follows it.Existing
useLayer.test.tsxmove, clip and next-frame cases pass, now fed reports that carry a box and a root box as an engine sends them.Scope
Testing
vitest run --project ui packages/core/src/Layer/useLayer.test.tsx(112 tests)pnpm review:pre: 0 findings.pnpm specs:for --diff origin/main: AST-059 FR8 and layer-runtime INV12 keep their wording.