Repository navigation
fix(supabase): don't treat an update that matched 0 rows as success - #674
Open
LinusOssmann wants to merge 1 commit into
Open
LinusOssmann wants to merge 1 commit into
LinusOssmann wants to merge 1 commit into
Conversation
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
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.
Summary
When a row is created and edited before the create request has returned, syncedCrud sends the update right away, in parallel with the create. That's intentional (see the test "update before create returns does update"). With
syncedSupabasethe defaultupdatedoes.update(input).eq(id).select(). If the PATCH reaches PostgREST before the INSERT, it matches 0 rows and PostgREST answers200 []. The plugin returnsdata[0](undefined) without an error, crud treats that as success and clears the pending change. The INSERT then lands with the create-time snapshot, and the edit is gone: no error, no retry,getPendingChanges()is empty.We saw this in our app as "fields filled in right after creating a record are sometimes empty on the server" and confirmed it with a HAR capture: PATCH with the new values →
200 [], then INSERT with the old values.Repro
Against current
main, with a minimal PostgREST mock where the INSERT is slower than the PATCH:The same happens whenever the row doesn't exist (yet) on the server: a row hidden by RLS, or a row deleted remotely. In all of these cases the local change is silently dropped as "synced".
Proposed fix
Treat an UPDATE that matched no rows as an error in the default
updateofsyncedSupabase. Withretryconfigured, the update is retried and succeeds as soon as the INSERT has landed. Without retry,onErroris called (withrevert), so the app at least finds out. PR follows.Open question: a row that is permanently missing (deleted remotely, or hidden by RLS) would now keep retrying when
retry.infiniteis set. Alternatives would be an option (e.g.updateRequiresRow: true, or upsert on update), or having crud wait for an in-flight create of the same id before sending the update. I tried the latter incrud.ts(if (pendingCreate?.promise) await pendingCreate.promise). It fixes this too, but it breaks the existing tests "update before create returns does update" and "stale queued update retry does not run after create failure", so it looks like a design decision on your side. Happy to go either way.