FE-600: serialize authenticated user updates - #9242
Conversation
Updates to the authenticated user all patch the same entity, and the graph rejects a patch which overlaps another. Preference changes were sent and forgotten, so two which overlapped left the UI showing a change the server had refused. Run these updates one at a time through a module-scoped serial queue, and build each one when it is sent rather than when it was clicked. Because callers construct their payload by spreading the preferences they last rendered, only the setting the caller actually changed is re-applied on top of the preferences on the server now.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
PR SummaryMedium Risk Overview
For Adds Reviewed by Cursor Bugbot for commit e38e8d6. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 52116dc. Configure here.
…yload from Two cases where the base the rebase compared a payload with was not the preferences the caller had built it from, so its diff was wrong: - A caller with no stored preferences renders the defaults which `useUserPreferences` substitutes, but the base was read straight from `authenticatedUser.preferences` and so was `undefined`. Every default the caller sent then looked like a change of its own, overwriting another first-time change which had landed in the meantime. The base is now read from `useUserPreferences`, and `rebaseUserPreferences` requires one. - A caller which changes the same setting twice before the first update lands builds its second payload from what it rendered, which does not include its own first change yet, so the second payload matched the base exactly and the rebase treated the revert as no change at all. Each hook instance now rebases from the payload it still has in flight, while it has one.

Requested by Ciaran Morinan · Slack thread
🌟 What is the purpose of this PR?
Before. Preference changes were sent and forgotten. Toggle a display setting in settings and expand a sidebar section a moment later, and the two saves overlapped on the same user record; the graph refused the second. The refusal came back inside a successful-looking response that nothing was reading, so the UI kept showing what you had just clicked while the server kept the old value, and your change was gone on the next page load. Even when both saves got through, each was assembled from a copy of your preferences taken before the other landed, so the later save wrote the earlier one back. In Playwright the failure was consistent rather than occasional, because tests click faster than a person.
After. Preference saves go one at a time, so two never collide and the graph never refuses one. Each save is assembled when it is sent rather than when it was clicked, and only the setting the caller actually changed is applied on top of what is already stored — so an expanded sidebar section and a switched display variant both stick, in either order. Favourites behave the same way. No call site changed.
🔗 Related links
🚫 Blocked by
🔍 What does this change?
How.
useUpdateAuthenticatedUsernow runs the body of each update through a module-scoped serial queue (serial-queue.ts), so an update starts only once every update enqueued before it has settled — the queue is module-scoped rather than per hook instance because each component that saves preferences has its own instance of the hook and they do not coordinate. Everything the update needs (the authenticated user,getMe,updateEntity,refetch) is read from a ref when its turn comes rather than captured when it was enqueued, so it sees the state the updates ahead of it left behind; the callback consequently has an empty dependency array and is stable. Before sending,rebaseUserPreferences(rebase-preferences.ts) diffs the preferences the caller built its payload from against the payload, and re-applies just that difference on top of the preferences read back from the server in the same turn — recursing through nested objects, and treating the favourites array as a set, since its order is display-only. The queue swallows rejections on the chain it keeps internally, so one failed update cannot stall the ones behind it, while still rejecting for the caller that submitted it.e38e8d6, after a reviewer was already requested: two cases were fixed and covered by tests, both in what the rebase treats as its base. The base wasauthenticatedUser?.preferences, which isundefinedfor a user with nothing stored, while every call site builds its payload fromuseUserPreferences, which substitutes the defaults — so with a concurrent change already on the server, every default was read as a caller edit and overwrote it, and two quick first-time saves still clobbered each other. Separately, a second toggle of the same field before the context refreshed produced a payload deep-equal to the base, so the rebase found no change to re-apply, returned the server's value and dropped the revert — a regression againstmain, where that payload was sent unrebased. The base now comes frompreferencesToRebaseFrom, which reads the sameuseUserPreferencesvalue the call sites build from, and prefers the payload the same hook instance still has in flight.getMe,updateEntity,refetch) — none were added — and all six preference call sites discard the promise and never read the hook'sloading, so they never awaited the round trip in the first place. The only call site that awaits a save behind a disabled button is the profile modal, which can now wait out a preference save that is already in flight if you toggled a sidebar section a moment before pressing Save; that is bounded by one update, and it is exactly the overlap that previously failed.maineither, because of an unbuilt Rust→wasm artifact and ungenerated GraphQL types). The hook's own wiring has no test — the queue and the rebase are tested as units, and the original race is not reproduced anywhere.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
rebaseUserPreferencestreats the favourites array as a set. That is right for favourites, and would be wrong for any future preference whose array order carries meaning.🐾 Next steps
loadingflag ever needs to reflect queue position rather than just this instance's own in-flight updates, that would be a separate change.🛡 What tests cover this?
New unit tests, 14 in total:
serial-queue.test.ts(4) — a task does not start while an earlier one is running; a task runs when its turn comes rather than when it was enqueued, so it reads what the one before it left; a failing task does not stop the tasks behind it; tasks resolve in the order they were enqueued.rebase-preferences.test.ts(10) — eight forrebaseUserPreferences: a change that landed while the caller's update was queued is kept; the caller's change wins when the server changed the same field; the server's value is taken for anything the caller did not change; a favourite is added, and separately removed, without dropping one added in the meantime; the caller's preferences are sent as-is when the server has none; fields the caller never saw are kept; and another caller's first-ever change survives when neither caller had stored preferences, rather than the defaults substituted for the caller overwriting it. Two forpreferencesToRebaseFrom: what the caller rendered is used when it has nothing in flight; and a second change to the same field, made before the first landed, is kept rather than lost to the server's value.❓ How to test this?
main, one of the two is lost.