Undocumented behavior: syncSupabase subscriptions require soft-delete

[warrenbhw]

See here: https://github.com/LegendApp/legend-state/blob/5555c0d515c49d580cde29b8624798f6fbe1e0c5/src/sync-plugins/supabase.ts#L222-L226

Docs only indicate that use of diff-sync requires soft delete, but it looks like subscriptions also have this requirement.

That said, as far as I can tell, we should be able to do a hard-delete with subscriptions, probably just need to add an extra case here. Let me know if there are any gotchas you can think of, otherwise I can put up a PR for it.

[warrenbhw]

Added a draft PR (implementation not complete) just to get the idea across. @jmeistrich just want a quick sanity check... is there a fundamental reason why synced-subscribes require soft-deletion?

[NekodRider]

I think there is one more undocumented behavior here. https://github.com/LegendApp/legend-state/blob/7e7cfeaf29fa6604b1854d94c6b559b48d05cd85/src/sync-plugins/supabase.ts#L202-L221 If I don't use diff-sync as the docs says (or just dont have fieldCreatedAt and fieldUpdatedAt), when INSERT and UPDATE events happen it will prevent observables from being updated, which deviates from common expectations.

I agree soft-delete is only required for persist sync, since it's the only way to keep them in db before we query again. For realtime we can receive the delete event with record id and just delete it right away.

[jmeistrich]

See here:

https://github.com/LegendApp/legend-state/blob/5555c0d515c49d580cde29b8624798f6fbe1e0c5/src/sync-plugins/supabase.ts#L222-L226

Docs only indicate that use of diff-sync requires soft delete, but it looks like subscriptions also have this requirement.

That said, as far as I can tell, we should be able to do a hard-delete with subscriptions, probably just need to add an extra case here. Let me know if there are any gotchas you can think of, otherwise I can put up a PR for it.

@warrenbhw This is not actually a soft delete. symbolDelete is used internally to mark nodes as needing to be deleted (as opposed to set to undefined/null) for the merge functions to perform the delete. So I believe subscriptions should work fine with hard deletes. Or are you seeing them not working?

[warrenbhw]

@jmeistrich yeah, I was hitting this error until I added a deleted column to my table.

[jmeistrich]

I think there is one more undocumented behavior here.

https://github.com/LegendApp/legend-state/blob/7e7cfeaf29fa6604b1854d94c6b559b48d05cd85/src/sync-plugins/supabase.ts#L202-L221

If I don't use diff-sync as the docs says (or just dont have fieldCreatedAt and fieldUpdatedAt), when INSERT and UPDATE events happen it will prevent observables from being updated, which deviates from common expectations. I agree soft-delete is only required for persist sync, since it's the only way to keep them in db before we query again. For realtime we can receive the delete event with record id and just delete it right away.

@NekodRider Ah you're right, that was an oversight! But that is important to protect against receiving self-updates. It seems like the realtime subscription also includes rows that were just upserted by this client, and they seem to , and it uses the updated_at time to check that the update is newer than the value returned by upsert. Otherwise it might overwrite local state on a slow connection:

1. Set text to A
2. That triggers upsert with A
3. Set text to B
4. Upsert returns with A - the save logic merges results that weren't changed locally since the save, so it leaves text as B
5. Realtime subscription triggered with text changed to A - ignores it because it's the same timestamp as #4

So if we don't have the updated_at field, I'm not sure how to ignore realtime events from local changes. Do you know of techniques to do that in Supabase?

[jmeistrich]

@jmeistrich yeah, I was hitting this error until I added a deleted column to my table.

@warrenbhw Ah ok, that is a problem. I reopened that issue and will look into it. Can you share what your syncedSupabase options look like? It may be some combination of options I didn't test enough.

[warrenbhw]

@jmeistrich here are my current options:

configureSyncedSupabase({
  generateId: () => nanoid(),
  changesSince: "last-sync",
  fieldCreatedAt: "created_at",
  fieldUpdatedAt: "updated_at",
  // Optionally enable soft deletes
  fieldDeleted: "deleted",
});

  const workflows$ = observable(
    syncedSupabase({
      supabase,
      collection: "workflows",
      filter: (select) => select.eq("workspace_id", workspaceId),
      actions: ["read", "create", "update", "delete"],
      realtime: { schema: "public" },
      persist: { name: "workflows" }, // Persist data and pending changes locally
    }),
  );

but I hit this issue before I enabled changesSince: "last-sync"

[NekodRider]

@jmeistrich oh I see the problem. Using updated_at to compare version is likely the most practical way to solve this. Firebase has timestamp by default, but supabase has nothing helpful afaik. I think you are right we need updated_at for this. Maybe make it a required param instead of setting undefined, or perhaps give an option for "unsafe" mode?

[NekodRider]

See here: https://github.com/LegendApp/legend-state/blob/5555c0d515c49d580cde29b8624798f6fbe1e0c5/src/sync-plugins/supabase.ts#L222-L226

Docs only indicate that use of diff-sync requires soft delete, but it looks like subscriptions also have this requirement. That said, as far as I can tell, we should be able to do a hard-delete with subscriptions, probably just need to add an extra case here. Let me know if there are any gotchas you can think of, otherwise I can put up a PR for it.

@warrenbhw This is not actually a soft delete. symbolDelete is used internally to mark nodes as needing to be deleted (as opposed to set to undefined/null) for the merge functions to perform the delete. So I believe subscriptions should work fine with hard deletes. Or are you seeing them not working?

In my code, it always took a while before the symbolDelete node got deleted and I had to filter the records.

Btw, just found we need to add an existence check for delete to reduce unnecessary update. Delete events are not filterable

[warrenbhw]

@jmeistrich are soft-deleted rows ever automatically garbage collected (that is to say, hard-deleted) either in local persistence or in the remote data store?

[warrenbhw]

@jmeistrich possibly related:

wen I turn off local persistence, but turn on realtime subscription from supabase, deleting a row in supabase (from a different client) causes my local state to update so that I actually create a new row with no attributes, but don't delete the row that should be deleted.

Creates from a different client behave as expected.

Updates from a different client don't show up at all.

[warrenbhw]

I put a similar comment on this issue, since I wondered if the bugs are somehow related.

[jmeistrich]

I believe alpha.26 should fix the deleting problems. And I think it should fix https://github.com/LegendApp/legend-state/issues/327 too. Does it work better for you after the update?

[warrenbhw]

Let me give it a try right now!

[warrenbhw]

Realtime deletes and updates are working for me now 🙌

thanks a bunch @jmeistrich!