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

skipToken passed to serializeQueryArgs #3151

Closed
SebastienGllmt opened this issue Feb 2, 2023 · 13 comments · Fixed by #4762
Closed

skipToken passed to serializeQueryArgs #3151

SebastienGllmt opened this issue Feb 2, 2023 · 13 comments · Fixed by #4762
Labels
bug Something isn't working rtk-query

Comments

@SebastienGllmt
Copy link
Contributor

It looks like when you specify and endpoint-specific serializeQueryArgs, it's possible for queryArgs to equal skipToken

This is unexpected since it's not reflected in the typescript type. Either the typescript type should be updated, or skipToken should be filtered out

@markerikson
Copy link
Collaborator

Can you give an example of this happening?

@szszoke
Copy link

szszoke commented Nov 18, 2023

I am also affected by this. In my case this is triggered by just calling the auto-generated lazy query hook in a component.

const MyComponent = () => {
  useLazyGetQuery();
  
  return null;
}

@markerikson markerikson added bug Something isn't working rtk-query labels Feb 6, 2024 — with Volta.net
@ninesunsabiu
Copy link

I am also affected by this

@markerikson
Copy link
Collaborator

Out in https://github.com/reduxjs/redux-toolkit/releases/tag/v2.4.0 !

@jshih7
Copy link

jshih7 commented Dec 9, 2024

Hi there. I just upgraded @reduxjs-toolkit to 2.4.0 but am still seeing that serializeQueryArgs gets called with skipToken being passed as the queryArgs. I would expect serializeQueryArgs to be skipped altogether. Here's some example code:

Calling query in React component:

import { skipToken } from '@reduxjs/toolkit/query/react';

// inside component
const [userId, setUserId] = useState('');
const { data, error, isFetching, isLoading } = useGetUserQuery(userId || skipToken );

API slice:

const apiSlice = createApi({ 
  ...
  endpoints: (build) => ({
    getUser: build.query<...>({
      queryFn: (userId) => { ... }
      serializeQueryArgs: ({ queryArgs: userId }) => {
        console.log({userId}); // this returns { userId: Symbol(RTKQ/skipToken) }
        return userId;
      };
    }),
 })
});

I realize that in this example, serializeQueryArgs is not necessary because it would be caching on userId by default anyway, but I just wanted to demonstrate that it's still getting called when I want to skip the query.

@markerikson
Copy link
Collaborator

@jshih7 hmm. out of curiosity, if you change that to a console.trace(), what's the stack trace that's calling serializeQueryArgs?

@jshih7
Copy link

jshih7 commented Dec 9, 2024

@jshih7 hmm. out of curiosity, if you change that to a console.trace(), what's the stack trace that's calling serializeQueryArgs?

Sorry, I'm debugging this within an application for work so it'll be difficult for me to share exact details. However, the stack trace looks something like this:

serializeQueryArgs @ apiSlice.ts
ChildComponent @ ChildComponent.tsx (this is the useGetUserQuery call)
...
dispatch @ page.bundle.js
(anonymous) @ ParentComponent.tsx (this is a dispatch)

In the ParentComponent, I'm calling a dispatch to the Redux store to modify state, which re-renders the ChildComponent and triggers the query fetch in the child, which then calls serializeQueryArgs.

@markerikson
Copy link
Collaborator

Ah. I bet it's because we also call it at the hook level, in a useStableQueryArgs hook, and we didn't adjust that to check for skipToken:

? serialize({ queryArgs, endpointDefinition, endpointName })

that needs to be addressed as well.

@markerikson markerikson reopened this Dec 9, 2024
@jshih7
Copy link

jshih7 commented Dec 9, 2024

Thanks - if you're able to address and create a new build (could be a beta build), I can install and confirm that it's working on my end.

@jshih7
Copy link

jshih7 commented Dec 10, 2024

Ah. I bet it's because we also call it at the hook level, in a useStableQueryArgs hook, and we didn't adjust that to check for skipToken:

