Move skip out of options, and avoid calling props and options if it's false

[stubailo]

I think if skip: true is passed, it should be as if this container doesn't exist at all. Right now, I have a props function that expects a data object, and it's still called but without any data if I pass skip: true.

[jbaxleyiii]

@stubailo it calls props to create the object sent to the client. If we didn't call it when skip is true, then things like the variables, refetch(), etc wouldn't be possible.

I'm not opposed to this but need to think through what all it would remove.

Just curious, do you have a use case where you don't want props() called?

[stubailo]

If we didn't call it when skip is true, then things like the variables, refetch(), etc wouldn't be possible.

I thought the point of skip is that the query doesn't run at all -- if the query is not running, why do you need refetch etc?

[jbaxleyiii]

@stubailo thinking through this more. Right now props() acts as a reducer to create custom props. When skip: true it doesn't run the query, but it is still useful to run the reducer. This also helps with PropType validation when working locally because the reducer method should create the required props. Example:

@graphql(QUERY, {
  props({ ownProps, data }){
     if (!data.myFieldToShow) return { canRender: false }
     return {
        canRender: true,
        myFieldName: data.myFieldToShow,
     }
  }
})
class Thing extends Component {

  static propTypes = {
    canRender: PropTypes.bool.isRequired
  }

  // etc

}

Thoughts? @stubailo

[stubailo]

Hmm, can't I use getDefaultProps for that?

[stubailo]

Another situation, if you have code like the below:

export default FooWithData = graphql(query, {
  options: ({foo}) => ({
    skip: !foo,
    variables: {
      bar: foo.bar
    }
  })
})(Foo);

It turns out that foo.bar doesn't exist.

My proposed solution is now:

graphql(query, {
  skip: ({ foo }) => !foo,
  options: ({ foo }) => ({ ... }),
});

skip would be evaluated before everything else, and it would short-circuit the entire HOC.

[stubailo]

Closing in favor of #245