refactor(app): extract layout side effect controllers#877
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 40 minutes and 52 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/pages/layout.tsx, packages/app/src/pages/layout/layout-sdk-event-effects.test.ts, packages/app/src/pages/layout/layout-sdk-event-effects.ts, packages/app/src/pages/layout/layout-update-polling.ts, packages/app/src/pages/update-install-flow-source.test.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request refactors the main Layout component by extracting the update polling and SDK notification logic into separate, testable modules. New unit tests have been introduced for the SDK event effects. The reviewer suggested optimizing performance by deferring session lookups until they are strictly necessary and using targeted lookups to minimize memory overhead when processing events.
Perf delta summaryComparator: pass
|
Summary
Extract the first non-visual side-effect controller boundary from
packages/app/src/pages/layout.tsx.layout-update-polling.ts.layout-sdk-event-effects.ts.Why
layout.tsxwas mixing shell composition with background update and SDK event behavior. This PR keeps the visible shell stable while making those side effects easier to review and safer to change later.Related Issue
Refs #875
Human Review Status
Pending
Review Focus
Please focus on behavior preservation: Solid owner lifecycle, reactive accessor usage, SDK listener cleanup, update polling interval cleanup, notification dedupe/cooldown, and worktree ready/failed effects.
Risk Notes
How To Verify
Screenshots or Recordings
Not required. This PR has no visible UI change.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.