fix: deliver toasts queued before Toaster subscribes (#723) - #760
Open
rolkec wants to merge 3 commits into
Open
Conversation
… mounts are not lost Closes emilkowalski#723 When `toast()` is called from a component that renders/mounts before `<Toaster />` (e.g. a sibling's `useEffect` running first), the toast is added to `ToastState.toasts` but `publish()` has no subscribers yet, so the toast is never displayed. The only workarounds were `setTimeout(0)` or re-ordering components. Fix: on `subscribe()`, replay any non-dismissed toasts already in the queue to the newly attached subscriber. Also guards the `unsubscribe` splice against `indexOf` returning -1 (which previously would silently remove the last subscriber). Adds a regression test at /issue-723 that reproduces the original race.
…s don't churn Companion to the previous commit. The Toaster's subscribe useEffect used `[toasts]` as its dependency array, but the setter inside is a functional update (`setToasts((toasts) => ...)`) and does not close over the value, so the dep was unnecessary. With the new replay-on-subscribe behavior from the previous commit, this stale dep caused an infinite loop: every state change triggered a re-subscribe, which replayed the queued toast, which produced a new state update, which triggered another re-subscribe, and so on. The per-toast auto-dismiss timer in Toast.tsx has `toast` in its deps, so it was cancelled and restarted on every iteration and never fired -- visible symptom: toast appears but never auto-dismisses. Independently of the replay fix, the old `[toasts]` dep also created a race where toasts published between an unsubscribe and the matching re-subscribe could be lost.
…nvoke Next dev mode runs `useEffect` twice on mount; without a guard the fixture fired two toasts and the regression assertion `toHaveCount(1)` became environment-dependent. A `useRef` "already fired" check makes the page render exactly one toast under both dev and prod builds, which matches how applications usually structure on-mount side effects.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #723.
Problem
toast(...)called before<Toaster />mounts (e.g. from a siblingcomponent's
useEffect, which runs before the parent's) was silentlydropped.
Observer.addToastpublishes tosubscribers, butToaster.subscribeonly attaches inside its ownuseEffect, so anytoast added before that ran was never delivered. The user could see it
in
ToastState.toastsbut it never reached the DOM.The only workarounds were
setTimeout(0)around thetoast()call orre-ordering the component tree so
<Toaster />mounts first — bothdocumented in the issue, neither pleasant.
Fix
Two small, related changes:
src/state.ts— replay queued toasts on subscribe. When a newsubscriber attaches, deliver any non-dismissed toasts already in
this.toasts. Also guards the unsubscribespliceagainstindexOfreturning-1(previously silently removed the lastsubscriber).
src/index.tsx— drop the stale[toasts]dep on the Toaster'ssubscribe effect. The setter is a functional update and doesn't
need the value in deps. Without this, change 1 caused an infinite
subscribe/replay/setState loop because every state change
re-triggered the effect and re-replayed the queue. The per-toast
auto-dismiss timer (which has
toastin its deps) was cancelled andrestarted on each iteration, so toasts appeared but never dismissed.
Independently of change 1, the old
[toasts]dep also created arace where any toast published between an unsubscribe and the
matching re-subscribe could be lost.
Test plan
/issue-723route intest/src/app/that reproduces theoriginal race: a sibling component fires
toast()from itsuseEffectwhile rendered above<Toaster />.test/tests/basic.spec.ts.pnpm --filter test build && pnpm --filter test start): toast appears and auto-dismisses afterthe default 4 s.