16.4 blog post #904
16.4 blog post #904
Conversation
|
Deploy preview for reactjs ready! Built with commit dc0ed4b |
| @@ -0,0 +1,128 @@ | |||
| --- | |||
There was a problem hiding this comment.
Don't forget to change the date in the filename
I think we decided against this for now. We'll just mention it in the CHANGELOG (which Dan has already done). |
| const prevProps = state.prevProps; | ||
| // Compare incoming props to previous props | ||
| const controlledValue = | ||
| prevProps.value !== state.controlledValue |
There was a problem hiding this comment.
This doesn't look quite right. Wouldn't this block every setState update?
| // Store the previous props in state | ||
| prevProps: props, | ||
| controlledValue, | ||
| }; |
There was a problem hiding this comment.
I think we should avoid this example. Even aside from gDSFP, this approach of half-controlled/half-uncontrolled leads to an awkward/ambiguous API.
For example, the only way to reset a half-controlled component's state to a value safely is to render twice, temporarily setting the prop to something else (e.g. null) and then setting it back.
I think instead, we should encourage fully controlled (no state) or fully uncontrolled (props.defaultValue sets initial state and never gets synced).
Maybe put another way, I think we may want to avoid a component ever updating a state attribute via setState that props also updates.
There was a problem hiding this comment.
Maybe it's okay we show this, since we want to cover a before and after case. I don't know. In general, I'm just concerned about any perceived encouragement of this API/design. 😄
|
|
||
| ```js | ||
| static getDerivedStateFromProps(props, state) { | ||
| if (props.value !== state.controlledValue) { |
There was a problem hiding this comment.
It might be worth mentioning that this pattern was flawed to begin with- even setting the recent gDSFP change aside- because it would override state even when unrelated props changed.
There was a problem hiding this comment.
I don't know if that would be more or less confusing. It might distract from the thing you're trying to emphasize. I don't know.
| controlledValue, | ||
| }; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Let's just mention here that most components don't need this pattern. We're only documenting for the cases in which you're already using it. But if you just need some value from props, you should just read it from props, and not attempt to put it into state.
There was a problem hiding this comment.
For example:
Note that typically the vast majority of the components don't need
getDerivedStateFromPropsat all. It shouldn't be used often, as making state dependent on props often leads to logic that is hard to understand. If possible, prefer to have one source of truth for any value (either in props or in state), and lift state up if it's necessary to share it between multiple components.
There was a problem hiding this comment.
Ok, but I have mixed feelings about telling people not to use a feature, especially one we just added, unless we give them a clear alternatives. We need to follow up with a more comprehensive post soon (like this week or next).
This PR adds a section about the state of Pointer Events in React. This should be merged only if facebook/react#12507 is accepted as well.
acdlite
force-pushed
the
16.4-release-post
branch
from
8c1a8ae to
55b2511
Compare
| static getDerivedStateFromProps(props, state) { | ||
| if (props.value !== state.controlledValue) { | ||
| return { | ||
| // Since this method fires on both props and state changes, local updates |
There was a problem hiding this comment.
Super tiny nit. Please feel free to ignore.
Suggestion: Change "Since this method fires" to "Because this method fires"
(so there aren't two "since"s in the sentence)
((no idea if this is a grammatical no-no))
I'll add it back once CodeSandbox starts resolving `latest` to 16.4.
…md into Chinese (reactjs#904) Co-authored-by: hexiaokang <[email protected]> Co-authored-by: KnowsCount <[email protected]>
TODO:
Section aboutNever mind.React.unstable_Profiler(@bvaughn)Explain why async rendering isn't in this release (?)Nah, don't need this either.getDerivedStateFromPropsdocs to reflect new behavior