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

HOC violates contract of componentWillReceiveProps #1611

Closed
rocketraman opened this issue Jan 31, 2018 · 6 comments
Closed

HOC violates contract of componentWillReceiveProps #1611

rocketraman opened this issue Jan 31, 2018 · 6 comments

Comments

@rocketraman
Copy link

rocketraman commented Jan 31, 2018

Intended outcome: When I wrap a component with a graphql HOC, the behavior of React's calls to componentWillReceiveProps on the underlying component should not change.

Actual outcome: Currently, when wrapping with a HOC, componentWillReceiveProps is no longer called in situations in which it should be.

This is due to the shallowEquals done on all props here:

shallowEqual(this.props, nextProps) &&

This comparison should either be removed, or it should account for props not managed by the HOC.

How to reproduce the issue:

  1. https://codesandbox.io/s/4q0645k8w
  2. Click on render.
  3. Click on Set state foo=false.
  4. Click on render again.

In step 4, the component state should update via componentWillReceiveProps. It does not.

In this sandbox, we have the exact same App component, without the Apollo HOC: https://codesandbox.io/s/xpyjmknp14 and you can see that the behavior is now correct.

Version: 2.0.4

  • apollo-client@2.2.1
  • react-apollo@2.0.4
@rocketraman rocketraman changed the title https://github.com/apollographql/react-apollo/blob/b1e346c7b8de1187f736a437888b4206c224fda8/src/graphql.tsx#L179 HOC violates contract of componentWillReceiveProps Jan 31, 2018
@rocketraman
Copy link
Author

rocketraman commented Feb 10, 2018

Note the React documentation has changed and now does explicitly add the will (my bold below):

Note that React will call this method even if the props have not changed, so make sure to compare the current and next values if you only want to handle changes. This may occur when the parent component causes your component to re-render.
(https://reactjs.org/docs/react-component.html#componentwillreceiveprops)

Since wrapping a component with an Apollo HOC causes this guarantee to be unmet, if a component relies on this behavior, we need to use crummy workarounds like adding a uuid or counter prop to the component where it is used, just to force componentWillReceiveProps to be called.

@shoebmogal
Copy link

@rocketraman can you give an example on how to force componentWillReceiveProps?

Thanks

@rocketraman
Copy link
Author

@rocketraman can you give an example on how to force componentWillReceiveProps?

@shoebmogal This is the repro recipe using the Apollo HOC "fixed" with a uuid prop: https://codesandbox.io/s/1v23om57v7. It's an ugly hack but it works.

@simonjoom
Copy link

I found the same issue, for me the workaround was to changed to a componentWillMount because our component is rebuilded every state..

simonjoom pushed a commit to simonjoom/prisma-ecommerce that referenced this issue Jun 25, 2018
@damiangreen
Copy link

damiangreen commented Aug 1, 2018

So I'm trying to use polling but my props are not updating at all even though the ajax request is sent. How do other people get this working?

@rocketraman Were you using a SSR boilerplate?

I've tried your workaround with the graph() hoc and passing a uuid() but it doesnt help. The uuid is only created once, and not with each graphql response.

export default compose(
  graphql(query, {
    options: {
      pollInterval: 1000,
    },
  }),
  withStyles(s),
)(MetricWidget);

...

<MetricWidget
             uuid={uuid()}
             title={widget.title}
             type={widget.type}
             id={widget.id}
             options={widget.options}
             dashboardId={dashboard.id}
           />

@hwillson
Copy link
Member

hwillson commented Sep 6, 2019

React Apollo has been refactored to use React Hooks behind the scenes for everything, which means a lot has changed since this issue was opened (and a lot of outstanding issues have been resolved). We'll close this issue off, but please let us know if you're still encountering this problem using React Apollo >= 3.1.0. Thanks!

@hwillson hwillson closed this as completed Sep 6, 2019
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

5 participants