refactor: replace toast bridge with RTK listener middleware#37
refactor: replace toast bridge with RTK listener middleware#37ryota-murakami merged 5 commits intomainfrom
Conversation
Closes #35 clearComponentHistory cleared renderHistory and renderCounts but left renderCountsByReason stale, creating a data inconsistency after reset.
Closes #33 Closes #36 - Create listenerMiddleware.ts that intercepts recordRender actions and dispatches toast notifications with 300ms batching - Remove lastRender relay slot from renderTrackerSlice (resolves #36 single-slot overwrite risk) - Delete useReRenderToasts hook (no longer needed) - Remove useReRenderToasts() call from AppShell in layout.tsx - Add listener middleware to store via .prepend() The suppressToasts/triple-setTimeout timing in useSuppressToasts is retained — it's inherent to useRenderTracker's double-setTimeout dispatch pattern and unrelated to the toast bridge mechanism.
Closes #34 Move suppressToasts state, beginSuppressToasts, and endSuppressToasts from renderTrackerSlice to toastSlice. renderTrackerSlice now has a single responsibility: recording render events (history, counts, per-reason breakdowns). Toast suppression belongs to the toast domain. Consumer code unchanged — all imports use @/store barrel export.
The footer added in PR #32 introduced a second "React Docs" link, causing Playwright strict mode to fail on getByRole. Scope selectors to the hero <header> element.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR replaces React hook-based toast signaling with Redux Toolkit listener middleware, removing the useReRenderToasts hook and its fragile triple-setTimeout pattern. Toast suppression state moves from renderTrackerSlice to toastSlice, and a new middleware handles render-to-toast conversion with 300ms debouncing. RenderTrackerSlice is simplified by removing lastRender and suppressToasts fields. E2E tests are refactored for improved selector scoping. Changes
Sequence DiagramsequenceDiagram
participant Component
participant Store as Redux Store
participant Middleware as Listener Middleware
participant Toast as Toast Slice
rect rgba(100, 150, 200, 0.5)
Note over Component,Toast: Old Flow (Hook-based)
Component->>Store: dispatch recordRender()
Store->>Component: Update lastRender state
Component->>Component: useReRenderToasts reads lastRender<br/>(triple-setTimeout race)
Component->>Toast: dispatch addToast/addBatchToast()
end
rect rgba(200, 150, 100, 0.5)
Note over Component,Toast: New Flow (Middleware-based)
Component->>Store: dispatch recordRender()
Middleware->>Middleware: Buffer render event<br/>(300ms debounce)
Middleware->>Middleware: On debounce timeout:<br/>flush buffer
alt Single render event
Middleware->>Toast: dispatch addToast()
else Multiple render events
Middleware->>Toast: dispatch addBatchToast()
end
Toast->>Toast: Update toast state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/store/listenerMiddleware.ts (1)
32-33: Module-levelbuffer/flushTimerstate undermines testability (issue#33acceptance criteria)
bufferandflushTimerlive at module scope, meaning they are shared across all store instantiations within the same process. In tests that create multiple stores or that import this module withoutjest.resetModules()between runs, stale buffer contents or dangling timers from one test can bleed into the next — violating the "make toast logic testable and robust" requirement from issue#33.The RTK-idiomatic debounce (
cancelActiveListeners+listenerApi.delay()) doesn't directly support accumulate-all batching, so module-level state is the pragmatic choice here. At minimum, export a reset helper that tests can call inafterEach:♻️ Suggested testability escape hatch
/** Module-level buffer for batching render events within the debounce window */ let buffer: RenderInfo[] = [] let flushTimer: ReturnType<typeof setTimeout> | null = null +/** Reset internal debounce state — use in test teardown (afterEach) only. */ +export const _resetBufferForTesting = () => { + buffer = [] + if (flushTimer !== null) { + clearTimeout(flushTimer) + flushTimer = null + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/listenerMiddleware.ts` around lines 32 - 33, Module-level shared state buffer and flushTimer cause cross-test leakage; add and export a reset helper (e.g. resetListenerMiddlewareState) that clears the buffer array (buffer.length = 0) and cancels any pending timer (clearTimeout(flushTimer)) then sets flushTimer = null so tests can call it in afterEach; implement the helper in the same module where buffer and flushTimer are declared and keep types consistent (ReturnType<typeof setTimeout> | null) so callers can reliably reset state between test runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/store/listenerMiddleware.ts`:
- Around line 47-57: The flush callback for flushTimer currently dispatches
addToast/addBatchToast using the buffer captured at recordRender time without
re-checking getState().toast.suppressToasts, allowing a race where
suppressToasts can be set true after buffering but before flush; modify the
setTimeout callback in listenerMiddleware (the block that uses flushTimer,
buffer and BATCH_DEBOUNCE_MS) to call getState().toast.suppressToasts again at
the top of the callback and bail out (do not dispatch) if it is true, leaving
buffer cleared or handled as desired to prevent emitting toasts while
suppressed.
---
Nitpick comments:
In `@src/store/listenerMiddleware.ts`:
- Around line 32-33: Module-level shared state buffer and flushTimer cause
cross-test leakage; add and export a reset helper (e.g.
resetListenerMiddlewareState) that clears the buffer array (buffer.length = 0)
and cancels any pending timer (clearTimeout(flushTimer)) then sets flushTimer =
null so tests can call it in afterEach; implement the helper in the same module
where buffer and flushTimer are declared and keep types consistent
(ReturnType<typeof setTimeout> | null) so callers can reliably reset state
between test runs.
Addresses CodeRabbit review: suppressToasts was only checked at event arrival, not at the 300ms flush. A UI chrome action between buffering and flushing could bypass suppression. Now re-checks getState() at flush time.
1 participant
Summary
Architecture refactor of the Redux render-tracker/toaster pipeline, replacing the
timing-based hook bridge with RTK listener middleware for cleaner separation of concerns.
Changes per Issue
Issue #35: Bug fix
delete state.renderCountsByReason[name]inclearComponentHistoryreducerIssue #33: Listener middleware (main refactor)
src/store/listenerMiddleware.ts— interceptsrecordRenderactions, 300ms batchinglastRenderrelay slot fromrenderTrackerSlice(resolves refactor: replace lastRender single-slot with event queue or middleware #36)useReRenderToastshook (replaced by middleware)useReRenderToasts()call fromAppShellinlayout.tsx.prepend()Issue #34: Slice separation
suppressToasts,beginSuppressToasts,endSuppressToastsfromrenderTrackerSlicetotoastSlicerenderTrackerSlicenow has single responsibility: render data recording@/store)E2E fix
landing-page.spec.tsto avoid duplicate "React Docs" link matchVerification
Summary by CodeRabbit
Refactor
Tests