feat: make applyDocumentActions more similar to the rest #630
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Sep 22, 2025
judofyr
force-pushed
the
holm/actions-unify
branch
from
9ebc4e0 to
8d9b001
Compare
ryanbonial
reviewed
Sep 23, 2025
| {state}: StoreContext<DocumentStoreState>, | ||
| {actions, transactionId = crypto.randomUUID(), disableBatching}: ApplyDocumentActionsOptions, | ||
| ): Promise<ActionsResult> { | ||
| const actions = Array.isArray(actionOrActions) ? actionOrActions : [actionOrActions] |
There was a problem hiding this comment.
It seems like moving these checks to the React package, useApplyDocumentActions, would allow developers using the core package directly to pass invalid actions
There was a problem hiding this comment.
We don’t actually validate the actions in core, though? If you’re using core methods you must pass data which matches the TypeScript definition or bad things happen.
I’ve just dropped support for having either an array or an object in core, but I don’t think there’s much difference from that.
This was referenced Sep 24, 2025
`applyDocumentActions` was the only function in the `core` package which interpreted the `SanityInstance` as a tree, automatically traversing the tree to find a matching instance. This is inconsistent with the other stores (e.g. query store) which assumes that it's being called with the right instance. In addition, this changes `applyDocumentActions` to only accept actions as an array (so we don't have to check if it's an array or not) and also it places the `actions` as part of option. This is consistent with e.g. `getQueryState` which has a required `query` field in its options. This now means that all functions which are wrapped in `bindActionByDataset` are now consistently getting passed an object as their first argument. This is going to be key so that we later can modify this function to consider the options (instead of the instance). There should be no breaking changes in the React package.
judofyr
force-pushed
the
holm/actions-unify
branch
from
8d9b001 to
606c73f
Compare
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.
Description
applyDocumentActionswas the only function in thecorepackage which interpreted theSanityInstanceas a tree, automatically traversing the tree to find a matching instance. This is inconsistent with the other stores (e.g. query store) which assumes that it's being called with the right instance.In addition, this changes
applyDocumentActionsto only accept actions as an array (so we don't have to check if it's an array or not) and also it places theactionsas part of option. This is consistent with e.g.getQueryStatewhich has a requiredqueryfield in its options. This now means that all functions which are wrapped inbindActionByDatasetare now consistently getting passed an object as their first argument. This is going to be key so that we later can modify this function to consider the options (instead of the instance).There should be no breaking changes in the React package.
What to review
Testing
Fun gif