Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

failing test for #170 #666

Closed
wants to merge 1 commit into from
Closed

failing test for #170 #666

wants to merge 1 commit into from

Conversation

jbaxleyiii
Copy link
Contributor

With the introduction of recycled queries, components that remount with different variables that get the same data (i.e. a null response from the server) get stuck in loading state.

This is currently just an error reproduction from https://github.com/comus/react-apollo-issue

TODO:

  • If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change
  • If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

@jbaxleyiii
Copy link
Contributor Author

@helfer it's been a while 👋 but I'm trying to be an actual maintainer again! I merged a couple PR's and tackled a couple low hanging fruit. I need to get my local AC => RA setup working on windows before I could fix this one outright.

Do you still have the tuesday meeting? How could I best help triage / add features/ contribute again?

@thebigredgeek
Copy link
Contributor

Well, at least we are able to lock it down to a test case now :D

@helfer
Copy link
Contributor

helfer commented May 6, 2017

@jbaxleyiii I think this test could be broken. When you change variables it's expected that the component is first notified while data is still loading. To test for #170, we would have to make sure that the last time the handle is called, loading is false. I'm trying to build a reproduction for apollo-client, but I'm not having much luck because I think it involves a race condition that's hard to reproduce.

@jbaxleyiii
Copy link
Contributor Author

@helfer hmm, the test was written from behavior I noticed on one of the sample error templates. The route was changing (which was the variable, but the result of the two queries was the same (no person data loaded). Instead of showing no person loaded, it maintained a loading state.

@helfer
Copy link
Contributor

helfer commented May 9, 2017

@jbaxleyiii sure, I don't doubt that. I'm just saying that it might not be wrong that the component first receives the props with loading: true, and then later goes to loading: false. The test should make sure that loading stays true.

@jbaxleyiii jbaxleyiii closed this May 30, 2017
@jbaxleyiii jbaxleyiii deleted the 170 branch July 5, 2017 20:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants