keystonejs / keystone

The superpowered headless CMS for Node.js — built with GraphQL and React
https://keystonejs.com
MIT License
9.3k stars 1.16k forks source link

Transaction should prevent after operation hooks until committed #9309

Open gautamsi opened 3 months ago

gautamsi commented 3 months ago

I should be seeing no afterOperation hook until the transaction is committed.

I believe this is debatable if the side effects should be allowed or not while transaction may be rolled back

Do we have a way to know if the afterOperation is running in transaction or not? at least I can try to defer the side effects somehow.

dcousens commented 3 months ago

@gautamsi this is intended, and arguably the only reasonable option when you have nested operations.

The underlying problem is that we have emphasised that beforeOperation and afterOperation should be used for out-of-band side effects; while simultaneously encouraging their usage for scenarios that should be wrapped in a transaction.

We cannot reasonably put beforeOperation hooks outside of a transaction; as the nested operations within a transaction cannot be easily known up front. And while we could queue up afterOperation hooks to after the transaction, this will break for users who expect their afterOperation hooks fire directly _after the intended operation, within the transaction.

When sparring about this with @molomby, we thought maybe a new hook that is equivalent to an afterCommit and or afterRollback could be appropriate; but fell short of adding this in the prototype as a result of not wanting to complicate that experiment before collecting feedback (like this).

hooks: {
  transaction: {
    commit: () => {},
    rollback: () => {}
  }
}

If the relevant item operation has not been executed within a transaction context, we could optionally decide if commit should always be called as an equivalent to afterOperation.