New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: getClient calls `options` with with current props #1005

Closed
koenpunt opened this Issue Aug 22, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@koenpunt
Copy link
Contributor

koenpunt commented Aug 22, 2017

When skip transitions from true to false due to a prop change, the options method is called several times. The times it is called to retrieve for example the variables for the query it is called with the components nextProps. But the internal getClient method calls it with this.props. As this hasn't been brought up as of yet, I recon nobody has used skip in the way I did.

Take for example this options object:

{
  options: ownProps => ({
    variables: {
      gender: ownProps.person.gender,
    },
  }),
  skip: ownProps => !ownProps.person,
}

So as long as there's no person property available I don't want to execute the query.

But the moment the person property is set, options is called several times. Not sure of the order, but at least for skip, variables and client. When it is called for skip or variables, the nextProps are passed, and ownProps.person.gender will return the persons gender.

But when options is called for the client, this.props is passed, which at that point does not yet contain the person prop, and thus accessing ownProps.person.gender will fail with an error.

Intended outcome:

For the client, options should also be called with the value of nextProps

Actual outcome:

App throws an error: Uncaught TypeError: Cannot read property 'gender' of undefined
image

How to reproduce the issue:

I've created a minimal example demonstrating the issue using the error-template (note the branch):
Just run it with the developer console open and the problem will present itself.

https://github.com/koenpunt/react-apollo-error-template/tree/get-client-prev-props

Version

  • apollo-client@1.9.1
  • react-apollo@1.4.14

@jbaxleyiii jbaxleyiii self-assigned this Aug 24, 2017

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Aug 24, 2017

@koenpunt thanks for the detailed usage and report! I'll take a look!

jbaxleyiii pushed a commit that referenced this issue Aug 24, 2017

@jbaxleyiii jbaxleyiii referenced this issue Aug 24, 2017

Merged

1005 #1025

1 of 1 task complete

jbaxleyiii pushed a commit that referenced this issue Aug 24, 2017

jbaxleyiii pushed a commit that referenced this issue Aug 24, 2017

James Baxley
1005 (#1025)
* use up to date props for options, fixes #1005

* changelog

* spelling

jbaxleyiii pushed a commit that referenced this issue Aug 24, 2017

James Baxley
1007 (#1027)
* use up to date props for options, fixes #1005

* changelog

* handle componenDidMount refetch call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment