[^3.0.0] data is undefined while loading

[mrtnbroder]

Intended outcome:

Let's take the example from your website:

const GET_DOG_PHOTO = gql`
  query Dog($breed: String!) {
    dog(breed: $breed) {
      id
      displayImage
    }
  }
`;

function DogPhoto({ breed }) {
  const { loading, error, data } = useQuery(GET_DOG_PHOTO, {
    variables: { breed },
  });

  if (loading) return null;
  if (error) return `Error! ${error}`;

  return (
    <img src={data.dog.displayImage} style={{ height: 100, width: 100 }} />
  );
}

when the breed variable changes, it should set loading = true and return the past data until it resolves.

Actual outcome:

when the breed variable changes, it sets loading = true and returns undefined for data until it resolves.

How to reproduce the issue:

See the example above. This used to work in Apollo 2.x. I think it has to do with the cache, but there is nothing mentioned on the docs. This is indeed an app-breaking bug.

Versions

[lukewlms]

Was just about to log something very similar. Our case only happens when rapidly refetching though. I can log a separate issue if needed. These are breaking changes for us - we can't deploy if fetching new data wipes the old data while new data is fetching (contrary to 2.x behavior). Can't find documentation for these apparently breaking changes.

Repro steps:

• Get the refetch function from useQuery result
• Rapidly call refetch (we refetch when the user changes the date in our app to view a different day, and they may tap "previous date" several times rapidly)

Expected Result (2.x):

1. While refetching, network status is set to 2 - "refetch"
2. Refetches can be fired unlimited times

Current Result (3.x):

1. While refetching, network status is set to 7 - "ready" (!). If firing a larger number of refetches, this actually is set to 2 briefly, but not for a single refetch like before.
2. After firing many quick refreshes, data resets to undefined until the query finishes. This will happen multiple times in the same session if more refetches are fired for more as-yet-unknown data (in our case, past dates)

Thanks!

[benjamn]

@mrtnbroder This was an intentional change introduced in AC3 by #6566 to fix #6039, and listed as a [BREAKING] change in CHANGELOG.md:

• [BREAKING] Results with loading: true will no longer redeliver previous data, though they may provide partial data from the cache, when available.
  @benjamn in #6566

It's fair to say this hasn't been a popular change, but the only way I can see to restore reasonable behavior (so that #6039 remains fixed) would be to provide result.previousData separately from result.data, as I proposed in #6603 (comment).

The old behavior was ambiguous, because result.data could either be a cache result for the new variables, or a stale result for some older/different variables. If we're going to be providing stale data when variables change, it needs to come clearly labeled (e.g. result.previousData).

[mrtnbroder]

@benjamn thanks for clarification. I was browsing a bit more around here and eventually found out that this was an intentional change.

Any idea on when this result.previousData will land?

[balazsorban44]

Came here for a solution, I can see there is only a proposal for this right now. Here is my workaround:

//...
  const [getData, query] = useLazyQuery() // Uses a GraphQL query that has an $id argument/variable
  const [data, setData] = React.useState()
  React.useEffect(() => {
    if (query.data) {
      setData(query.data)
    }
  }, [data])

  const handleChange = (id) => {
    getData({
      variables: {id}
    })
  }
//...

UPDATE:
I have read the ongoing discussion you linked to, so this one here is an unnecessary duplicate. Please ignore.
About the result.previousData, @benjamn wouldn't it be easier to support an optional flag, like so?:

useLazyQuery(query, {
  staleWhileRevalidate: true,
  // or
  swr: true
})

This would not require using result.data ?? result.previousData everywhere further down.

[mrtnbroder]

For anyone using graphql-codegen, the way I worked around this to create your own useQuery and use that version:

src/utils/apollo-react-hooks.ts

import { useRef } from "react"
import type { DocumentNode, QueryHookOptions } from "@apollo/client"
import { useQuery as useQueryT } from "@apollo/client"

export * from "@apollo/client/react"

// code from: // https://github.com/apollographql/apollo-client/issues/6603
// OVERWRITE DEFAULT useQuery from @apollo/client/react on version 3.x until
// https://github.com/apollographql/apollo-client/issues/7038 will be resolved!
export function useQuery<TData, TVariables>(
  query: DocumentNode,
  options?: QueryHookOptions<TData, TVariables>,
) {
  let cachedData = useRef<TData | undefined>(undefined)

  const queryResult = useQueryT<TData, TVariables>(query, options)

  if (
    queryResult.loading !== true &&
    queryResult.data !== undefined &&
    // Check for empty object due to https://github.com/apollographql/apollo-client/issues/6876
    Object.keys(queryResult.data).length > 0
  ) {
    cachedData.current = queryResult.data
  }

  return { ...queryResult, data: cachedData.current }
}

and in the codegen.yml adjust the path for apolloReactHooksImportFrom: "src/utils/apollo-react-hooks"

I also agree with @balazsorban44, a flag would be easier and stop us from writing a lot of duplicated code:

const data = result.data ?? result.previousData

[lukewlms]

Here's our current solution, seems to be working well.

The point is just to ensure that once data is defined, it's never reset to undefined.

export function useQueryWithTypes<TData, TVariables>(
  query: TypedDocumentNode<TData, TVariables>,
  options?: QueryHookOptions<TData, TVariables>,
) {
  const result = useQuery(query, options);

  const [data, setData] = useState<TData | undefined>(undefined);
  useEffect(() => {
    if (result.data !== undefined) {
      setData(result.data);
    }
  }, [result.data]);

  return { ...result, data };
}

[balazsorban44]

I suggest further discussions on this are made in #6603 as it seems to be a duplicate. This way the maintainers can concentrate on it in a single thread/issue. There are over 500 open issues on this repo, I guess it is hard enough to maintain that. We could potentially make this issue harder to resolve by arguing about the same thing in different places.

[mrtnbroder]

Yes. Closing this one as a dupe of #6603

[benjamn]

As I commented over in #6603, we finally have an implementation of result.previousData (thanks to @hwillson in #7082), and you can test it now using @apollo/client@3.3.0-beta.6.

We recommend the idiom result.data ?? result.previousData to obtain the most recent useful data (if you're comfortable with it possibly being stale). While this may seem like more code than just result.data, the ?? expression represents a choice that should be made consciously (specifically, that you're okay with stale data), and explicitly enabled with a minimum of extra syntax. We hope result.previousData gives you the power to make that choice, while ensuring result.data is never ambiguously stale.

[SachaG]

I'm a bit late to the discussion but I just wanted to mention this change broke the UI for multiple of my apps and I didn't notice the issue for a while… If it was changed on purpose, it should certainly have been highlighted in huge red type in the Apollo Client release notes or migration guide!

[ryparker]

Not sure if this was fixed but as of v3.6.9 when loading, react query hooks are still returning undefined instead of the old data. Has the intended behavior changed or should I open a bug report?

[claycoleman]

The intended behavior changed, if you want the previous data you can use previousData from the result object returned from the React Query