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

data set to {} between re-fetches #117

Open
FezVrasta opened this issue Mar 26, 2019 · 10 comments · May be fixed by #134
Open

data set to {} between re-fetches #117

FezVrasta opened this issue Mar 26, 2019 · 10 comments · May be fixed by #134

Comments

@FezVrasta
Copy link

I created this small example to show my issue:

https://codesandbox.io/s/ovw0m6yl4z

Basically, every time I re-render the component and useQuery fetches new data, the data object is set to {} while loading is true.

How can I preserve the previously fetched data while I load new data?

@Dastari
Copy link

Dastari commented Mar 28, 2019

I generally store the data in another state variable then update it in a useEffect that checks if data exists and it's not loading before assigning the state variable.

const MyFuncComponent = () => {
  const [notNullData, setNotNullData] = useState();
  const { data, error, loading } = useQuery(QUERY);

  useEffect(() => {
    if (data && !loading) {
      setNotNullData(data);
    }
  }, [data]);

  return <ComponentThatRendersData data={notNullData} />;
};

Not sure if there is a better way or not. Also notNullData can actually be null when it's initialized, just bad naming variable on my part.

Also here's the updated example:

https://codesandbox.io/s/93ypnxoqv4

@FezVrasta
Copy link
Author

FezVrasta commented Mar 28, 2019

@Dastari thanks, but it would really make sense to fix it in this library... I tried to look at the code but couldn't figure out where the problem is.

Also, just for completeness, let's use hooks :-P

const useQueryNotBugged = (...args) => {
  const [notNullData, setNotNullData] = useState();
  const { data, error, loading } = useQuery(...args);

  useEffect(() => {
    if (data && !loading) {
      setNotNullData(data);
    }
  }, [data]);

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

@FezVrasta
Copy link
Author

FezVrasta commented Mar 28, 2019

I tried to add || result.loading to https://github.com/trojanowski/react-apollo-hooks/blob/master/src/useQuery.ts#L140

The issue is the observableQuery gets invalidated when the query variable change, so the observableQuery.getLastResult() returns undefined, because we are asking the lastResult to the wrong observableQuery...

edit: I think I fixed it, I sent a PR (#121)

@trojanowski
Copy link
Owner

trojanowski commented Apr 3, 2019

@FezVrasta long term it will be solved with a combination of React Suspense and Concurrent Rendering (it should even work in the current version, but it's experimental, and I don't recommend to use it in production). About PR #121 - I don't think it's a good idea - I think it's better to be more explicit than to show stale data. I'm also not sure if it works with fetchMore - please notice that loading is set to true during loading more data. I'd also prefer not to work differently than react-apollo here.

@FezVrasta
Copy link
Author

FezVrasta commented Apr 3, 2019

@trojanowski react-apollo doesn't empties the data object between refetches, this issue exists exactly because the behavior is different (and generates a bug on our app)

@FezVrasta
Copy link
Author

Please take a look at the same example, but made with react-apollo:
https://codesandbox.io/s/ywp7w756vv

@lukewlms
Copy link

lukewlms commented Apr 9, 2019

This is broken at present - upgrading from vanilla React Apollo to this breaks our app because the data disappears as soon as the refresh starts. Added CR comments, please prioritize if possible!

@FezVrasta
Copy link
Author

@trojanowski may we get an updated take on this issue after my recent clarifications?

If your concern with my PR is just the fetchMore behavior, I can work on it and fix the issue (if present).

If you intend to keep this bug, please at least let us know so we can find alternative solutions.

Thank you for your work!

trojanowski added a commit that referenced this issue Apr 18, 2019
…hat 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

Closes #117
Closes #121
Closes #129
@trojanowski trojanowski linked a pull request Apr 18, 2019 that will close this issue
2 tasks
@trojanowski
Copy link
Owner

@FezVrasta I'm not a fan of this behavior (it's a breaking change for me in some cases), but I'm going to fix it to make it similar to react-apollo. I prepared #134 - it uses setValues similar to how it's done there. It should also fix #129. Unfortunately, it requires further work (currently it breaks the suspense mode).

FezVrasta added a commit to FezVrasta/react-apollo-hooks that referenced this issue Apr 23, 2019
@karibertils
Copy link

Any news on this ?

levindixon pushed a commit to levindixon/react-apollo-hooks that referenced this issue Aug 22, 2019
…hat 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

Closes trojanowski#117
Closes trojanowski#121
Closes trojanowski#129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants