Comments (23)
I don't get it. I want to compute some state from the props, but unless I know what the current props are, I'm not sure how I can avoid performing a potentially expensive operation.
e.g. I have some data
coming in as a prop (an array with many elements). I want to to normalize this data and then put it in the state.
getDerivedStateFromProps
sounds like it would be perfect for this, however, if some other prop changes, I really don't want to run this normalization step again because my data
hasn't changed.
Previously I could check if(this.props.data === nextProps.data)
in componentWillReceiveProps
but now I can't.
Why do I have to store two copies in my state now? One for the normalized data, and another one for the unnormalized data so that I can check against the unnormalized data in the props This seems like a common use-case that used to be easy and now is hard, and now I have this dangling unormalizedData
state that I don't even care about.
from rfcs.
"Why can't I use prevProps" question already was asked many times. Because there is not previous props then component is initialized.
Couldn't prevProps
just be null
or undefined
in this scenario? Why does there need to be an "additional check"? If developers want to ignore prevProps
they can.
Your example will work AFAICT, it just feels like a downgrade from componentWillReceiveProps
though. prevData
shouldn't even be in the state IMO. Also, we can't defer setting the state with getDerivedStateFromProps
, which means if normalizeData
is async, we're out of luck, not even reselect will save us. With componentWillReceiveProps
we could call this.setState
whenever we were ready.
I guess the only choice is to use componentDidUpdate
then? Seems strange to me to have to wait until after the component is rendered before I'm allowed to to start processing the new props.
e.g. This feels simpler to me but it has a silly caveat:
class MyComponent extends React.Component {
state = {
normalizedData: {}
}
async componentDidUpdate(prevProps, prevState, snapshot) {
if (this.props.data !== prevProps.data) {
this.setState({loading: true}); // <-- will immediately cause another render after we *just* updated
const normalizedData = await normalizedData(this.props.data);
this.setState({loading: false, normalizedData});
}
}
}
from rfcs.
It's better to discuss the "why no prevProps
param" issue in #40- but I think there's a couple of misunderstandings in the above thread that are worth addressing separate from that argument.
From comment:
I used to store things that don't affect rendering under
this.
too, but sincegetDerivedStateFromProps
is static, we can't really do that anymore either.
Assuming these things are derived from (or otherwise related to) props
- storing them on the class instance is usually not safe. This is the main reason why getDerivedStateFromProps
is static.
This section of the React docs explains the difference between "render" and "commit" phases as does this lovely diagram. Computing and storing values on the instance from a render phase method (like componentWillReceiveProps
) is usually unsafe because React may invoke render phase lifecycles without committing at all (because of an error or a higher priority interruption). In this case, your instance values could be based off of the wrong props. React manages this.props
and this.state
for you though, and always resets them to the correct values before calling your component lifecycles.
From comment:
e.g. This feels simpler to me but it has a silly caveat:
Sync rendering behavior for setState
calls made from componentDidMount
/componentDidUpdate
is actually a very important feature for libraries that need to do things like measure or position an element after rendering.
from rfcs.
// nextProps.keys = [1, 1, 2, 3] will never happen
// nextProps.keys = [1, 2, 3, 1, 4]; is valid and this.cache = [0, 1, 3, 6 ,7 ,11]
I don't understand this.
this.state.index = fn1(this.cache);
You should not directly mutate state
like this. You should use setState
. (I assume this may have been a typo and you may have meant to write nextState.index = ...
)
And you suggest that I should move this.cache to this.state. Am I correct?
Maybe.
Since you're mutating cache
, it would actually be safer to move what you're currently doing into componentDidMount
/componentDidUpdate
. If you were to move it to getDerivedStateFromProps
(or leave it inside of componentWillReceiveProps
) it's possible that you will mutate the cache unnecessarily. (This is explained in more detail in RFC 6 as well as my comment here.)
If you need to update cache before rendering, then a safer approach would be something like this:
class extends React.Component {
state = {
cache: [],
index: 0,
prevKeys: [],
};
static getDerivedStateFromProps(nextProps, prevState) {
if (nextProps.keys !== prevState.prevKeys) {
// Figure out 'i' by comparing nextProps.keys to prevState.prevKeys
let i = 0;
while (i < nextProps.keys.length && prevState.prevKeys.keys[i] === nextProps.keys[i]) {
i++;
}
const cache = this.fn0(nextProps.keys, i);
const index = this.fn1(cache);
// Update state
return {
cache,
index,
prevKeys: nextProps.keys
}
}
// Nothing to update in state
return null;
}
fn0(keys, max) {
// Don't mutate cache
const cache = this.cache.slice(0);
for (let i = 0; i <= max; i++) {
cache.push(cache[cache.length - 1] + keys[i]);
}
return cache;
}
}
That being understand, I don't really understand what the partial code snippet you shared is doing so I'm just taking a guess.
from rfcs.
As for the answer 1, nextProps
and prevProps
will become redundant. I want to know whether it is a valid statement? If so, I hope you guys consider to remove it in the future.
As for the answer 2, I am not directly mutating an array from props
. Here is an example
class extends React.Component {
constructor(props) { // props.keys = [1, 2, 3]
super(props);
this.cache = [0];
this.fn0(props.keys); // this.cache = [0, 1, 3, 6]
this.state = {};
this.state.index = 0;
}
fn0(keys) {
for (let i = 0; i < keys.length; i++) {
this.cache.push(this.cache[this.cache.length - 1] + keys[i]); // this.cache[n] = this.cache[n - 1] + keys[i]
}
}
componentWillReceiveProps(nextProps) {
// this.props.keys will keep growing and existing elements will never change
// nextProps.keys = [1, 1, 2, 3] will never happen
// nextProps.keys = [1, 2, 3, 1, 4]; is valid and this.cache = [0, 1, 3, 6 ,7 ,11]
let i = 0;
while (i < nextProps.keys.length && this.props.keys[i] === nextProps.keys[i]) {
i++;
}
fn0(nextProps.keys.slice(0, i));
const nextState = Object.assign({}, this.state);
this.state.index = fn1(this.cache);
this.setState(nextState);
}
}
And you suggest that I should move this.cache
to this.state
. Am I correct?
Anyway, this is what I have observed you guys try to force us to get rid of all data members and move them to the state with that small static method.
from rfcs.
If the goal is not necessarily handling state but wanting to reduce the performance cost of an expensive one-way computation ny not executing it when data has not changed. Perhaps rather than using getDerivedStateFromProps a better fit for this use case is something like reselect's memoized selectors in render.
from rfcs.
@wood1986 current props cannot be accessed in getDerivedStateFromProps
lifecycle. It's pure function. You just need to store new prop in state.
You can read this thread for more information.
from rfcs.
@TrySound this sounds odd because inside the component there will be two versions of props
state.props
and this.props
Yes, as long as you know what you are doing, this is fine. But I do not feel well with two versions of props
from rfcs.
@wood1986 You have two version of props when you use componentWillReceiveProps
and componentDidUpdate
. It's not different, except you save that data explicitly by yourself.
And what is wrong for you to keep previous props in state? Do you think it may introduce performance regression or something else?
from rfcs.
I think my issue is not about the performance. It's main about the code migration from 16 to 17 and the code consistency.
In the Basic example, the comment inside gDSFP
says
Called after a component is instantiated or before it receives new props.
Regardless of *context
, does it means starting from 17
shouldComponentUpdate(nextProps, nextState) -> shouldComponentUpdate(nextState)
componentDidUpdate(prevProps, prevState) -> componentDidUpdate(prevState)
?
If so, I am totally fine. If no, then this is odd part because shouldComponentUpdate
and componentDidUpdate
still hold nextProps
and prevProps
. And this is the inconsistent part.
from rfcs.
@wood1986 The problem with prevProps
in gDSFP is that is does not exists on first call. So you need to extra check if it's not null and one of it's prop is not changed. This is overhead. State solves that problem via initial value.
from rfcs.
I get this part. If this.state
will have props
information via the implementation ofgetDerivedStateFromProps
, what is the point of having nextProps
and prevProps
in shouldComponentUpdate
and componentDidUpdate
respectively?
from rfcs.
They just wasn't touched yet.
from rfcs.
Hey @bvaughn,
-
Could you tell us that if we have to put the
props
intostate
viagetDerivedStateFromProps
, do you need the argumentnextProps
andprevProps
inshouldComponentUpdate
andcomponentDidUpdate
? -
How can I update the data member inside the
getDerivedStateFromProps
before giving the next state which depends on updated data member?
Currently I have a scenario that I need to re-compute some elements of an array which is the data member of theReact.Component
onnextProps
is received incomponentWillReceiveProps
. The next state depends on the result of this array.this.state.index
is for accessing the result of this array. I do not want to put the whole array into the state and this array cannot be static.
from rfcs.
- Could you tell us that if we have to put the
props
intostate
viagetDerivedStateFromProps
, do you need the argumentnextProps
andprevProps
inshouldComponentUpdate
andcomponentDidUpdate
?
We have no current plans to change the signature for methods like shouldComponentUpdate
and componentDidUpdate
with regard to next/prev props.
- How can I update the data member inside the
getDerivedStateFromProps
before giving the next state which depends on updated data member? ...
I don't really understand your question. Can you provide an example?
What inputs (in your current componentWillReceiveProps
) determine whether you "re-compute" elements or not. Can you just mirror those inputs in prevState
so that you can access them in getDerivedStateFromProps
(as shown here)?
It sounds like you may be mutating an array (or objects within an array) that's passed in via props
. This is not particularly safe. Generally we would suggest you store a filtered or mapped version of that array in your component's state
and work with it.
from rfcs.
Yes you are right. It was a typo. It should be nextState.index = this.fn1(this.cache)
Anyway, thanks for your answer! I need to prepare for that change. I hope I can share the real code with you in the future because it is related to “react-virtualized”.
from rfcs.
You are welcome. Good luck!
from rfcs.
Hey @mnpenner, I am not giving you an advice. I just want to share my experience and I am not saying they make a good or bad move.
In my scenario, props.data
is a simple int array and is used to compute a lookup table.
Before 16.3
, I stored this lookup table in this
instead of this.state
. The reason of putting it in this
was any changes to this lookup did not need to trigger a rendering immediately. I had a theory was any parameters affecting the rendering should be in this.state
. Here is the sample and this.lookup
also has contained the props.data
.
this.props = [1, 2, 4];
this.lookup = [
{ "key": 1, "value": 1},
{ "key": 2, "value": 3},
{ "key": 4, "value": 7}
]
As I am doing the 16.3
migration for my lib, I have to move this.lookup
table to this.state
in order to have a similar 16.2
comparison between prevProps
and nextProps
. Luckily, this.lookup
has contained the props
information and my comparison has not been a simple reference check at the very beginning (before 16.2
). So I still survive.
I guess your normalization process from this.props.data
to normalizedData
is irreversible (my scenario is reversible). This means if you only have normalizedData
, you cannot get the original this.props.data
. If it is reversible, you should be able to do the comparison without introducing the this.state.unormalizedData
. But the downside is you need to have some special logic for the comparison.
from rfcs.
@wood1986 Even if I could denormalize it, it would be nearly as expensive as the normalization algorithm. There would be no point, I'd just normalize every time.
I used to store things that don't affect rendering under this.
too, but since getDerivedStateFromProps
is static, we can't really do that anymore either.
from rfcs.
@mnpenner You don't have to store two copies of your state. You just store references for the next comparing. And it's do not introduce to you any cost except probably more characters for it. For me this api is much better than any other because it's explicit and allows to type code well.
"Why can't I use prevProps" question already was asked many times. Because there is not previous props then component is initialized. Passing any value will require additional check which makes code less readable. With state you have defaults in initial state which you may compare with new props. So the problem is solved.
There is not any special code here. It's dead simple I'd say.
class MyComponent extends React.Component {
state = {
prevData: [],
normalizedData: {}
}
static getDerivedStateFromProps(props, state) {
let state = null
if (state.prevData !== props.data) {
state = {
...state,
prevData: props.data,
normalizedData: normalizeData(props.data)
}
}
return null
}
}
from rfcs.
@mnpenner If normalized data is async then you should use componentDidMount
and componentDidUpdate
(you have nextProps
and prevProps
here), which are safe place for side effects in opposite to componentWillReceiveProps
.
You will always have to do null check. You do not get anything helpful here, just more arguments with harder types and runtime check which may be omitted with state.
class MyComponent extends React.Component {
static getDerivedStateFromProps(props, prevState, prevProps) {
let state = null
if (!prevProps || prevProps.data !== props.data) {
state = {
...state,
normalizedData: normalizeData(props.data)
}
}
return null
}
}
from rfcs.
Just an FYI, somebody opened an RFC to add prevProps
to the getDerivedStateFromProps
API over at #40.
They're looking for real use cases to justify adding it, so perhaps you can share over there if interested.
from rfcs.
I am sorry if I have overlooked this, but to summarize, reading through this and linked comments, what I understand is that main reason for not having the current props as the third argument is that it will be a potential memory consumption/leak (and prevProps may even be included from other lifecycle methods for some reason, not sure why though). Is there any other reason?
from rfcs.
Related Issues (20)
- onScroll event propagation HOT 5
- [Feature Request]: distinguish "what" and "when" dependencies in useEffect HOT 2
- [Feature Request] Return ref rather than forwardRef HOT 1
- the ability to check if something is function or class or an arrow function HOT 5
- npx-create-react-app creating a folder tempnodejsnpm HOT 2
- [Feature Request] Can we do some Static Analysis for diff with babel ? HOT 6
- Improving the RFC workflow process HOT 18
- useIf: Conditional hooks HOT 6
- Is useReducer an overengineering? HOT 10
- Improve profiling react applications HOT 1
- Introduce GUI tooling to speed up web application development HOT 1
- [React Server Components] Idea to simplify overall design HOT 11
- psql: could not connect to server: No such file or directory HOT 1
- [Question] The new JSX transform HOT 1
- [Feature Request]: Add array of updated deps indices to `useEffect` hooks arg HOT 1
- [Feature Request]: React Hooks `Equality` **AKA:** `[isEqual]` Callback HOT 3
- Functional Attribute/Prop Node HOT 2
- [Feature Request][eslint-plugin-react-hooks] no-ref-checks, display error when using useRef's return value as condition HOT 1
- [Feature Request]: useStateRef HOT 5
- Typo: 'exiting' might be 'existing' HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rfcs.