-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
split up middleware, add OptionalPromise, add cache entry lifecycle #1034
split up middleware, add OptionalPromise, add cache entry lifecycle #1034
Conversation
Deploy preview for redux-starter-kit-docs ready! Built with commit 2b7c65b https://deploy-preview-1034--redux-starter-kit-docs.netlify.app |
b64b365
to
8bcd7f5
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2b7c65b:
|
what's still left todo here code-wise is removing the existing endpoint lifecycle onStart/etc. |
8bcd7f5
to
4e062a8
Compare
…eature/lifecycles
@@ -40,11 +40,14 @@ export const build: SubMiddlewareBuilder = ({ | |||
originalArgs | |||
) | |||
|
|||
const extra = mwApi.dispatch((_, __, extra) => extra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Genius, isn't it? :D
I was like "the old API had it, so we mayybe need it here too?" and then "how on earth do I get that?". Yup.
Btw, Bikeshedding: we need a discussion if |
And internal TODO: I'm thinking of adding a Also, maybe adding a |
queryThunk, | ||
mutationThunk, | ||
}) => { | ||
type CacheLifecycle = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be moved outside, or is there a reason it's declared here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only used in a locally scoped variable in the next line and once more to declare a value to add to that variable.
Moving it outside would raise questions like "should this be exported" etc, just keeping it here just circumvents all that and will also 100% remove it on transpilation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Tx for the explanation.
@@ -277,6 +277,26 @@ export function buildCreateApi<Modules extends [Module<any>, ...Module<any>[]]>( | |||
} | |||
x.providesTags ??= x.provides | |||
} | |||
if (x.onStart || x.onSuccess || x.onError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, glad u added a message here!
add `updateCacheEntry` to `QueryLifecycleApi`, add `.undo` to `updateCacheValue` return type re-scope middleware variables tests
}), | ||
onStart({ id, ...patch }, { dispatch, context }) { | ||
context.undoPost = dispatch( | ||
async onQuery({ id, ...patch }, { dispatch }, { resultPromise }) { | ||
const { undo } = dispatch( | ||
api.util.updateQueryResult('post', id, (draft) => { | ||
Object.assign(draft, patch) | ||
}) | ||
).inversePatches | ||
}, | ||
onError({ id }, { dispatch, context }) { | ||
dispatch(api.util.patchQueryResult('post', id, context.undoPost)) | ||
) | ||
resultPromise.catch(undo) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's way too late, but this diff is kinda worth it
Since Size Change: +24.6 kB (+5%) 🔍 Total Size: 533 kB
|
…eature/lifecycles
Background on the "cache entry lifecycle": rtk-incubator/rtk-query#215