perf(richtext-lexical): 3-15x less main thread blocking via centralized toolbar state#15832
perf(richtext-lexical): 3-15x less main thread blocking via centralized toolbar state#15832
Conversation
packages/richtext-lexical/src/features/toolbars/fixed/client/Toolbar/index.tsx
Show resolved
Hide resolved
| * Maximum number of active items allowed. This is a performance optimization to prevent | ||
| * unnecessary item active checks when the maximum number of active items is reached. | ||
| */ | ||
| maxActiveItems?: number |
There was a problem hiding this comment.
This functionality still exists, it was just moved to useToolbarStates
There was a problem hiding this comment.
The problem is that we're exporting this component, so it's a breaking change.
📦 esbuild Bundle Analysis for payloadThis analysis was generated by esbuild-bundle-analyzer. 🤖
Largest pathsThese visualization shows top 20 largest paths in the bundle.Meta file: packages/next/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_shared.json, Out file: esbuild/exports/shared.js
Meta file: packages/richtext-lexical/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_shared.json, Out file: esbuild/exports/shared_optimized/index.js
DetailsNext to the size is how much the size has increased or decreased compared with the base branch of this PR.
|
There was a problem hiding this comment.
Pull request overview
This PR reduces richtext-lexical toolbar rendering overhead by centralizing toolbar active/enabled state computation into a single hook, and adds a Playwright-based benchmark suite to track editor typing performance under CPU throttling.
Changes:
- Introduces
useToolbarStatesto compute all toolbar group/item states from a single Lexical update subscription. - Refactors fixed + inline toolbars to pass centralized state into stateless
ToolbarButton/ToolbarDropdown. - Adds a Lexical benchmark collection + seeded doc and a new Playwright benchmark spec.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/runE2E.ts | Removes --start-memory-db from spawned dev args. |
| test/playwright.config.ts | Expands Playwright testMatch to include *perf.spec.ts. |
| test/lexical/slugs.ts | Adds lexicalBenchmarkSlug. |
| test/lexical/seed.ts | Seeds a benchmark Lexical doc containing 30 blocks. |
| test/lexical/payload-types.ts | Updates generated types for the new benchmark collection. |
| test/lexical/collections/LexicalBenchmark/index.ts | Adds a new LexicalBenchmark collection config with Blocks + FixedToolbar features. |
| test/lexical/collections/Lexical/e2e/main/e2e.spec.ts | Makes internal link option selection more robust by filtering by text. |
| test/lexical/benchmarks/perf.spec.ts | Adds CPU-throttled typing benchmark tests + summary output. |
| test/lexical/baseConfig.ts | Registers LexicalBenchmark in the lexical test config. |
| packages/richtext-lexical/src/features/toolbars/shared/useToolbarStates.ts | New centralized state hook for toolbar items/groups. |
| packages/richtext-lexical/src/features/toolbars/shared/ToolbarDropdown/index.tsx | Refactors dropdown to be state-less and consume groupState. |
| packages/richtext-lexical/src/features/toolbars/shared/ToolbarButton/index.tsx | Refactors button to be state-less and consume active/enabled. |
| packages/richtext-lexical/src/features/toolbars/inline/client/Toolbar/index.tsx | Wires inline toolbar to useToolbarStates. |
| packages/richtext-lexical/src/features/toolbars/fixed/client/Toolbar/index.tsx | Wires fixed toolbar to useToolbarStates. |
Comments suppressed due to low confidence (1)
test/lexical/benchmarks/perf.spec.ts:111
p95Lagcalculation usessorted[Math.floor(lags.length * 0.95)], which is off by one for typical percentile definitions (and can skew the reported p95, especially for small sample sizes). Consider using an index based on(lags.length - 1) * 0.95(orMath.ceil(lags.length * 0.95) - 1) to better approximate the 95th percentile.
const sorted = [...lags].sort((a, b) => a - b)
const avgLag = lags.length > 0 ? lags.reduce((a, b) => a + b, 0) / lags.length : 0
const p95Lag = lags.length > 0 ? (sorted[Math.floor(lags.length * 0.95)] ?? Math.max(...lags)) : 0
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const ToolbarDropdown = ({ | ||
| anchorElem, | ||
| classNames, | ||
| editor, | ||
| group, | ||
| groupState, | ||
| Icon, | ||
| itemsContainerClassNames, | ||
| label, | ||
| maxActiveItems, | ||
| onActiveChange, | ||
| }: { | ||
| anchorElem: HTMLElement | ||
| classNames?: string[] | ||
| editor: LexicalEditor | ||
| group: ToolbarDropdownGroup | ||
| groupState: ToolbarGroupState | ||
| Icon?: React.FC | ||
| itemsContainerClassNames?: string[] | ||
| label?: string |
There was a problem hiding this comment.
ToolbarDropdown now requires a groupState prop and removes maxActiveItems / onActiveChange. Because ToolbarDropdown is exported from the package client exports, this is a breaking API change for consumers that render dropdowns directly. Consider making groupState optional (with an internal fallback to compute state), or introducing a new internal-only component and keeping the exported component’s previous props supported.
| // Look for test files in the "test" directory, relative to this configuration file | ||
| testDir: '', | ||
| testMatch: '*e2e.spec.ts', | ||
| testMatch: ['*e2e.spec.ts', '*perf.spec.ts'], |
There was a problem hiding this comment.
Including *perf.spec.ts in the default testMatch means the CPU-throttled benchmark suite will run as part of every E2E run/CI run. These tests are intentionally long and performance-sensitive, so this change can significantly increase CI duration and introduce noise/flakiness. Consider moving benchmarks under a separate Playwright config/project, or gating them behind an env var / --grep so they only run when explicitly requested.
There was a problem hiding this comment.
Yeah, I've already validated it. I would actually remove the benchmark. No point in merging it into main. What do you think @AlessioGr?
| active, | ||
| children, | ||
| editor, | ||
| enabled, | ||
| item, | ||
| }: { | ||
| active: boolean | ||
| children: React.JSX.Element | ||
| editor: LexicalEditor | ||
| enabled: boolean |
There was a problem hiding this comment.
ToolbarButton now requires active and enabled props. Since this component is exported from the package client exports, this is a breaking public API change for any consumers rendering ToolbarButton directly. Consider keeping backward compatibility by making these props optional with safe defaults (e.g. active=false, enabled=true) and/or providing a fallback state calculation when they aren’t supplied, or stop exporting this internal component.
| active, | |
| children, | |
| editor, | |
| enabled, | |
| item, | |
| }: { | |
| active: boolean | |
| children: React.JSX.Element | |
| editor: LexicalEditor | |
| enabled: boolean | |
| active = false, | |
| children, | |
| editor, | |
| enabled = true, | |
| item, | |
| }: { | |
| active?: boolean | |
| children: React.JSX.Element | |
| editor: LexicalEditor | |
| enabled?: boolean |
There was a problem hiding this comment.
Yes. Similar to my other comment
GermanJablo
left a comment
There was a problem hiding this comment.
Almost good to go. I would remove the benchmark and avoid the breaking change.
Replaces per-component
editor.registerUpdateListenersubscriptions in toolbar buttons and dropdowns with a single centralizeduseToolbarStateshook that computes all active/enabled states in one pass. Previously every keystroke triggered 15+ independent React state updates across toolbar items - now it triggers one.ToolbarButtonandToolbarDropdownare now stateless presentational components. The dropdown icon/label derivation that previously used auseState/useEffect/onActiveChangecallback loop is replaced with a directuseMemofromgroupState.activeItems.Also adds a dedicated benchmark suite (
test/lexical/benchmarks/) with CPU-throttled typing for tracking performance.Benchmark Results (6x CPU throttle, 5 iterations)
Empty Editor
With 30 Blocks
Raw Results
Before:
After:
Main thread blocking dropped 3x with 30 blocks in the editor and 15x on an empty editor. I ran this test multiple times, and the results for both before and after were consistently in a similar range.
In practice, typing in the editor should feel smoother, especially on slower devices or in documents with lots of blocks and fields.