Repository navigation
perf(renderer): load Radix primitives through public package entries - #26256
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (30)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughRenderer components now import Radix primitives from component-specific subpaths instead of the package root. Components that use Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to This change loads Radix primitives through their dedicated package entries to reduce load work. No behavior change or merge-blocking risk is evident. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1ab47cf to
9a0dbbd
Compare
9a0dbbd to
ba90fc7
Compare
ELI5
Loading a small UI component currently pulls in the broad Radix entry during tests. Use each component's published package entry to reduce that import work while retaining its implementation.
What Changed
Replace 30
radix-uiroot imports with publicradix-ui/<primitive>entries. Component bodies, styles, refs, events, dependencies and lockfile remain byte-identical. The entries resolve to the same primitive implementations, including cmdk's separate Dialog version.Why
These existing public entries remove unnecessary imports without adding a transform, dependency or isolation exception. A separate three-suite import observation retained Slot and removed the root entry; it did not trace native package internals or establish exclusive import costs.
Linked Issue
Related test performance work: #26250 and #26237. Incoming manifest lint blocker repaired in #26342.
Visual Proof
N/A — import paths only; no appearance or interaction change.
Testing
Before/after measurements below use frozen source
76d1b7bf29932f6e1edfb6b0b34d65c5c88e5f55, unchanged project setup and routing, isolation, four workers and filesystem module cache disabled. Every sample measures a completepnpm testcommand through normal exit. Exact case/status maps, zero errors and process checks passed. All first, slower and regressing samples are retained; no warmup, retry or discard.The broader measured cohort contains 130 original cases across 20 existing DOM suites:
All three broader pairs improved. The slower first qualified baseline is retained and contributes substantially to the mean; the other paired improvements are 3.57% and 2.95%. No sample was rejected, retried or discarded.
The first, smaller cohort contained 17 original cases across three Slot-heavy suites. Its mixed result prompted the broader measurement; every original sample remains here:
The third small-cohort pair regressed by 4.83%; it remains included. These cohorts do not measure the marginal benefit of all 30 changes against a Button-only alternative, stock warm-cache CI, or the complete test suite. They do not establish the overall 2× goal.
7abd74e8187d39cbdc8ff127969c5cd384a2a20b; new headba90fc7994e823d6ac4f557e689d155e41ec9bea. All 30 incoming and candidate files retain their qualified bytes. Upstream work is preserved.pnpm tc:webpass locally on macOS with background launch policy.The timings retain their original source labels; they are not measurements of the newer complete main tree. No native integration or app was launched on the user's desktop.
Review
Independent source, import, correctness and all 12 timing reviews passed. Root verifies the 30 exact files after this rebase. CodeRabbit's review covered older head
c3e400b3with no actionable findings; bot coverage of the new head is not assumed. Fresh current-head checks passed; no unresolved review threads or actionable findings remain. Independent source review also verifies all 30 namespace uses retain their implementation bindings. No fresh bot coverage is claimed.Agent skill upstream boundary
Notes
No SSH execution, folder/worktree ownership, remote protocol, platform behavior or runtime-authority changes. Import overlap and mocks prevent a whole-suite forecast. These measurements do not establish the overall 2× goal.
All four earlier failed full-CI attempts remain failures and are preserved:
The separate Windows diagnostic first rejected LF/CRLF source preflight before launching contracts; the corrected definition ran all 60 native files successfully but its frozen collector rejected an inactive project name. Independent raw analysis passed; that workflow remains failed, the original Windows timeout was not reproduced, and no Windows repair is claimed. The new integration includes actual source changes since each failed attempt; it is not an unchanged retry or a repair claim for unknown failures.
Checklist
Pre-merge main also contains relay-only #26350 (
a07f4b2cbd95), whose ten cloud/workflow paths do not overlap the 30 import targets or test runtime configuration. All 30 incoming bytes remain exact. CI above covers the qualified PR head; it does not cover this later relay-only commit as part of the eventual combined squash tree.Merged squash
de4ee59cba964d45489a2fbc3ab1a2f7937557aahas actual parentb1e09a4c8e8a9ebfaa3b798692c5d6c5412b4b02. Root checked that exactly the 30 qualified target paths changed and every before/after byte matches. Between pre-merge inspection and squash, main gained #25845 (OpenCode structured chat) and #26334 (remote chat history), in addition to relay-only #26350. The actual parent therefore includes all three later upstream commits. Every one of the 30 incoming and candidate files remains exact. Qualified-head CI does not cover these extra commits as part of the combined squash tree.