redux-toolkit/packages/toolkit/src/query/react/useSerializedStableValue.ts

Line 16 in e848a55

? serialize({ queryArgs, endpointDefinition, endpointName })
that needs to be addressed as well.

I took a look at the code you sent, it looks like useStableQueryArgs is called here with the possibility of args being a skipToken. Since useStableQueryArgs checks if typeof queryArgs == 'object' before calling serialize in the code snippet you sent me, I don't think that's where the issue lies.

I went back to the console.trace you asked me to add and realized that some information was hidden the first time around that I overlooked - sorry about that!

Here is the more complete stack trace, with code snippets at the line number:

serializeQueryArgs @ apiSlice.ts (console.trace)

finalSerializeQueryArgs @ createApi.ts:244 (`const initialResult = endpointSQA(queryArgsApi);`)

serializeQueryArgs @ createApi.ts:260 (`return finalSerializeQueryArgs(queryArgsApi);`)

queryStatePreSelector @ buildHooks.ts:627 (`if (serializeQueryArgs(...) === serializeQueryArgs(...)`)

recomputationWrapper @ createSelectorCreator.ts:392 (`return (resultFunc as Combiner<inputSelectors, Result>).apply(...)`)

memoized @ weakMapMomoize.ts:228 (`result = func.apply(...)`)

dependenciesChecker @ createSelectorCreator.ts:412 (`lastResult = memoizedResultFunc.apply(...)`)

memoized @ weakMapMemoize.ts:228 (`result = func.apply(...)`)

(anonymous) @ buildHooks.ts:864 (`const currentState = useSelector(...)`)

useSelectorWithStoreAndSubscription @  useSelector.js:29 (`var newSelectedState = selector(storeState)`)

useSelector @  useSelector.js:129 

useQueryState @  buildHooks.ts:864

useQuery @  buildHooks.ts:894

Based on this, I think the issue is that we shouldn't need to perform the serializeQueryArgs equality check here.

@jshih7
Copy link

jshih7 commented Dec 11, 2024

Hey @markerikson , I created a code sandbox to reproduce this issue: https://codesandbox.io/p/sandbox/zen-brook-hhmw4c

If you type in "charizard" in the input field, you'll see results from calling useGetPokemonByNameQuery. Then if you clear the field, you'll see that in the console, serializeQueryArgs will still gets called.

There is actually another unexpected behavior that I encountered during my testing (I can open a new issue for this as well). When clearing the field, I expect that the query would return undefined for data because it should get skipped, but it returns the data from the last query call.

In summary, here's the current behavior:

  • After I enter "charizard", the following gets logged to the console:
    • inside serializeQueryArgs, queryArgs: charizard
    • after useGetPokemonByNameQuery, pokemonName: charizard, data: {"abilities":[{"ability" {"name":"blaze","url":"https://pokeapi.co/api/v2/ability/66/"},"is_hidden":...
  • After I clear the input field, the following gets logged the console:
    • inside serializeQueryArgs, queryArgs: Symbol(RTKQ/skipToken)
      • Expected behavior: serializeQueryArgs should not get called
    • after useGetPokemonByNameQuery, pokemonName: , data: {"abilities":[{"ability":{"name":"blaze","url":"https://pokeapi.co/api/v2/ability/66/"},"is_hidden":...
      • Expected behavior: data should be undefined, not the data for charizard

@markerikson
Copy link
Collaborator

@jshih7 : fwiw off the top of my head I think that the data behavior is correct. That should always point to the last successful result. If you must have the exact result that matches the current arg, use currentData instead:

@jshih7
Copy link

jshih7 commented Dec 11, 2024

@markerikson Thanks, I jumped the gun, haha. Shortly after I posted that, I found a couple discussions where you and others pointed out the difference between data and currentData:

For my use case, currentData is what I need, since I want the query result to update according to my args. Appreciate the references!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rtk-query
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants