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

[Feature] add prop onResolved={data => xxx} to Query #1947

Closed
xialvjun opened this issue Apr 26, 2018 · 11 comments
Closed

[Feature] add prop onResolved={data => xxx} to Query #1947

xialvjun opened this issue Apr 26, 2018 · 11 comments

Comments

@xialvjun
Copy link

Use some concept in redux to describe this question:

  1. browser come to a route;
  2. do a query on server to get data;
  3. use the data do some other effect, for example, log;
  4. render the data to html;

So, how can we do 3?

graphql(queyr_document, options) and <Query /> are both declarative programming, but some times we need imperative programming...

If I use client.query() in componentDidMount, then I lost the goods from declarative programming(subscribing to later changes)...

Maybe add API onResolved={data => xxx} or whatever name is OK.

Of course, graphql(queyr_document, options) need it too.

@jarommadsen
Copy link

jarommadsen commented Jun 28, 2018

Especially in regards to the <Query> component I think that having callbacks is more inline with react conventions than render props anyway. Apollo is basically trying to force you to have stateless components which seems like a bit of an anti-pattern to React. Why even make a component if you're not going to provide a callback of any kind?

@jarommadsen
Copy link

jarommadsen commented Jun 28, 2018

As a workaround I've set notifyOnNetworkStatusChange=true and compared networkStatus against a custom component variable this.lastNetworkStatus to create my own callback.

<Query
  query={query}
  notifyOnNetworkStatusChange>
  {(args) => {
    const { loading, data, networkStatus } = args;
    if (!loading && this.lastNetworkStatus !== networkStatus) {
      this.onChange(args);  // Your callback here
      this.lastNetworkStatus = networkStatus;
    }
</Query>

@xialvjun
Copy link
Author

My use case is:

<Query
  query={query}
  variables={variables}
  onComplete={({ query, variables, data, error }) => do_some_side_effects()}
>
  {({ loading, error, data }) => show_the_ui({ loading, error, data })}
</Query>

Because do side effect in render function is not good.
And it's not convenient to detect Query is complete event in render function because the props of Query may be changed even it's rare.

So, maybe the best way is to give a prop onComplete event handler. And since the props of Query may be changed, onComplete should get it's origin query, variables as params.

@jarommadsen
Copy link

I realize that my previous post may have not been clear, but I do agree that it would be far more convenient for Apollo to provide a callback for a network status change or an onCompleted callback like the one on the Mutation component. That said you should still be able to accomplish your desired effect using a custom callback in the render function. Do be aware that the render function does get recalled every time the network status changes or new props are passed in. The only main drawback to making your own callback is that changing state inside the render function is dangerous as it can lead to infinite rerendering loops if you aren't careful.

@jarommadsen
Copy link

In my use case I have an infinite scroll list with each item having a checkbox. I also have a list control to select/unselect all of the checkboxes that has it's value controlled by a state. I needed a way to change the value of the select all after getting new items. Without some kind of callback this functionality is awkward at best requiring a half-controlled-half-uncontrolled checkbox.
Using my workaround of monitoring the networkStatus works but does give a warning for modifying state inside of the render. I've removed the warning by adding a 0ms interval to trigger the callback.

onChange(args) {
  clearInterval(this.changeTimer);
  this.props.onChange(args);  // Your callback here
}

render() {
  return (
    <Query
      query={query}
      notifyOnNetworkStatusChange>
      {(args) => {
        const { loading, data, networkStatus } = args;
        if (!loading && this.lastNetworkStatus !== networkStatus) {
          this.changeTimer = setInterval(this.onChange, 0, args);
          this.lastNetworkStatus = networkStatus;
        }
      }}
    </Query>
  )
}

@jarommadsen
Copy link

PR made several months ago for this feature: #1922

@xialvjun
Copy link
Author

┓( ´∀` )┏

@hwillson
Copy link
Member

PR #1922 has been merged (and will be included in the next react-apollo release on June 3, 2018). Thanks!

@jarommadsen
Copy link

@hwillson I assume you mean *July 3rd? Or does Apollo time travel?

@windgaucho
Copy link

I am asking my self the same too :)

@hwillson
Copy link
Member

Yes definitely - thanks for catching that @jarommadsen @windgaucho! I guess I'm not quite ready for July yet ... ⏲ ✈️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants