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

RTK Query gets stuck in "pending" state when following the persistence example from the website #4715

Open
GertSallaerts opened this issue Nov 15, 2024 · 2 comments

Comments

@GertSallaerts
Copy link

GertSallaerts commented Nov 15, 2024

Problem description

When you set up your store to persist your api reducer as outlined on the website, there's a chance your persisted queries get stuck in "pending" state if the app gets interrupted while fetching a query (e.g. the window is reloaded.) This only happens when a few conditions are met:

  1. You've set up redux-persist to persist the api reducer, not the root reducer.
  2. None of the queries that get rehydrated are fulfilled or rejected, e.g. they were all pending when the app got interrupted
  3. You're using autoMergeLevel1 as state reconciler for redux persist
  4. You're using extractRehydrationInfo in your api slice as documented on the website

When both those condition are met, the following will happen upon rehydration:

  1. The app loads, state is initialized, etc.
  2. The current state of your api slice looks a bit like this: { queries: {}, mutations: {}, config: {...} }
  3. redux-persist's REHYDRATE action is triggered
  4. Our extractRehydrationInfo runs and returns the REHYDRATE payload to RTKQ
  5. RTKQ takes that payload and sets all queries that are either fulfilled or rejected on the queries object. In our case that's none of them (condition 2 above.) This happens here:
    const { queries } = extractRehydrationInfo(action)!
    for (const [key, entry] of Object.entries(queries)) {
    if (
    // do not rehydrate entries that were currently in flight.
    entry?.status === QueryStatus.fulfilled ||
    entry?.status === QueryStatus.rejected
    ) {
    draft[key] = entry
    }
    }
  6. At the end of REHYDRATE, redux-persist will use autoMergeLevel1 to do a shallow compare between the state before and after REHYDRATE. It will not detect any changes to the queries subkey of the state at this point and resort to retaining what it loaded from storage (i.e. the pending queries.) This happens here: https://github.com/rt2zz/redux-persist/blob/d8b01a085e3679db43503a3858e8d4759d6f22fa/src/stateReconciler/autoMergeLevel1.ts#L24
  7. RTKQ will not load the data for those queries anymore because (since they're in pending state) it's waiting for them to complete which they will never do.

Example

I've set up a minimal example that shows the issue here: https://codesandbox.io/p/sandbox/qpv4m9

Inside the getPokemonByName endpoint, it does a window.reload() to simulate the user reloading the page. So if the code worked, the example would be in an infinite reload loop. Instead it will rehydrate the pending query from localStorage after the first reload and get stuck there.

Towards a solution

I don't think there's a bug in redux-persists, nor in RTK Query. Both work as advertised by themselves.

The problem is that the example code on the RTK website on how to make them work together doesn't work in all scenarios. I can think of two ways to resolve this:

  • Using a custom state reconciler, I think it could be as simple as writing one that always uses the so-called reducedState (i.e. just use whatever is returned by the api reducer during REHYDRATE.)
  • Writing a transform for redux-persist that doesn't persist these pending queries in the first place

I'm happy to help out on either one but I'd be interested in hearing the team's thoughts on this first.

  • I'm assuming you would like to prevent having any kind of redux-persist-specific code in the library?
  • Do you see this as something that gets written out fully in the documentation page on the website for people to copy-paste?
  • Would you be willing to either maintain or link to a separate transform package for redux-perist? (e.g. redux-persist-transform-rtk-query like the others they list on their README by independent developers.)
@markerikson
Copy link
Collaborator

Yeah, persistence is outside the scope of RTK, so better documentation is the best answer here.

@GertSallaerts
Copy link
Author

Do you have a preference on which of the solution you'd like to see in the documentation?

I'll add a bit of example code and pros and cons to get started. It will be a longer post again, sorry in advance 😅.

Using transforms

  • Pro: You're not perisisting data you won't use anyway
  • Con: It's a bit more complicated to (a) understand and (b) document in its entirety
  • Con: The implementation is significantly different when persisting the api reducer then when persisting the root reducer, increasing the length and complexity of the docs
  • Con: Consumers of RTK-query are adding code to their application that (to me at least) feels a bit like playing with RTKQ's internals (i.e. knowledge about the structure of its state object)

The first two cons could be countered by either:

  • Adding a dedicated page for this to the docs, or
  • Releasing a separate redux-persist-transform-rtk-query package and documenting its use, abstracting away most of the complexity from the documentation

The last con could be countered by a separate package too, provided the package has a trusted and active maintainer (which is why I suggested releasing that under the reduxjs namespace, but I'm sure Ambassify would be willing to releasing it too if desired.)

Note: While it won't hurt to add it, it is no longer required to use extractRehydrationInfo.

Some code illustrating what we did at Ambassify (for persisting the api reducer.)

import { pickBy } from 'lodash';
import { createTransform } from 'redux-persist';
import storage from 'redux-persist/lib/storage'
import { QueryStatus } from '@reduxjs/toolkit/query';

/**
 * We only want to persist fulfilled queries. Caching anything else doesn't
 * make much sense and can even cause bugs: https://github.com/reduxjs/redux-toolkit/issues/4715
 */
const whitelistFulfilledQueries = createTransform(
    (inboundState: any) => pickBy(inboundState, { status: QueryStatus.fulfilled }),
    (outboundState: any) => pickBy(outboundState, { status: QueryStatus.fulfilled }),
    { whitelist: [ 'queries' ] }
);

/**
 * We don't want to persist mutations, subscriptions, config, ... The cache is
 * only used when the application loads so none of those are useful at that point.
 */
const removeNonQueries = createTransform(
    () => ({}),
    () => ({}),
    { blacklist: [ 'queries' ] }
);

const persistConfig = {
    key: 'root',
    storage,
    transforms: [
        whitelistFulfilledQueries,
        removeNonQueries,
    ]
}

Using a custom state reconciler

  • Pro: The code for this is a lot less complicated
  • Pro: extractRehydrationInfo still hides the RTKQ internals
  • Pro and con: It's only required for users that want to persist the api reducer, so while the docs do need to explain that complication, it is still only more work for some users
  • Con: It persists data the user will never need or use (since entire state is persisted)
  • Con: I'm not 100% sure it works properly, as we've only briefly tested this before choosing the transforms solution internally
import storage from 'redux-persist/lib/storage'

function rtkqStateReconciler(inboundState, originalState, reducedState) {
    return reducedState;
}

const persistConfig = {
    key: 'root',
    storage,
    stateReconciler: rtkqStateReconciler,
}

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

No branches or pull requests

2 participants