Skip to content
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.

Sign up for GitHub

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

Jump to bottom

componentWillReceiveProps() does not trigger for new context changes #5756

Closed
milesj opened this issue Dec 30, 2015 · 6 comments
Closed

componentWillReceiveProps() does not trigger for new context changes #5756

milesj opened this issue Dec 30, 2015 · 6 comments

Comments

@milesj
Copy link
Contributor

milesj commented Dec 30, 2015

The componentWillReceiveProps() method only triggers if props change. However, there are situations where the context changes, but the props do not, but this method does not trigger. It makes it really difficult to alter the state before an update, as we can't do this in componentWillUpdate().

Perhaps a new method, like componentWillReceiveContext() can be added, or the previous componentWillReceiveProps() can simply be renamed to something like componentWillReceive(), which is always called if any prop/state/context has changed.
@yangshun
Copy link
Contributor

In fact, componentWillReceiveProps does trigger when context changes. Here's an example fiddle of how it can be used and accomplished:

https://jsfiddle.net/bzytthat/

Read more about using context in lifecycle methods: http://facebook.github.io/react/docs/context.html#referencing-context-in-lifecycle-methods

@milesj
Copy link
Contributor Author

milesj commented Dec 31, 2015

I have an implementation that does not work.

https://github.com/titon/toolkit/blob/3.0/src/ui/components/carousel/ItemList.js

When I add componentWillReceiveProps() to ItemList, it never gets triggered, even when the context constantly changes from the parent. No idea why, since in your example it does. Perhaps an ES6 issue?

I am using the latest React 0.14.

Here's an example of the Carousel in use: https://github.com/titon/toolkit/blob/3.0/src/test.js

Edit: Out of curiosity, I added componentWillReceiveProps() to Item, which does trigger. Still no idea why ItemList does not.

Edit 2: I removed everything from ItemList except for a basic render method, and it still would not trigger.

@milesj
Copy link
Contributor Author

milesj commented Dec 31, 2015

I did some debugging in the React source, and it seems to always trigger true for this clause.

react/src/renderers/shared/reconciler/ReactCompositeComponent.js

Line 657 in 3b96650

