fix(react): set preferences/schedule data directly on fetch to prevent undefined state on re-mount#11756
Conversation
…t undefined state on re-mount Fixes novuhq#10057
👷 Deploy request for dashboard-v2-novu-staging pending review.Visit the deploys page to approve it
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a re-mount edge case in packages/react where usePreferences / useSchedule could miss synchronous mitt events emitted from a warm in-memory cache, leaving preferences/schedule permanently undefined after unmount + re-mount. The change aligns these hooks with useNotifications by hydrating state directly from the fetch response instead of relying solely on event listeners.
Changes:
- Update
usePreferencesto callsetData(response.data)on successful fetch (and remove the unsaferesponse.data!path). - Update
useScheduleto callsetData(response.data)on successful fetch (and remove the unsaferesponse.data!path).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/src/hooks/usePreferences.ts | Sets preferences state directly from preferences.list() response to avoid missing synchronous cache events on re-mount. |
| packages/react/src/hooks/useSchedule.ts | Sets schedule state directly from preferences.schedule.get() response to avoid missing synchronous cache events on re-mount. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const response = await preferences.schedule.get(); | ||
| if (response.error) { | ||
| setError(response.error); | ||
| onError?.(response.error); | ||
| } else { | ||
| onSuccess?.(response.data!); | ||
| } else if (response.data) { |
| const response = await preferences.list(props?.filter); | ||
| if (response.error) { | ||
| setError(response.error); | ||
| onError?.(response.error); | ||
| } else { | ||
| onSuccess?.(response.data!); | ||
| } else if (response.data) { |
|
T-Rex pricing update — T-Rex was free through June 2026. Effective July 1, 2026, T-Rex adds 2 credits on top of the standard 1-credit review (3 total). T-Rex settings |
Summary
Fixes #10057
usePreferences(and the siblinguseSchedule) can get permanently stuck in a broken state after a component that uses them unmounts and re-mounts:isLoadingtrue→falsetrue→falseerrorundefinedundefinedpreferencesundefined(never resolves)Root Cause
Both hooks populate their
datastate only via event listeners (preferences.list.pending/preferences.list.resolved, and the equivalentpreference.schedule.get.*events). TheuseEffectstarts the async fetch before it registers those listeners:The
@novu/jsevent emitter is built onmitt, which dispatches synchronously. When the in-memory cache is already warm,preferences.list()emitspendingandresolvedsynchronously, without awaiting any network request:callWithSessionqueues the call and resolves it later. That async gap lets the hook register its listeners first, so the events are captured andsync()sets the data. Works.list()runs fully synchronously insidefetchPreferences()— thepending/resolvedevents fire before the listeners are registered and are lost. Since the success branch only callsonSuccess?.(response.data!)and neversetData,preferencesstaysundefinedforever.useNotifications— which the issue reporter confirms works correctly — does not have this bug precisely because its success branch callssetData(responseData.notifications)directly from the response rather than depending on events.Fix
Align
usePreferencesanduseSchedulewith the provenuseNotificationspattern by setting state directly from the fetch response:if (response.error) { setError(response.error); onError?.(response.error); - } else { - onSuccess?.(response.data!); + } else if (response.data) { + setData(response.data); + onSuccess?.(response.data); }This also removes the unsafe non-null assertion (
response.data!) in favor of an explicitresponse.dataguard, which additionally prevents clobbering state in the (previously unhandled) edge case where a response carries neitherdatanorerror.Scope
packages/react/src/hooks/usePreferences.tspackages/react/src/hooks/useSchedule.tsNo public API/type changes, no new dependencies. A previous community PR (#10071) applied the same fix to
usePreferencesand was auto-closed for inactivity rather than rejected on merit; this PR revives that fix and additionally coversuseSchedule, which has the identical defect.Testing
mittdispatches synchronously and that a warm cache inPreferences.list()emitspending/resolvedbefore the hook's listeners are attached on re-mount.usePreferences, unmount it, then re-mount —preferencesnow resolves to the cached array instead of remainingundefined.Greptile Summary
This PR updates the React preferences hooks to set fetched data directly. The main changes are:
usePreferencesnow stores successfulpreferences.list()responses in hook state.useSchedulenow stores successfulpreferences.schedule.get()responses in hook state.onSuccesswith a non-null assertion when the response has no data.Confidence Score: 5/5
Safe to merge with minimal risk.
The changes are narrowly scoped to two matching hook success paths and follow the existing
useNotificationspattern.No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Component participant Hook as usePreferences/useSchedule participant Client as @novu/js preferences API participant Cache Component->>Hook: fetch preferences/schedule Client->>Cache: read cached data Cache-->>Client: cached response Client-->>Hook: response.data Hook->>Hook: setData(response.data) Hook-->>Component: preferences/schedule available Hook->>Client: register cache event listeners Client-->>Hook: future updated/pending/resolved events Hook->>Hook: sync event data%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Component participant Hook as usePreferences/useSchedule participant Client as @novu/js preferences API participant Cache Component->>Hook: fetch preferences/schedule Client->>Cache: read cached data Cache-->>Client: cached response Client-->>Hook: response.data Hook->>Hook: setData(response.data) Hook-->>Component: preferences/schedule available Hook->>Client: register cache event listeners Client-->>Hook: future updated/pending/resolved events Hook->>Hook: sync event dataReviews (1): Last reviewed commit: "fix(react): set preferences/schedule dat..." | Re-trigger Greptile