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

How to determine the mutation loading state? #421

Closed
ctavan opened this issue Jan 16, 2017 · 9 comments
Closed

How to determine the mutation loading state? #421

ctavan opened this issue Jan 16, 2017 · 9 comments

Comments

@ctavan
Copy link

ctavan commented Jan 16, 2017

This is essentially a re-post of http://stackoverflow.com/questions/39379943/how-to-determine-mutation-loading-state-with-react-apollo-graphql

I am wondering if there is an idiomatic way of determining the loading state or networkStatus of a mutation. It's obvious how this is done for queries but unlike queries I cannot find a built-in loading state indication for mutations.

What I have been doing so far is set the components state in the handler that calls this.props.mutate() and then reset that state in the result- and error-callbacks of mutate(). However this adds a lot of repetitive code to any component that triggers a mutation.

I also reckon that optimistic updates are often more suitable than indicating a loading state for mutations, however I think there are still enough cases where this is not reasonably possible. While it's probably best for something like a blog comment or chat message to be handled in an optimistic fashion I think there are still many mutations where you cannot reasonably update the UI without feedback from the server, e.g. when a user wants to log into your application or for any transaction that involves payments…

So I wonder: What is the recommended way of handling mutation loading state with react-apollo?

Version

  • apollo-client@0.7.2
  • react-apollo@0.8.1
@tmeasday
Copy link
Contributor

I think you have it pretty much right and the issue is more a lack of documentation about it.

It doesn't really make sense to include loading state on the mutation container; if you think about it you could call the mutation twice simultaneously -- which loading state should get passed down to the child? My feeling is in general it's not nice to mix imperative (this.mutate(x, y, z)) with declarative (props) things; it leads to irresolvable inconsistencies.

Maybe there's a library that could help reducing the boilerplate?

@ctavan
Copy link
Author

ctavan commented Jan 17, 2017

@tmeasday thanks for the response. I ended up writing a higher order component to wrap my components that contain mutations, see https://gist.github.com/ctavan/7219a3eca42f96a5c5f755319690bda7

This allows me to reduce the code duplication to a minimum.

@lhz516
Copy link

lhz516 commented Jul 31, 2017

I also wrote a HOC to handle this issue. I hope it can help.

https://github.com/lhz516/react-apollo-mutation-state

@tkvw
Copy link

tkvw commented Sep 29, 2017

Just for reference, I created a hoc for this as well, which is a bit more transparent:
Configuration:

compose(
    graphql(mutation,/*config*/),
    withStateMutation(/*config*/)
)

This passes down the properties: mutateLoading,mutateError,mutateResult. Actually it will use the name of the mutate property in the config, i.e. withStateMutation({name:'delete'}) resolves in deleteLoading,deleteError and deleteResponse.

Usage: just call mutate({variables..} like you normally would.

hoc:

const withStateMutation = ({ name = 'mutate' } = {}) => WrappedComponent => class extends React.Component {
    loadingProperty = `${name}Loading`;
    errorProperty = `${name}Error`;
    resultProperty = `${name}Result`;

    state = { loading: false, error: null, result: null };
    
    handleMutation(options) {
        this.setState({
            loading: true,
            error: null,
            result: null,
        });
        return this.props[name](options)
            .then((result) => {
                this.setState({
                    loading: false,
                    error: null,
                    result: result,
                })
            })
            .catch((err) => {
                this.setState({
                    loading: false,
                    error: err,
                    result: null
                });
            })
    }

    render() {
        const props = {
            ...this.props,
            [name]: this.handleMutation.bind(this),
            [this.loadingProperty]: this.state.loading,
            [this.errorProperty]: this.state.error,
            [this.resultProperty]: this.state.result,
        };
        return <WrappedComponent {...props}/>
    }
};

@erichochhalter
Copy link

@tkvw, I really like this HOC solution.

I found in my implementation that if we wish to chain on the original mutation's promise elsewhere in our code, we need to add a return result; to the handleMutation .then block.

So the revision looks like this, for anyone else who may find it helpful:

    handleMutation(options) {
        this.setState({
            loading: true,
            error: null,
            result: null,
        });
        return this.props[name](options)
            .then((result) => {
                this.setState({
                    loading: false,
                    error: null,
                    result: result,
                });
                return result;  // <-- there!
            })
            .catch((err) => {
                this.setState({
                    loading: false,
                    error: err,
                    result: null
                });
            })
    }

Which restores our ability to work imperatively with the side-effect, should you wish to do so:

handleSubmitNamedMutation = (value) => {
  this.props.myNamedMutation({
    variables: { var: value }
  })
  .then(response => {
    // do imperative stuff
  });
};

@bostrom
Copy link
Contributor

bostrom commented Dec 13, 2018

I also found it useful to re-throw the error in the .catch, otherwise it will return undefined, which will be interpreted down the line as the mutation having being resolved.

    handleMutation(options) {
        this.setState({
            loading: true,
            error: null,
            result: null,
        });
        return this.props[name](options)
            .then((result) => {
                this.setState({
                    loading: false,
                    error: null,
                    result: result,
                });
                return result;
            })
            .catch((err) => {
                this.setState({
                    loading: false,
                    error: err,
                    result: null
                });
                throw err;
            })
    }

Otherwise the .then branch would get executed when an error occurs:

handleSubmitNamedMutation = (value) => {
  this.props.myNamedMutation({
    variables: { var: value }
  })
  .then(response => {
    // we'd end up here
  })
  .catch(err => {
    // now we also get this
  }) ;
};

@bxt
Copy link

bxt commented Jan 3, 2019

Thank you all for the workarounds! But maybe this issue can be solved in Apollo after all? Judging by the number of comments and workarounds here I think this issue is still interesting to some people.

@tmeasday Your rationale back in the day for closing it was:

It doesn't really make sense to include loading state on the mutation container; if you think about it you could call the mutation twice simultaneously -- which loading state should get passed down to the child?

Interestingly, we now have the loading state as part of the <Mutation>'s render props, so I think this is not valid anymore.

Maybe this issue can be reopened and the HoC can provide loading just like the render prop API?

@tmeasday
Copy link
Contributor

tmeasday commented Jan 4, 2019

Seems reasonable to me although I am not working on this package at the moment so it is not up to me 🤷🏼‍♂️

@Infonautica
Copy link

Infonautica commented Sep 11, 2019

Here is my TypeScript solution:

export interface WithStateMutationProps<TMutateFn = Function, TData = any> {
  mutateFn: TMutateFn;
  data: NonNullable<MutationResult<TData>['data']>;
  loading: NonNullable<MutationResult<TData>['loading']>;
  error: NonNullable<MutationResult<TData>['error']>;
}

export function withStateMutation<TData = any, TVariables = any>(
  query: any,
  operationOption: OperationOption<TData, TVariables>
) {
  return (WrappedComponent: React.ComponentType<any>) => (parentProps: any) => {
    return (
      <Mutation<TData, TVariables> mutation={query}>
        {(mutateFn, { data, loading, error }) => {
          const name = operationOption.name || 'mutation';
          const props = {
            [name]: {
              mutateFn: mutateFn,
              data,
              loading,
              error,
            },
          };

          return <WrappedComponent {...parentProps} {...props} />;
        }}
      </Mutation>
    );
  };
}

And usage example:

// Provide gql query and operationOptions as usual
export default withStateMutation(UPDATE_CUSTOMER_FILE_MUTATION, { name: 'updateCustomerFile' })(CustomerSecurityPhotos);

// Use mutateFn in code
this.props.updateCustomerFile
      .mutateFn({
        variables: {
          id,
          data: {
            state: CustomerFileState.ACCEPTED,
          },
        },
      })

// Use data, loading and error from props in render
const { loading } = this.props.updateCustomerFile;
return <Button disabled={loading} type="primary" icon="check" onClick={this.handleAccept(id)} />

// Dont forget to add in props. I use generated from code-generator typings
interface IProps extends IExternalProps {
  updateCustomerFile: WithStateMutationProps<UpdateCustomerFileMutationFn, UpdateCustomerFileMutation>;
}

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

8 participants