if (prevParentElement === nextParentElement) {

Which seems to agree with my original issue, that the componentWillReceiveProps() isn't being called because the props haven't changed.

@milesj milesj mentioned this issue Jan 4, 2016
Closed
@milesj
Copy link
Contributor Author

milesj commented Jan 5, 2016

I have a perfect example of this not working correctly. I added a console log statement to every lifecycle method for each child of Carousel. Tested with the following JSX markup.

import Carousel from './ui/components/carousel';

const log = function(e, ...args) {
    console.log(e.constructor.name, e.type, e.detail, e, args);
};

<Carousel
    component="slideshow" modifier="slide"
    debug={true} perCycle={1} loop={true}
    infinite={true} autoStart={false} pauseOnHover={false}>

    <Carousel.ItemList swipe={true} onSwipe={log} onSwipeRight={log}>
        <Carousel.Item index={0}>0</Carousel.Item>
        <Carousel.Item index={1}>1</Carousel.Item>
        <Carousel.Item index={2}>2</Carousel.Item>
        <Carousel.Item index={3}>3</Carousel.Item>
        <Carousel.Item index={4}>4</Carousel.Item>
        <Carousel.Item index={5}>5</Carousel.Item>
        <Carousel.Item index={6}>6</Carousel.Item>
        <Carousel.Item index={7}>7</Carousel.Item>
        <Carousel.Item index={8}>8</Carousel.Item>
    </Carousel.ItemList>

    <Carousel.TabList onClick={log} />

    <Carousel.Next onClick={log}>Next</Carousel.Next>
    <Carousel.Prev onClick={log}>Prev</Carousel.Prev>
    <Carousel.Start onClick={log}>Start</Carousel.Start>
    <Carousel.Stop onClick={log}>Stop</Carousel.Stop>
</Carousel>

And the console output on initial load.

ItemList componentWillMount 
Item componentWillMount (x9)
TabList componentWillMount 
Tab componentWillMount (x9)
Next componentWillMount 
Prev componentWillMount 
Start componentWillMount 
Stop componentWillMount 
Item componentDidMount (x9)
ItemList componentDidMount 
Tab componentDidMount (x9)
TabList componentDidMount 
Next componentDidMount 
Prev componentDidMount 
Start componentDidMount 
Stop componentDidMount 
--------- Updated the state for number of visual items, will render 3 items and 9 tabs ---------
ItemList shouldComponentUpdate 
ItemList componentWillUpdate 
Item componentWillReceiveProps 
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item shouldComponentUpdate 
Item componentWillUpdate 
TabList shouldComponentUpdate 
TabList componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Next shouldComponentUpdate 
Next componentWillUpdate 
Prev shouldComponentUpdate 
Prev componentWillUpdate 
Start shouldComponentUpdate 
Start componentWillUpdate 
Stop shouldComponentUpdate 
Stop componentWillUpdate 
Item componentDidUpdate (x3)
ItemList componentDidUpdate 
Tab componentDidUpdate (x9)
TabList componentDidUpdate 
Next componentDidUpdate 
Prev componentDidUpdate 
Start componentDidUpdate 
Stop componentDidUpdate

The order of the events match the order of the elements in the JSX. HOWEVER, if you look closely, you'll notice that componentWillReceiveProps only triggers for Item and Tab, both of which are 2nd level children. None of the first level children trigger.

And here's the output when you cycle to the next item in the list.

SyntheticMouseEvent click 1 Object { dispatchConfig: Object, dispatchMarker: ".0.2", nativeEvent: click, target: <button.carousel-next>, currentTarget: <button.carousel-next>, type: "click", eventPhase: 3, bubbles: true, cancelable: true, timeStamp: 492246832, 22 more… } Array [  ] 

ItemList shouldComponentUpdate 
ItemList componentWillUpdate 
Item componentWillReceiveProps 
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillMount 
TabList shouldComponentUpdate 
TabList componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Next shouldComponentUpdate 
Next componentWillUpdate 
Prev shouldComponentUpdate 
Prev componentWillUpdate 
Start shouldComponentUpdate 
Start componentWillUpdate 
Stop shouldComponentUpdate 
Stop componentWillUpdate 
Item componentDidUpdate 
Item componentDidMount 
ItemList componentDidUpdate 
Tab componentDidUpdate 
TabList componentDidUpdate 
Next componentDidUpdate 
Prev componentDidUpdate 
Start componentDidUpdate 
Stop componentDidUpdate 

Carousel#6762at6g0kk 3334.095 cycling 1 
Carousel#6762at6g0kk 3334.355 cycled 1

The same thing happens again, componentWillReceiveProps is not called for 1st level children, only 2nd level. I'll try and debug some more, but not really sure where to begin.

@milesj
Copy link
Contributor Author

milesj commented Jan 5, 2016

I tested this using my pull request here: #5776

ItemList componentWillMount 
Item componentWillMount (x9)
TabList componentWillMount 
Tab componentWillMount (x9)
Next componentWillMount 
Prev componentWillMount 
Start componentWillMount 
Stop componentWillMount 
Item componentDidMount (x9)
ItemList componentDidMount 
Tab componentDidMount (x9)
TabList componentDidMount 
Next componentDidMount 
Prev componentDidMount 
Start componentDidMount 
Stop componentDidMount 
--------- Updated the state for number of visual items, will render 3 items and 9 tabs ---------
ItemList componentWillReceiveContext ***
ItemList shouldComponentUpdate 
ItemList componentWillUpdate 
Item componentWillReceiveProps 
Item componentWillReceiveContext ***
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item componentWillReceiveContext ***
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item componentWillReceiveContext ***
Item shouldComponentUpdate 
Item componentWillUpdate 
TabList componentWillReceiveContext ***
TabList shouldComponentUpdate 
TabList componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Next componentWillReceiveContext ***
Next shouldComponentUpdate 
Next componentWillUpdate 
Prev componentWillReceiveContext ***
Prev shouldComponentUpdate 
Prev componentWillUpdate 
Start componentWillReceiveContext ***
Start shouldComponentUpdate 
Start componentWillUpdate 
Stop componentWillReceiveContext ***
Stop shouldComponentUpdate 
Stop componentWillUpdate 
Item componentDidUpdate (x3)
ItemList componentDidUpdate 
Tab componentDidUpdate (x9)
TabList componentDidUpdate 
Next componentDidUpdate 
Prev componentDidUpdate 
Start componentDidUpdate 
Stop componentDidUpdate

And here's the output when you cycle to the next item in the list.

SyntheticMouseEvent click 1 Object { dispatchConfig: Object, dispatchMarker: ".0.2", nativeEvent: click, target: <button.carousel-next>, currentTarget: <button.carousel-next>, type: "click", eventPhase: 3, bubbles: true, cancelable: true, timeStamp: 492246832, 22 more… } Array [  ] 

ItemList componentWillReceiveContext ***
ItemList shouldComponentUpdate 
ItemList componentWillUpdate 
Item componentWillReceiveProps 
Item componentWillReceiveContext ***
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillReceiveProps 
Item componentWillReceiveContext ***
Item shouldComponentUpdate 
Item componentWillUpdate 
Item componentWillMount 
TabList componentWillReceiveContext ***
TabList shouldComponentUpdate 
TabList componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Tab componentWillReceiveProps 
Tab componentWillReceiveContext ***
Tab shouldComponentUpdate 
Tab componentWillUpdate 
Next componentWillReceiveContext ***
Next shouldComponentUpdate 
Next componentWillUpdate 
Prev componentWillReceiveContext ***
Prev shouldComponentUpdate 
Prev componentWillUpdate 
Start componentWillReceiveContext ***
Start shouldComponentUpdate 
Start componentWillUpdate 
Stop componentWillReceiveContext ***
Stop shouldComponentUpdate 
Stop componentWillUpdate 
Item componentDidUpdate 
Item componentDidMount 
ItemList componentDidUpdate 
Tab componentDidUpdate 
TabList componentDidUpdate 
Next componentDidUpdate 
Prev componentDidUpdate 
Start componentDidUpdate 
Stop componentDidUpdate 

Carousel#6762at6g0kk 3334.095 cycling 1 
Carousel#6762at6g0kk 3334.355 cycled 1

So far it works correctly. Still trying to find out why componentWillReceiveProps isn't.

@milesj milesj mentioned this issue Jan 6, 2016
Merged
@milesj
Copy link
Contributor Author

milesj commented Jan 7, 2016

Fixed here: #5787

@milesj milesj closed this as completed Jan 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@milesj @yangshun