query results are undefined during loading (due to breaking change in @apollo/client v3)

[kevinashworth]

There was an intentional breaking change in Apollo Client 3.0.0.

When refetching or when using loadMore, results and totalCount revert temporarily to undefined.

For in-depth information and debate, see threads like

• https://github.com/apollographql/apollo-client/blob/46706b53cd36a073800e9abbd4788d0b72c0dab2/CHANGELOG.md#react,
• [^3.0.0] data is undefined while loading apollographql/apollo-client#7038 (comment)
• AC3: useQuery does not return old data anymore when variable change apollographql/apollo-client#6603

It could impact Vulcan in a number of ways, but so far I have only been looking at multi2.js. See Vulcan discussion at https://vulcanjs.slack.com/archives/C2LJQHTLP/p1601657376018800

A possible solution is to update to @apollo/[email protected] and override useQuery. Or change

Vulcan/packages/vulcan-core/lib/modules/containers/multi2.js

Lines 110 to 113 in 78d0e99

	const { refetch, networkStatus, error, fetchMore, data, graphQLErrors } = returnedProps;
	// Note: Scalar types like Dates are NOT converted. It should be done at the UI level.
	const results = data && data[resolverName] && data[resolverName].results;
	const totalCount = data && data[resolverName] && data[resolverName].totalCount;

to something like this:

  const { refetch, networkStatus, error, fetchMore, graphQLErrors, data, previousData } = returnedProps
  const existingData = data ?? previousData
  const results = existingData && existingData[resolverName] && existingData[resolverName].results;
  const totalCount = existingData && existingData[resolverName] && existingData[resolverName].totalCount;

[ErikDakoda]

Thank you for bringing this to our attention. I say we upgrade to @apollo/[email protected] and try the suggested change to multi2.js. I would be happy to look at multi.js as well.

[kevinashworth]

Seems beta.10 is latest; should work, too. And is there a better variable name than 'existingData'. Probably, I didn't think about that very long! Anyway, my two-repo install seems to be working fine with these two changes.

[eric-burel]

If that's just a matter of waiting Apollo to update you can probably quickfix in your codebase and let them fix it. Is there an action required from Vulcan in the long run?

[kevinashworth]

The Apollo breaking change is intentional and probably won't be fixed, in the sense of reverted to previous behavior, so I think Vulcan needs to change its codebase. I do not yet know if the suggested fix from Apollo comments (data ?? previousData) is the change for Vulcan to make in the long run, however.

[eric-burel]

Ok makes sense, I'll check that. In the meantime you can implement the "data/previousData" stuff locally with useState.
I'll do a big pass on all tickets when the NPM version is a bit more advanced (it is easier to test so I am more productive than with Meteor).

[ErikDakoda]

In the meantime you can implement the "data/previousData" stuff locally with useState.

@eric-burel I am sorry, maybe I misunderstand what you are suggesting, but that would require changes in 30+ components in my own codebase and would be a big setback to productivity.

[eric-burel]

Yeah makes sense indeed. I would be happy to merge the proposed change asap, it sounds relevant to me.

[kevinashworth]

What if we create a new setting acceptPossiblyStaleDataFromApollo? It defaults to true so that nobody has to set it unless they know the complexities of this issue. It tells multi and multi2 (and perhaps others) to use data ?? previousData if the setting is true.

[eric-burel]

results is a Vulcan specific shortcut (btw in NPM version I renamed it documents for consistency with mutations) so we are free to implement whatever default behaviour we want, no need for an option, and I think what you proposed is the best approach as it prevents a breaking change in Vulcan.
People can still access the default data object from Apollo if they want the default behaviour.

[AaronWatson2975]

Hey all,



I was just wondering if you solved this, and if you did, what implementation did you use?



We ran into the same issue and luckily we already had a custom hook useSafeQuery we were using for queries that throws errors for our error boundaries. Since every one of our queries was already using this we created a custom hook to persist the data and implemented that in our existing hook.

In the interest of avoiding the beta release of Apollo, we chose to store the data in useState and only update it when loading is false. 

We flagged it with a custom option called clearPreviousDataOnLoad which defaults to false, this way we didn’t have to update a single component that depends on it, and if we wanted the functionality we can simply call it like this.



const { data } = useSafeQuery(query, { clearPreviousDataOnLoad: true });



It works great but I’m just curious what other people are using to get around this. I feel like we got lucky that we already had useQuery wrapped in a custom hook.

[GraemeFulton]

I'm using oldschool react classes, so keep the results in component state. Then I check and update the results in componentDidUpdate, and render from the state's copy:

    constructor(props) {
        super(props)
        this.state = {
            results: props.results
        }
    }

    componentDidUpdate(prevProps, prevState) {
        if (this.props.results && (this.props.results != prevProps.results)) {
            if (this.props.results.length > 0) {
                this.setState({
                    results: this.props.results
                })
            }
        }
    }

render() {
  <>
      {this.state.results && this.state.results.map(user => {
        return (
          <ProfileCard user={user} />
          );
      }) 
    }
  </>
}

Then showing loading state based on whether there are results:

{(!this.props.results && this.props.results!==0) && 'Loading...'}

It's not ideal but does the trick for now