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

graphql HOC Creates New Reference to Mutation if any props Change #725

Closed
derek-duncan opened this issue May 25, 2017 · 4 comments
Closed

Comments

@derek-duncan
Copy link

Steps to Reproduce

import React from 'react';
cont withMyMutation = graphql(gql`...`);

class Komponent extends React.PureComponent {
  shouldComponentUpdate(nextProps) {
    console.log(nextProps.mutate === this.props.mutate); // this will always be false, even if the `name` prop never changes.
    return true;
  }
  render() {
    return <button onClick={this.props.mutate}>{this.props.name}</button>
  }
}

export default withMyMutation(Komponent);

Buggy Behavior

The graphql wrapper will always create a new reference to the mutate property. The same issue applies for any function in the props option. Thus, using any shallow comparison like React's PureComponent will always fail.

Expected Behavior

If possible, I the graphql wrapper should keep a single reference to the mutate prop. This should greatly improve performance. If the same logic could be applied to graphql's props option too (similar to what recompose's withHandlers does), that would be perfect.

The "work around" right now is to assign the mutate prop to a function on a wrapper class, and then pass that class function reference down to PureComponent children.

import React from 'react';
cont withMyMutation = graphql(gql`...`);

class Komponent extends React.PureComponent {
  render() {
    return <button onClick={this.props.mutate}>{this.props.name}</button>
  }
}

class WrapperKomponent extends React.Component {
  mutate = () => this.props.mutate;
  render() {
    const { mutate, ...props } = this.props;
    return <Komponent mutate={this.mutate} {...props} />
  }
}

export default withMyMutation(WrapperKomponent);

Version

@derek-duncan derek-duncan changed the title graphql HOC Creates New Reference to Mutation if props Change graphql HOC Creates New Reference to Mutation if any props Change May 25, 2017
@stale stale bot added the wontfix label Jul 5, 2017
@stale
Copy link

stale bot commented Jul 5, 2017

This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if not further activity occurs. Thank you for your contributions to React Apollo!

@settings settings bot removed the wontfix label Jul 8, 2017
@stale
Copy link

stale bot commented Jul 29, 2017

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

@stale
Copy link

stale bot commented Aug 22, 2017

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

@stale stale bot closed this as completed Sep 5, 2017
@stale
Copy link

stale bot commented Sep 5, 2017

This issue has been automatically closed because it has not had recent activity after being marked as no recent activyt. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to React Apollo!

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

1 participant