react-apollo-hooks icon indicating copy to clipboard operation
react-apollo-hooks copied to clipboard

[wip] fix(useQuery): use `setOptions` for updating query

Open trojanowski opened this issue 5 years ago • 12 comments

Thanks to that previous value of data is shown while loading new one and networkStatus should have a correct value in that case

BREAKING CHANGE: previous value of data is shown while loading new one

  • [ ] fix suspense mode
  • [ ] add tests

Closes #117 Closes #121 Closes #129

trojanowski avatar Apr 18 '19 17:04 trojanowski

WIP - unfortunately suspense mode doesn't work with that version

trojanowski avatar Apr 18 '19 17:04 trojanowski

Thanks so much for this!!

FezVrasta avatar Apr 18 '19 17:04 FezVrasta

I'm super excited about showing stale data while refreshing.

fbartho avatar Apr 18 '19 17:04 fbartho

@trojanowski can we do something to help you with this PR? I could add tests if you agree

FezVrasta avatar Apr 23 '19 08:04 FezVrasta

@trojanowski can we do something to help you with this PR? I could add tests if you agree

@FezVrasta it would be nice, thanks. Right now the suspense mode is also broken - I'll try to fix (in the next week I think).

trojanowski avatar Apr 23 '19 11:04 trojanowski

@trojanowski I tried to add a test for #117 but I can still reproduce the bug.

You can try my branch here

FezVrasta avatar Apr 23 '19 12:04 FezVrasta

@FezVrasta yes, it seems this PR requires further work. We should implement merging previous data with the last one. react-apollo implements it that way: https://github.com/apollographql/react-apollo/blob/master/src/Query.tsx#L474. Strange that your codesandbox demo from #117 worked for me with this PR in the current form.

trojanowski avatar Apr 23 '19 14:04 trojanowski

@trojanowski is this still on your radar? I'm sorry for being so pushy but I would really love to use this project in production :-(

FezVrasta avatar May 06 '19 15:05 FezVrasta

@FezVrasta in case it helps, we're currently working around the data reset issue with the following:

import { useRef, useEffect } from 'react';
import { useQuery as useReactApolloHooksQuery } from 'react-apollo-hooks';

export function useQuery(query, options) {
	const prevDataRef = useRef();
	const { loading, error, data } = useReactApolloHooksQuery(query, options);
	const hasData = data && Object.keys(data).length;
	const activeData = hasData ? data : prevDataRef.current;

	useEffect(() => {
		if (hasData) {
			prevDataRef.current = data;
		}
	});

	return {
		data: activeData,
		loading,
		error,
	};
}

jjenzz avatar May 07 '19 16:05 jjenzz

@jjenzz i had thought that updating a ref doesn't trigger a rerender. is the idea here that something else (like error) updating triggers the rerender, then you just use the new value of prevDataRef.current?

(this gets deeper into my hooks confusion, so apologies if that doesn't totally make sense)

zhammer avatar May 07 '19 16:05 zhammer

@zhammer you are correct that a ref doesn’t trigger a rerender. The code above keeps a reference to successfully retrieved data so that if the data becomes empty in a future render it can return that cached data instead. Does that help?

jjenzz avatar May 07 '19 17:05 jjenzz

@trojanowski any idea if we can anyhow go around the Suspense issue? Any idea why it doesn't work on this PR?

jtomaszewski avatar Jul 22 '19 12:07 jtomaszewski