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.

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

componentDidReceiveProps Please #3279

Closed
iammerrick opened this issue Feb 27, 2015 · 94 comments
Closed

componentDidReceiveProps Please #3279

iammerrick opened this issue Feb 27, 2015 · 94 comments

Comments

@iammerrick
Copy link
Contributor

I would like to humbly request a componentDidReceiveProps hook, often times I would like to do something on both componentWillMount and componentWillReceiveProps, but because this.props hasn't been set yet I am forced to pass props around instead of reading directly from this.props.

Before New Hook

componentWillMount() {
  this.setup(this.props.id);
}

componentWillReceiveProps(next) {
  this.setup(next.id);
}

setup(id) {
  UserActions.load(id);
}

After New Hook

componentWillMount() {
  this.setup();
}

componentDidReceiveProps() {
  this.setup();
}

setup() {
  UserActions.load(this.props.id);
}

In this simple example it may seem like a small thing, but often times the passing of props runs deep and instead of conveniently referencing this.props one is forced to plumb the props throughout the component.

Please consider adding componentDidReceiveProps as a hook to leverage the same code that is leveraged in componentWillMount without forcing both to plumb props throughout the component.

@syranide
Copy link
Contributor

Why not setup(props)?

@insin
Copy link

insin commented Feb 27, 2015

Looking through my own projects I can see I've done this where I had similar needs (another one is deriving a chunk of state based on props), just passing in a different set of props when needed so you only have to pass something extra when reusing, without duplicating knowledge of which props are needed:

setup(props) {
  props = props || this.props
  UserActions.load(props.id)
}

@iammerrick
Copy link
Contributor Author

@syranide The trouble is when setup needs to call methods that also need props, which needs to call methods which also needs props... Eventually your entire component is plumbing around props.

@JakeLingwall
Copy link

+1 seems like a bunch of needless wiring up in the app with the current pattern. Could be a concise standardized way to solve the problem. Seen a bunch of people get burned on this.props inside of ComponentWillReceiveProps, a clear sign that it's not intuitive.

@gaearon
Copy link
Collaborator

gaearon commented Mar 1, 2015

+1, I also find this frustrating.

No longer find this to be an issue.

@dandelany
Copy link

+1, I am also finding myself passing around props a lot. Similar to @insin above I was using default params for awhile:

setup(props = this.props) {
  doSomething(props);
}

But decided it was an anti-pattern due to the subtle bugs it can cause if you forget to pass newProps.

@monsanto
Copy link
Contributor

+1

@gaearon
Copy link
Collaborator

gaearon commented Mar 25, 2015

I think the reason it's not available is because this.props and this.state always correspond to rendered values. There's no way to implement componentDidReceiveProps that doesn't trigger another render without breaking this constraint.

Most of the times, if you're using componentWillReceiveProps, you actually want either a higher order component a la Relay, or something like observe hook proposed for React 0.14.

@mienaikoe
Copy link

+1
Also, you can implement componentDidReceiveProps without changing this.props or this.state. If all you need to do is read from these, then you won't trigger another render. If you're writing to props or state within this proposed function call, then you're shooting yourself in the foot, but that's the same case for every other method in the lifecycle.

@tristang
Copy link

+1

I want to be able to respond to the new props event when shouldComponentUpdate returns false, so in my case I can't use componentDidUpdate.

@dev-rkoshlyak
Copy link

+1

1 similar comment
@jnmandal
Copy link

+1

@quantizor
Copy link
Contributor

What about componentWillMount and componentWillUpdate @iammerrick

@gaearon
Copy link
Collaborator

gaearon commented Sep 15, 2015

I want to be able to respond to the new props event when shouldComponentUpdate returns false, so in my case I can't use componentDidUpdate.

Use componentWillReceiveProps?

@devjoca
Copy link

devjoca commented Sep 18, 2015

+1

@kmalakoff
Copy link

+1 this helps with DRY code and simplifying logic.

I would be open for componentDidReceiveProps to be called on the initial setup to make the example starting this thread just a single function:

componentDidReceiveProps() {
  UserActions.load(this.props.id);
}

@YourDeveloperFriend
Copy link

Thoughts?

componentWillReceiveProps() {
  setTimeout(()=> {
    if(this.isMounted()) {
      this.componentDidReceiveProps();
    }
  });
}
componentDidReceiveProps() {
  UserActions.load(this.props.id);
}

What do you think?

@kmalakoff
Copy link

@YourDeveloperFriend I think it depends on how it works relative to other lifecycle hooks and rendering delays due to cascading timeouts on nested components.

These sorts of lifecycle hooks should be called synchronously in a pass that is guaranteed to be called before render. I haven't studied React's codebase, but I'm assuming render isn't called on a timeout.

Most likely, the best solution is going to be something along the lines of registering pre-rendering hooks or to mark components that have had their props changed so the render logic calls componentDidReceiveProps before calling render.

@robyoder
Copy link

robyoder commented Nov 28, 2015

+1 please. Having the same issue where I have to pass props around to just about every method in my component class. So ugly.

Nah, I'm good. There are better solutions.

@micky2be
Copy link

micky2be commented Dec 4, 2015

+1

@me-andre
Copy link

componentDidReceiveProps() already exists: it's called render(). However there can be a case (like in @iammerrick 's example) where you need to load/trigger/etc something before you perform a render. This means one and the only one thing: you do something wrong.

When you do things like

setup(id) {
    UserActions.load(id);
}

you introduce statefullness either in the component or (much worse) outside. Why do you ever need to load() data every time a component receives props (they aren't even guaranteed to be new)? If you do lazy-loading, the proper way is to request the data in the render() method:

render() {
    var actions = UserActions.load(id);
    if (actions) // render
    else // show spinner or return null, etc.
}

UserActions.load = function(id) {
    if (data) return data;
    else // request the data, emit change on success
}

@robyoder , passing arguments to functions isn't ugly. It may seem too verbose but it's natural in the programming language you've chosen. Trashing the API by adding a spare lifecycle method just to reduce verbosity is definitely ugly indeed.

@robyoder
Copy link

So tell me, @me-andre, why do we have both componentWillUpdate and componentWillReceiveProps?

In my opinion, it's because they serve different purposes and both are helpful. Just like those two are not the same, render is not the same as the hypothetical componentDidReceiveProps because it's called for reasons other than new props; plus, you aren't given access to the previous props.

The benefit of the previous props is that you wouldn't have to load data every time; it can be conditional based on the props and whether they've changed.

Obviously, "[ugliness] is in the eye of the beholder", because adding a proper lifecycle method seems like a much cleaner solution to me.

@kmalakoff
Copy link

Maybe there is another way to look at this...

Let's say the main goals here is are:

(1) reduce human error - I forgot to pass parameters or use props = props || this.props yet again
(2) to reduce non-value adding boilerplate code - why write something unnecessarily
(3) to reduce the cognitive burden of trying to decide which lifecycle methods to use - why do I need to think about this sort of thing, why do I keep using slightly different approaches depending on how I feel that day, etc

So maybe the solution space is around simplifying of both the use of React and the React API itself to achieve these goals.

If you think about the problem with these goals in mind, maybe some lifecycle methods could be merged and if you are interested in knowing it is initialization vs update, you can tell by the calling signature / arguments. For example: beforeRender(prevProps, prevState) or update(prevProps, prevState).

Personally, it feels like there are too many lifecycle methods and if they were more called consistently (without or with prevProps / prevState on initial pass and update), it could simplify my code and increase my productivity. Plus, the lifecycle method names are pretty long (I tend t copy / paste them around rather than typing them) and it is hard to remember which will / did's exist which makes me think they could / should be simplified.

@me-andre
Copy link

@robyoder , the short answer why we have both componentWillUpdate and componentDidReceiveProps is there is a huge difference between them. They're similar of course, but mainly in being prefixed with componentWill.

state has changed component did not update
componentWillUpdate() yes no
componentWillReceiveProps() no yes

As you may have noticed there are lifecycle points / conditions where one method gets called and the other does not. That's why we have 2 distinct lifecycle methods.

However componentDidReceiveProps() as it was described in this topic does not represent any component state or condition and it doesn't give an access to anything componentWillReceiveProps() does not. It just adds a slight syntactical sugar which may or may not seem useful or easier - this is a personal preference, not a technical requirement. And that is exactly the reason why it shouldn't be added: it's subjective. You say passing arguments is ugly - but you also said "[ugliness] is in the eye of the beholder". For me it seems like you've answered yourself.

@kmalakoff
Copy link

@me-andre you might be responding to me (@kmalakoff) and @robyoder at the same time. Or maybe not, but I'll take this opportunity to move the discussion forward...

What I was raising was that perhaps thinking above the current API with these three above goals could give new insights or perspectives into how one might simplify the API to achieve them. Of course, after going through the exercise, we might end up at the same place.

Let's go through the exercise....

Let's assume this topic is being followed and +1-ed because there is something important to it and let's say the addition of componentDidReceiveProps is not a good idea because it increases the API surface area.

Keeping in mind the table and the 3 goals I put forward, does anyone have any ideas for simplify the API and / or another way to give the people in this thread what they desire and to not expand the API (maybe even reducing / simplifying it)?

@me-andre
Copy link

@kmalakoff , the 3 points you talk about are connected with the API only in that you use the API when you encounter them. They aren't caused by a bad design.

The first problem you talk about is that lifecycle methods take different arguments. Well, that's perfectly natural - they serve different purposes. componentWillReceiveProps takes props not because the moon was full when this method was added - but because it's about receiving props which have not been yet assigned to the component. props are being passed only in methods where they (may) differ from this.props. This is actually a hint, not an issue.
The problem # 3 is that it's hard for you to decide which callback / approach to use. Well, that's not really a problem until the methods I talk above have the same signature. Once you have both state and this.state, props and this.props which are equal (read meaningless) in most methods, then you get entangled in spare options and alternatives.
The problem # 2 is about boilerplate code... well, React is a thin library - and it's beautiful, easy and strict because of that. If you want to make a wrapper which would simplify our lives and reduce the amount of code we write every day - why not do it? Publish it as your own npm that depends on react and you're done.

Regarding "there are too many lifecycle methods" - not yet and won't ever be if you stop requesting for new callbacks for which you don't have a technical need. Technical need is when you implement a complex component and in the middle of work you understand there is absolutely no way of doing it without some ugly hack. It's about "to trigger a tricky DOM method I need something to get called at the time the component does X but there's no such method and I cannot add one, so I use setTimeout and hopefully it wouldn't land earlier than I need". Not when you say "oh, I got tired of writing props = this.props".

And one more thing...
In a well-designed React application, you don't need most of the methods we talk about or you use them very rarely. getInitialState, componentWillMount, componentWillUnmount suffice 99% of the time. In my current project which is a relatively large commercial app, componentWillReceiveProps is used twice throughout all the app. I would not use it at all, but the environment (read browser) is imperfect - manipulating certain "stateful-in-itself" things such as scrollTop or relying on layout computations for future renders requires manual synchronizing between props transitions and DOM changes. However in most "normal" cases, you just don't need to know when props transition happens.

@kmalakoff
Copy link

@me-andre based on my experience working with React, I believe there is room to improve / simplify the API. I'm not alone which is why this issue was raised in the first place and is being +1-ed.

If I answer by providing feedback on your analysis, I don't think it will really move this forward (which is why I have avoided doing it so far) since it is a bit biased towards the status quo and justifying the current API, but I'd be happy to provide feedback on potential solutions! People engaged in this issue are looking to explore improvement ideas and potential API changes to address the issues raised through their experience using React (like I outlined above).

Maybe you are right that the community should do the innovation to move the API forward and maybe have it rolled back into core or maybe the current API is perfect as is, but I think this is a great opportunity to consider what change may look like with the React team.

Maybe the reason you are making arguments as you are is because we are just users of the library with less depth of understanding on the currently API and decisions that went into it. Maybe you could pull in some other React team members who is equally experienced and knowledgeable as yourself to see if we get a different perspective and maybe even come up with a great solution or to get consensus that this is as good as it gets?

@me-andre
Copy link

@kmalakoff , imagine React is an engine and 4 wheels. Now every time you want to go somewhere, you build the rest of the car and that eventually starts being annoying. Otherwise you complain about the need to remember low-level engine details and turning front wheels by hands doesn't feel the right way.
What I would recommend is either to get a complete vehicle (a full-fledged framework) or to build the needed components in the way that you can reuse them over time.
What I see in this thread is engine users complaining that the engine has no hydraulic system and no interior lights. What I feel is that we'd get a complete crap if we add these features to where they don't belong.
React is not a framework: it's more a diff-powered rendering engine that exposes a powerful component system. It has low level and minimalistic API yet powerful enough to enable you to build literally anything on top of it.
Please don't hesitate contacting me via email if you feel like I need to clarify something - I don't want this thread to become a tutorial.

@kmalakoff
Copy link

@me-andre I believe we understand your position and your line of argument well now. Thank you! You may be right that the API is already as good as it will get, but there is also the possibility that it can be improved.

Can someone make a case for changing the API? eg. providing comparative analysis of various options (add componentDidReceiveProps vs merge / simplify APIs vs no change)

@jimfb
Copy link
Contributor

jimfb commented Dec 14, 2015

The React team has been watching the issue, and we do discuss it internally. In principal, I always lean toward eliminating API surface area, so I find myself leaning toward @me-andre's arguments. However, our team sometimes affectionately refers to componentDidReceiveProps as "the missing lifecycle method", and there are serious discussions about potentially adding it. The important thing is that we enable best practices without catalyzing bad practices.

What would be most useful for us (or at least me) is to have a solid understanding of WHY people want this lifecycle method. What are you trying to do in this lifecycle that isn't sufficiently covered by the other lifecycle methods? Why would someone want to do UserActions.load(id); within componentDidReceiveProps instead of within render as suggested in #3279 (comment) (I can think of a reason, but I'm curious what your reasons are)? Are there any use cases other than initiating a data-load based on props?

@kmalakoff
Copy link

@jimfb I've given some thought to this after you suggested I move this to the gist and actually do not believe this line of analysis is off topic...the reason I am interested in this issue around received props is that I believe having a received props lifecycle method will allow me to reduce boilerplate.

Please hear me...reducing boilerplate is the problem that the lifecycle method in this issue could solve for me.

I believe that it also could be a significant part of the reason why people are interested in this lifecycle method because the current API encourages boilerplate more than we would like.

That said, I'd be happy to try to decouple the API improvements and for example, only focus on exploring the solution space around improving the API related to props in this issue.

@jimfb
Copy link
Contributor

jimfb commented Dec 22, 2015

@kmalakoff The topic of the thread is defined by the title and first post. The title is componentDidReceiveProps Please not lots of general ways to reduce boilerplate. This thread is about reducing boilerplate specifically through the introduction of a very specific lifecycle method. That discussion alone is already nuanced enough, without introducing additional topics (like renaming all the functions) that are already being discussed in other issues.

Your points are valuable, and I encourage you to have the discussion, just please move it to a different thread, because we want to keep github threads focused on the topic at hand. Otherwise the discussions are too difficult to track/manage. Threads do exist for most of the topics you raised.

@kmalakoff
Copy link

@jimfb why are people asking for componentDidReceiveProps? The reason I am interested in it is to reduce boilerplate related to props. I'm not sure how state this more clearly so I can be heard and understood.

You have asked for the problems and use cases, and I've stated this as the problem for me and shown a use case above (responding to @shea256) which seem on-topic, within scope and, from my perspective, important for resolving this issue.

I have agreed to hold off on expanding the scope to general API issues / improvements and will. I promise! 😉

FYI: as I stated previously, the reason I started on the thought experiment around this issue was because the arguments seemed a little too narrowly focussed (eg. we can already do everything we need with the current API) rather identifying the underlying motivations behind the issue. As you know, sometimes you need to step back, then focus in, then view from a different angle, then revisit assumptions, investigate the underlying reasons behind the request, etc to iterate towards the best solution to a problem. Hopefully, I have helped draw attention to this and my participation will help get this issue resolved to our satisfaction! (of course, I'm also hoping the React API will evolve to be more use-friendly and will continue that discussion elsewhere...thank you for the links)

@me-andre
Copy link

@kmalakoff , ok, to reduce boilerplate you just make reusable classes or components from which you subclass or compose another components. Nobody prevents you from defining a common method for all components in your app. React does provide excellent extendability: you can define components as objects, functions or classes. You can extend components by subclassing, mixins or factories.

@kmalakoff
Copy link

@me-andre that absolutely is an option if this issue does not lead to an API improvement.

This issue was raised to promote discussion around a desire to make a change to the API to improve it. Exploring alternatives in client code definitively should be considered, but if a strong case is made that the API should be changed, these sort of workarounds would be unnecessary. In order to counter argue, cases also need to be made for what an improved API should be and then evaluated against the current API to assess them.

For example, if you were to demonstrate that the current API without defining a custom superclass could be used in a way to reduce boilerplate (eg. we are using the API incorrectly, or there are features in the current API that can be used to achieve a similar level of reduction of boilerplate, etc) or prove why there is no improvement possible (eg. no general solutions are exist because there is a major use case that could not be supported in our API improvement options), etc, you could make a case that the API is good enough as is.

If the API requires people to write custom superclasses for basic and common control flow patterns, it strengthens the argument that there is a case for the API to be improved.

@shea256
Copy link

shea256 commented Dec 23, 2015

Curious - what is the rationale for not having componentWillReceiveProps triggered before mounting? If you think about it, the component is really receiving props when it is initialized. It's just not receiving new props.

What are the use-cases where you would only want to trigger something upon receiving new props (and not on the initial prop reception)?

I could be missing something obvious here but just trying to reconcile the viewpoints of the various parties.

In any case, if componentWillReceiveNewProps were important in some cases, one could still simulate it with a modified componentWillReceiveProps by performing an check on the props coming in.

@kmalakoff if componentWillReceiveProps triggered the first time, would that meet your standards for API simplification?

@akinnee-gl
Copy link

componentWillReceiveProps not being triggered on mount is not the reason people are asking for componentDidReceiveProps. People are asking for componentDidReceiveProps because they wrote all of their methods to access this.props. When componentWillReceiveProps is called, the nextProps are passed, but this.props has not changed yet, meaning we have to pass nextProps into all of the methods that are called in response to componentWillReceiveProps, instead of leaving them as-is. We end up with a ton of props = props || this.props and a ton of passing props into every function we call.

@kmalakoff
Copy link

@shea256 good points. Having different code paths for initialization vs changes is one of the root causes of prop boilerplate. The other root cause is having with different signatures for handling props like @akinnee-gl points out.

This is why I'm trying to expand the solution space being considered (eg. also call on init) since there could actually be an even better solution to reduce prop boilerplate more.

I hope we will be able to get further:

Before New Hook

componentWillMount() {
  this.setup(this.props.id);
}

componentWillReceiveProps(next) {
  this.setup(next.id);
}

setup(id) {
  UserActions.load(id);
}

After New Hook (Revised)

componentDidReceiveProps(prevProps) {
  UserActions.load(this.props.id);
}

or if UserActions.load cannot check the currently loaded id:

componentDidReceiveProps(prevProps) {
  if (!prevProps || (prevProps.id !== this.props.id))
    UserActions.load(this.props.id);
}

@me-andre
Copy link

@kmalakoff , what I'm talking about is that API improvement is absolutely available for you right now: you can make your own factory of components and then share with us (along with use case samples). This will make your proposal and rationale hundred times more clear. Since all lifecycle points already have appropriate callbacks, you can introduce any new method at any lifecycle point / component state. You can even mixin an event emitter into your component and make it possible to attach several handlers for a state change.

@shea256 , one possible reason for componentWillReceiveProps not to get triggered before first render is that there's no such thing as this.props at that time. What you commonly do in componentWillReceiveProps is comparing this.props[propName] with newProps[propName]. Having the method triggered before 1st render would make you also have to check for presence of this.props. Moreover, the whole component is completely uninitialized at the time it receives props before the initial render, it doesn't even have state.

@kmalakoff , I've twice posted code samples which show how to organize a React component in a way that it doesn't need any setup or similar hacks. Could you please tell why do you still strive to change the behavior of a React component instead of adjusting your code so that it integrates with React? It would be great if you just point at where my samples are inappropriate for your use-case.

@akinnee-gl , the reason not to introduce a new method for just accessing this.props on update is that we have such method - it's called render(). It even gets called after a check for shouldComponentUpdate() - which is normally a place where you do this.props !== newProps or _.isEqual(this.props, newProps) etc.
If you feel like you should have a separate method for some setup, why don't just subclass a React component and define a following render() method

this.setup(this.props);
return this._render();

I just don't see how it simplifies things in the React ecosystem, but that is what you keep requesting.

@kmalakoff
Copy link

@me-andre the premise of this issue is not that we cannot achieve what we want with the current API nor that we cannot work around the current API in client code. The premise of this issue is that the current React API is sub-optimal and needs to be improved; for example, if you were to come up with principles of what an optimal API would look (like I tried to above), the current React API would score in a mid-level range because it is suboptimal / deficient in a number of areas.

Unfortunately, providing ways in client code to work around the React API does not address the root causes of the problem, shifts the debate away from addressing the underlying the issues, and will not lead to a debate around potential solutions to improve the React API.

In short, I already have workarounds that are working for me because I have a bunch of React apps in production so best practices could be great for a blog post, but using them as a line of debate in this issue will just sidetrack us from a real debate on the true purpose of this issue: how to improve the React API (in this issue, limited to prop use cases and boilerplate).

React 1.0 should aim for the top, not the middle. A major version increase can break backwards compatibility so let's go for the best possible API based what we've learned from all of these years of using the 0.x version.

(FYI: I think people might not be engaging with your examples as much as you are hoping because they are not coming here to get taught about the current API, but because they are looking for / hoping for improvements to the API, eg. they could be perceived as with good intentions, but being slightly misaligned)

@me-andre
Copy link

I think people might not be engaging with your examples as much as you are hoping because they are not coming here to get taught about the current API, but because they are looking for / hoping for improvements to the API

Ok, you come with an API improvement proposal, but then you show the code which is way too far from "React best practices". Some things seem very much like worst practices. Then you tell: "that is the reason for a change". Yes, that is the reason for a change but not in the React codebase.
I show you how to reorganize the code to make it work well with React but just ignore proper usage showcase and keep insisting. That doesn't seem to be a constructive debate.

Unfortunately, providing ways in client code to work around the React API does not address the root causes of the problem

When you wrap a low-level API with a higher level API that is not a workaround, but a common practice. Many brilliant frameworks follow that idea.
What I keep telling is wrap the existing ugly API with what you would like to use and share with us. Along with the examples of use and explanations why it got better it would be incredible.
Again, I just don't see a reason why don't you just do that. Because if what you're talking about is a common problem, that would be a great help for many people.

I have a bunch of React apps in production so best practices could be great for a blog post, but using them as a line of debate in this issue will just sidetrack us from a real debate

What you commonly do when you architect/design an API is predicting problems, hypothesizing about use-cases etc. The reason why people hypothesize is not that they try to abstract themselves from a real world in search of the ideal. It's just lack of experience - you can't get experience "in advance".

You tell you have that experience, you've seen real problems and you have some workarounds you can share. That is exactly what can turn this debate into being "real" and "effective". The React itself was built on real problems and challenges - that's why it solves real architecture problems and not just "how to write hello world in 3 lines".

@kmalakoff
Copy link

@me-andre I hear you and your line of argument is clear.

You are correct that central to my argument is that if we establish better principles based on our collective experiences of using the current React API and open up debate to include non-dogmatic solutions that may change the API in some fundamental ways, we can and should seize the opportunity to improve the React API to make it even better than it is today. Let's not rest on our laurels when there is room to improve!

How would you rate the current React API around props? (let's say using letter grades: F to A+), why, and what would you do to improve it if it is less than an A+?

@kmalakoff
Copy link

@me-andre have you had a chance to prepare your ranking and analysis? I'm interested in hearing what you believe are the strengths, weaknesses, and opportunities with the current API.

@ghost
Copy link

ghost commented Jan 11, 2016

+1

@gilangmugnirespaty
Copy link

+1 Please

@AlexCppns
Copy link

Is there a workaround? I need ComponentDidReceiveProps

@iammerrick
Copy link
Contributor Author

I made this issue over a year ago and have been using React daily since. I no longer think there is a use case for componentDidReceiveProps that justifies increasing the API.

@AlexCppns what is it you are trying to do?

@AlexCppns
Copy link

@iammerrick, actually it's alright, I just misunderstood how componentWillReceiveProps is used.

@aweary
Copy link
Contributor

aweary commented Sep 29, 2016

Related discussion: #7678

@brigand
Copy link
Contributor

brigand commented Oct 1, 2016

I've ran into this a few times, and what I ended up doing was:

componentWillReceiveProps(nextProps) {
  if (this.props.foo !== nextProps.foo) this.needsUpdate = true;
}
componentDidUpdate() {
  if (this.needsUpdate) {
    this.needsUpdate = false;
    // update the dom
  }
}

It's not too bad.

@me-andre
Copy link

me-andre commented Oct 2, 2016

@brigand , there's no need for a flag - you can compare props within componentDidUpdate():

componentDidUpdate(prevProps) {
    let needsUpdate = prevProps.foo !== this.props.foo;
    // ...whatever
}

Furthermore, you solution could be easily broken by shouldComponentUpdate(), which may prevent re-rendering.

@brigand
Copy link
Contributor

brigand commented Oct 2, 2016

Oh cool, thanks!

@chanakasan
Copy link

@jimfb I think I have a synchronous use case below. I think a componetDidReceiveProps would've been perfect for this.

  componentDidMount() {
    this._selectAll()
  }

  componentDidUpdate(prevProps) {
    let shouldUpdateSelected = (prevProps.recordTypeFilter !== this.props.recordTypeFilter) ||
      (prevProps.statusFilter !== this.props.statusFilter) ||
      (prevProps.list !== this.props.list)

    if (shouldUpdateSelected) { this._selectAll() }
  }

  _selectAll() {
    this.setState({ selectedIds: this._getFilteredOrders().map((order) => ( order.id )) })
  }

  _getFilteredOrders() {
    let filteredOrders = this.props.list

    // recordTypeFilter
    let recordTypeFilter = this.props.recordTypeFilter
    filteredOrders = _.filter(filteredOrders, (order) => {
		// somelogic
    })

    // statusFilter
    let statusFilter = this.props.statusFilter
    filteredOrders = _.filter(filteredOrders, (order) => {
		// somelogic
    })

    return filteredOrders
  }

@me-andre
Copy link

@chanakasan , your example lacks render() method which is essential for both understanding your example and suggesting a better solution.
Second, your code is connected to some custom business logic and isn't easy to read. Don't hesitate to explain, not just throw a copy-paste into others.
I've read your example however, and would like to share following thoughts:

  1. This is the use case of componentWillReceiveProps, not componentDidUpdate. If you switch to componentWillReceiveProps, your component would re-render twice as less while preserving the same logic. After a year since I left this discussion I still see zero use case for componentDidUpdate except for reacting to DOM changes.
  2. Much better, however would be avoiding hooks at all and moving all logic to the render() method / migrating to a stateless component, because this.state.selectedIds isn't actually a state. It's purely computed from props.

@chanakasan
Copy link

Hi @me-andre, Thanks for taking the time to reply. I've read the discussion on this page and I like to thank you for participating in it.

Yes, a componentDidReceiveProps is not needed, but things seems mysterious without it.
You've said your component would re-render twice as less if I use componentWillReceiveProps. That's still a mystery to me.

I did think and try the two things you've suggested before, but failed.

  1. componentWillReceiveProps won't work because the _getFilteredOrders() function called from _selectAll() needs the newProps.
  2. I couldn't think of a way to derive the selectedIds without storing it in state.

I just created a complete example from my use case. I've made it easy to understand much as possible.

If you could please have a look and point me in the right direction. If I am able to understand this properly I'll share this example in a blog post to help others as well.

@me-andre
Copy link

me-andre commented Nov 28, 2016

@chanakasan, come on, this looks like a production code. I'm not gonna read all of it and help you with your project for free.

However I can point you to the right direction:

  1. Both componentWillReceiveProps() and componentDidUpdate() can access prev and next props. That's clearly seen from the official React docs.
  2. From the complete copypaste of your code it's now obvious that you use state for storing selected ids which a user can toggle. It's fine to use state then, but still componentWillReceiveProps() would trigger re-render twice as less. (because a render is gonna happen after componentWillReceiveProps() anyway, setState() would just update the state for the upcoming render).
  3. Please take a look at the docs. Save your time and respect others.

@chanakasan
Copy link

@me-andre I think I understand your point about componentWillReceiveProps using this line in the docs:

componentWillReceiveProps() is not invoked if you just call this.setState()

But the caveat of using componentWillReceiveProps is I'll have to pass along nextProps to functions.

I'll try to follow your advice. Thanks I appreciate it.

Btw, it wasn't my intention for you to help with my production project for free :). It's a more full example from my previous short example to fill in any blanks about my use case.

@nurtureJamesTan
Copy link

nurtureJamesTan commented Jan 12, 2018

what if we use this in conjunction with shouldComponentUpdate?
where by we do not wish to update component state or rerender,
but we need this.props to be using latest props to do some manual script work after props is received

my work around is to set this.props to the new props before running the desired function

@danvc
Copy link

danvc commented Jul 26, 2020

It really would be nice to have the ComponentDidMount or some hook for that. Why? Because, some times, we need to do other calculations which doesn't depend on React lifecycle.

For example: I have a parent component which may be resized. The child component is responsible to render an OpenLayer map which has a function that is responsible to resize the map. However, it should happen after the child get props from the parent and also committed other calculations within React lifecycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests