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

Add onCompleted prop to Query #1922

Merged
merged 10 commits into from
Jun 29, 2018
Merged

Add onCompleted prop to Query #1922

merged 10 commits into from
Jun 29, 2018

Conversation

jeshep
Copy link
Contributor

@jeshep jeshep commented Apr 11, 2018

Allows a function to be passed into the Query component that will be run once the query has completed. Useful for modifying the local state with data from a query, e.g. initialize the select value to the first item from the query results.

Reference Issue: #1862

Not sure if there is a better place in the Query component to call the provided onCompleted function, but this implementation seems to work.

Checklist:

  • [] If this PR is a new feature, please 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
  • If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

@apollo-cla
Copy link

@jeshep: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@ghost ghost added feature New addition or enhancement to existing solutions labels Apr 11, 2018
@tab00
Copy link

tab00 commented Apr 23, 2018

I hope this will be merged soon, as I actually need it right at this moment.

@tab00
Copy link

tab00 commented Apr 29, 2018

What's the hold up? This should have been in the original implementation.

@ajoslin
Copy link

ajoslin commented May 11, 2018

Ping. This needs to happen. I'm forced to use withApollo and props.client.query in componentDidMount until this lands.

@dispix
Copy link

dispix commented May 30, 2018

Was about to open the same PR when I saw yours. This feature would be indeed much appreciated, especially with the new render prop approach that makes the data unavailable through props.

Shouldn't this PR also includes an onError prop ?

@jimshepherd
Copy link

Added onError prop to the PR.

Would be nice to get some feedback on this PR from the maintainers.

@ajoslin
Copy link

ajoslin commented Jun 5, 2018

Apollo devs, how can we support in getting this merged? This would really help.

@jarommadsen
Copy link

Would love to see this make it through. Haven't peeked at the changes yet but will onCompleted be fired on fetchMore?

@hwillson hwillson self-assigned this Jun 29, 2018
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome @jeshep - thanks very much for working on this (and sorry for the long delay)! We're all set here, but I'll hold off on merging for a few more mins while I get the API docs updated in the apollo-client repo. Thanks again!

@jimshepherd
Copy link

Awesome! Glad I could contribute.

hwillson added a commit to apollographql/apollo-client that referenced this pull request Jun 29, 2018
@hwillson hwillson merged commit 55e0392 into apollographql:master Jun 29, 2018
@ajoslin
Copy link

ajoslin commented Jun 29, 2018

🎉

@olistic
Copy link
Contributor

olistic commented Jul 20, 2018

Hi! Would anyone in this thread mind shedding some light on #2177? It looks like an oversight of this PR, but maybe it's not and it's by design. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants