Skip to content
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

📝 Expand on implementing manual cache updates #1525

Merged

Conversation

Shrugsy
Copy link
Collaborator

@Shrugsy Shrugsy commented Sep 19, 2021

  • Add Manual Cache Updates page
  • Replace Optimistic Updates sidebar link with
    Manual Cache Updates link, and move closer to top
  • Move Optimistic Updates content to a recipe within
    Manual Cache Updates
  • Delete old unused cache-management page
    (replaced by cache-management-utils, but old one
    was accidentally left behind)

The purpose of this PR is to more explicitly advertise how to perform 'manual cache updates' in a number of ways, rather than specifically 'optimistic updates' only. This should hopefully make it easier both for:

  • people who want to perform optimistic updates, but aren't looking for those particular keywords
  • people who want to perform other manual updates (e.g. pessimistic updates), but don't have explicit guidance for how to translate an optimistic update to a pessimistic update

- Add `Manual Cache Updates` page
- Replace `Optimistic Updates` sidebar link with
  `Manual Cache Updates` link, and move closer to top
- Move `Optimistic Updates` content to a recipe within
  `Manual Cache Updates`
- Delete old unused `cache-management` page
  (replaced by `cache-management-utils`, but old one
  was accidentally left behind)
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 19, 2021

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 2670c15:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration

@netlify
Copy link

netlify bot commented Sep 19, 2021

✔️ Deploy Preview for redux-starter-kit-docs ready!

🔨 Explore the source changes: 2670c15

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/614a0b22e5fd020008bdab7b

😎 Browse the preview: https://deploy-preview-1525--redux-starter-kit-docs.netlify.app

Copy link
Member

@msutkowski msutkowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reads very well 🎉. Only one small suggestion, but can be discarded.

@markerikson
Copy link
Collaborator

If we're removing old pages, can you add an entry into the website/_redirects file so that any external links will end up in the right spot?

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented Sep 20, 2021

If we're removing old pages, can you add an entry into the website/_redirects file so that any external links will end up in the right spot?

@markerikson no problem, all done for the optimistic-updates relocation.

FYI for clarity: I realised I said this in my commit message:
Delete old unused cache-management page
But I really just deleted the file. The page itself was still turned into cache-management-utils before the official v1.6 release

@msutkowski
Copy link
Member

@markerikson @phryneas I gave this another read and I'm good with it being merged 👍

@markerikson markerikson merged commit 97319a3 into reduxjs:master Oct 1, 2021
@thedriveman
Copy link

I have some feedback about the pessimistic update example (https://redux-toolkit.js.org/rtk-query/usage/manual-cache-updates#pessimistic-updates).

My feedback is based on two assumptions:

  1. A pessimistic update may be used to update the cache with data returned from the server following an update action, and then update subscribers to a specific query without triggering an unnecessary fetch to the server (i.e., automated re-fetching).
  2. That an effective example would demonstrate how manual update skips an automated re-fetching.

However, it seems that the example does not demonstrate this particular use case, due to the way invalidatesTag is setup. When invalidatesTags: ['Post'] (or even invalidatesTags: [{type: 'Post', id}]) is specified, getPost will still automatically re-fetch data from the server, even if the cache is updated via updateQueryData.

It looks like the way to skip the server fetch for that use case would be to remove the invalidatesTag, as the updateQueryData implicitly invalidates the cache for that specific post.

Thoughts?

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented Oct 4, 2021

Thanks @thedriveman, you're correct, neither the optimistic nor pessimistic examples should have invalidatesTags included, that's just an oversight leaving them in. Feel free to open a PR that removes them, otherwise I'll open one eventually.

If anything, the pessimistic update example could invalidate the Post tag in the catch block, but it's up to the user how they want to handle failed updates exactly

@PEsteves8
Copy link

Couldn't leaving out the tag invalidation raise problems in some edge cases like mentioned here #1529 ?

In the case of the example, if the GET request for the post failed or returns successfully but with a falsy body (eg null and false are valid json), then manual cache updates won't work, since it requires data to exist in the cache for the manual update to occur. I know, these are edge cases, but I could see some people bumping into these sorts of issues (like me :D)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants