From 9818f57bfa286b9a21861b09ceec4ff7f668a88a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 14:06:52 -0800 Subject: [PATCH 01/22] Initial proposal for async-friendly, static lifecycle hooks --- text/0000-static-lifecycle-methods.md | 256 ++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 text/0000-static-lifecycle-methods.md diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md new file mode 100644 index 00000000..3059e019 --- /dev/null +++ b/text/0000-static-lifecycle-methods.md @@ -0,0 +1,256 @@ +- Start Date: 2017-12-08 +- RFC PR: (leave this empty) +- React Issue: (leave this empty) + +# Summary + +Replace error-prone render phase lifecycle hooks with static methods to make it easier to write async-compatible React components. + +Provide a clear migration path for legacy components to become async-ready. + +# Basic example + +## Current API + +The following example combines several patterns that I think are common in React components: + +```js +class ExampleComponent extends React.Component { + constructor(props) { + super(props); + + this.state = { + derivedData: null, + externalData: null, + someStatefulValue: null + }; + } + + componentWillMount() { + asyncLoadData(this.props.someId).then(externalData => + this.setState({ externalData }) + ); + + addExternalEventListeners(); + + computeMemoizeData(nextProps, nextState); + } + + componentWillReceiveProps(nextProps) { + this.setState({ + derivedData: computeDerivedState(nextProps) + }); + } + + componentWillUnmount() { + removeExternalEventListeners(); + } + + componentWillUpdate(nextProps, nextState) { + if (this.props.someId !== nextProps.someId) { + asyncLoadData(nextProps.someId).then(externalData => + this.setState({ externalData }) + ); + } + + if (this.state.someStatefulValue !== nextState.someStatefulValue) { + nextProps.onChange(nextState.someStatefulValue); + } + + computeMemoizeData(nextProps, nextState); + } + + render() { + if (this.state.externalData === null) { + return
Loading...
; + } + + // Render real view... + } +} +``` + +## Proposed API + +This proposal would modify the above component as follows: + +```js +class ExampleComponent extends React.Component { + constructor(props) { + super(props); + + this.state = { + derivedData: null, + externalData: null, + someStatefulValue: null + }; + } + + static deriveStateFromProps(props, state, prevProps) { + // If derived state is expensive to calculate, + // You can compare props to prevProps and conditionally update. + return { + derivedData: computeDerivedState(props) + }; + } + + static prefetch(props, state) { + // Prime the async cache early. + // (Async request won't complete before render anyway.) + // If you only need to pre-fetch before mount, + // You can conditionally fetch based on state. + asyncLoadData(props.someId); + } + + componentDidMount() { + // Event listeners are only safe to add after mount, + // So they won't leak if mount is interrupted or errors. + addExternalEventListeners(); + + // Wait for earlier pre-fetch to complete and update state. + // (This assumes some kind of cache to avoid duplicate requests.) + asyncLoadData(props.someId).then(externalData => + this.setState({ externalData }) + ); + } + + componentDidUpdate(prevProps, prevState) { + // Callbacks (side effects) are only safe after commit. + if (this.state.someStatefulValue !== prevState.someStatefulValue) { + this.state.onChange(this.state.someStatefulValue); + } + } + + componentWillUnmount() { + removeExternalEventListeners(); + } + + render() { + // Memoization that doesn't go in state can be done in render. + // It should be idempotent and have no external side effects though. + computeMemoizeData(this.props, this.state); + + if (this.state.externalData === null) { + return
Loading...
; + } + + // Render real view... + } +} + +``` + +# Motivation + +The React team recently added a feature flag to stress-test Facebook components for potential incompatibilities with our experimental async rendering mode (facebook/react/pull/11587). We enabled this feature flag internally so that we could: +1. Identify common problematic coding patterns with the legacy component API to inform a new async component API. +2. Find and fix async bugs before they impact end-users by intentionally triggering them in a deterministic way. +3. Gain confidence that our existing products could work in async. + +I believe this GK confirmed what we suspected about the legacy component API: _It has too many potential pitfalls to be safely used for async rendering._ + +## Common problems + +Some of the most common problematic patterns that were uncovered include: +* **Initializing Flux stores in `componentWillMount`**. It's often unclear whether this is an actual problem or just a potential one (eg if the store or its dependencies change in the future). Because of this uncertainty, it should be avoided. +* **Adding event listeners/subscriptions** in `componentWillMount` and removing them in `componentWillUnmount`. This causes leaks if the initial render is interrupted (or errors) before completion. +* **Non-idempotent external function calls** during `componentWillMount`, `componentWillUpdate`, or `componentWillReceiveProps` (eg registering callbacks that may be invoked multiple times, initializing or configuring shared controllers in such a way as to trigger invariants, etc.) + +The [example above](#basic-example) attempts to illustrate a few of these patterns. + +## Proposal + +This proposal is intended to reduce the risk of writing async-compatible React components. + +It does this by removing many1 of the potential pitfalls in the current API while retaining important functionality the API enables. I believe this can be accomplished through a combination of: + +1. Choosing lifecycle method names that have a clearer, more limited purpose. +2. Making certain lifecycles static to prevent unsafe access of instance properties. + +1 It is not possible to detect or prevent all side-effects (eg mutations of global/shared objects). + +# Detailed design + +## New static lifecycle methods + +### `static prefetch(props: Props, state: State): void` + +This method is invoked before `render` for both the initial render and all subsequent updates. It is not called during server rendering. + +The purpose of this method is to initiate asynchronous request(s) as early as possible in a component's rendering lifecycle. Such requests will not block `render`. They can be used to pre-prime a cache that is later used in `componentDidMount`/`componentDidUpdate` to trigger a state update. + +Avoid introducing any side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. + +### `static deriveStateFromProps(props: Props, state: State, prevProps: Props): PartialState | null` + +This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. + +Note that React may call this method even if the props have not changed. I calculating derived data is expensive, compare new and previous prop values to conditionally handle changes. + +React does not call this method before the intial render/mount and so it is not called during server rendering. + +## Deprecated lifecycle methods + +### `componentWillMount` + +This method will log a deprecation warning in development mode recommending that users use the new static `prefetch` method instead. It will be removed entirely in version 17. + +### `componentWillUpdate` + +This method will log a deprecation warning in development mode recommending that users use the new static `prefetch` method instead. It will be removed entirely in version 17. + +### `componentWillReceiveProps` + +This method will log a deprecation warning in development mode recommending that users use the new static `deriveStateFromProps` method instead. It will be removed entirely in version 17. + +# Drawbacks + +The current component lifecycle hooks are familiar and used widely. This proposed change will introduce a lot of churn within the ecosystem. I hope that we can reduce the impact of this change through the use of codemods, but it will still require a manual review process and testing. + +This change is **not backwards compatible**. It will require library maintainers to drop support for older versions of React in order to support the future component API. Unfortunately, I believe this is unavoidable in order to safely transition to an async-compatible world. + +# Alternatives + +## Try to detect problems using static analysis + +It is possible to create ESLint rules that attempt to detect and warn about potentially unsafe actions inside of render-phase lifecycle hooks. Such rules would need to be very strict though and would likely result in many false positives. It would also be difficult to ensure that library maintainers correctly used these lint rules, making it possible for async-unsafe components to cause problems within an async tree. + +Sebastian has also discussed the idea of side effect tracking with the Flow team. Conceptually this would enable us to know, statically, whether a method is free of side effects and mutations. This functionality does not currently exist in Flow though, and if it did there will still be an adoption problem. (Not everyone uses Flow and there's no way to gaurantee the shared components you rely on are safe.) + +## Don't support async with the legacy class component API + +We could leave the class component API as-is and instead focus our efforts on a new stateful, functional component API. If a legacy class component is detected within an async tree, we could revert to sync rendering mode. + +There are no advanced proposals for such a stateful, functional component API that I'm aware of however, and the complexity of such a migration would likely be at least as large as this proposal. + +# Adoption strategy + +Begin by reaching out to prominent third-party library maintainers to make sure there are no use-cases we have failed to consider. + +Assuming we move forward with the proposal, release (at least one) minor 16.x update to add deprecation warnings for the legacy lifecycles and inform users to use the new static methods instead. We'll then cordinate with library authors to ensure they have enough time to migrate to the new API in advance of the major release that drops support for the legacy lifecycles. + +We will also provide codemods to assist with the migration, although given the nature of the change, codemods will be insufficient to handle all cases. Manual verification will be required. + +# How we teach this + +Write a blog post (or a series of posts) announcing the new lifecycle hooks and explaining our motivations for the change, as well as the benefits of being async-compatible. Provide examples of how to migrate the most common legacy patterns to the new API. (This can be more detailed versions of the [basic example](#basic-example) shown in the beginning of this RFC.) + +# Unresolved questions + +## Can `shouldComponentUpdate` remain an instance method? + +Anectdotally, it seems far less common for this lifecycle hook to be used in ways that are unsafe for async. The overwhelming common usagee of it seems to be returning a boolean value based on the comparison of current to next props. + +On the one hand, this means the method could be easily codemodded to a static method, but it would be equally easy to write a custom ESLint rule to warn about `this` references to anything other than `this.props` inside of `shouldComponentUpdate`. + +Beyond this, there is some concern that making this method static may complicate inheritance for certain languages/compilers. + +## Can `render` remain an instance method? + +There primary motivation for leaving `render` as an instance method is to allow other instance methods to be used as event handlers and ref callbacks. (It is important for event handlers to be able to call `this.setState`.) We may change the event handling API in the future to be compatible with eg error boundaries, at which point it might be appropriate to revisit this decision. + +Leaving `render` as an instance method also provides a mechanism (other than `state`) on which to store memoized data. + +## Other + +Are there important use cases that I've overlooked that the new static lifecycles would be insufficient to handle? \ No newline at end of file From 839547f8f729a6e8355dbb3c8d7bf7ba78944b51 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 14:32:13 -0800 Subject: [PATCH 02/22] Added a note about the prefix --- text/0000-static-lifecycle-methods.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 3059e019..18afc1aa 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -142,7 +142,7 @@ class ExampleComponent extends React.Component { # Motivation -The React team recently added a feature flag to stress-test Facebook components for potential incompatibilities with our experimental async rendering mode (facebook/react/pull/11587). We enabled this feature flag internally so that we could: +The React team recently added a feature flag to stress-test Facebook components for potential incompatibilities with our experimental async rendering mode ([facebook/react/pull/11587](https://github.com/facebook/react/pull/11587)). We enabled this feature flag internally so that we could: 1. Identify common problematic coding patterns with the legacy component API to inform a new async component API. 2. Find and fix async bugs before they impact end-users by intentionally triggering them in a deterministic way. 3. Gain confidence that our existing products could work in async. @@ -193,21 +193,21 @@ React does not call this method before the intial render/mount and so it is not ### `componentWillMount` -This method will log a deprecation warning in development mode recommending that users use the new static `prefetch` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillMount` or use the new static `prefetch` method instead. It will be removed entirely in version 17. ### `componentWillUpdate` -This method will log a deprecation warning in development mode recommending that users use the new static `prefetch` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillUpdate` or use the new static `prefetch` method instead. It will be removed entirely in version 17. ### `componentWillReceiveProps` -This method will log a deprecation warning in development mode recommending that users use the new static `deriveStateFromProps` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillReceiveProps` or use the new static `deriveStateFromProps` method instead. It will be removed entirely in version 17. # Drawbacks The current component lifecycle hooks are familiar and used widely. This proposed change will introduce a lot of churn within the ecosystem. I hope that we can reduce the impact of this change through the use of codemods, but it will still require a manual review process and testing. -This change is **not backwards compatible**. It will require library maintainers to drop support for older versions of React in order to support the future component API. Unfortunately, I believe this is unavoidable in order to safely transition to an async-compatible world. +This change is **not fully backwards compatible**. Libraries will need to drop support for older versions of React in order to use the new, static API. Unfortunately, I believe this is unavoidable in order to safely transition to an async-compatible world. # Alternatives @@ -227,9 +227,11 @@ There are no advanced proposals for such a stateful, functional component API th Begin by reaching out to prominent third-party library maintainers to make sure there are no use-cases we have failed to consider. -Assuming we move forward with the proposal, release (at least one) minor 16.x update to add deprecation warnings for the legacy lifecycles and inform users to use the new static methods instead. We'll then cordinate with library authors to ensure they have enough time to migrate to the new API in advance of the major release that drops support for the legacy lifecycles. +Assuming we move forward with the proposal, release (at least one) minor 16.x update to add deprecation warnings for the legacy lifecycles and inform users to either rename with the `unsafe_` prefix or use the new static methods instead. We'll then cordinate with library authors to ensure they have enough time to migrate to the new API in advance of the major release that drops support for the legacy lifecycles. -We will also provide codemods to assist with the migration, although given the nature of the change, codemods will be insufficient to handle all cases. Manual verification will be required. +We will provide a codemod to rename the deprecated lifecycle hooks with the new `unsafe_` prefix. + +We will also provide codemods to assist with the migration to static methods, although given the nature of the change, codemods will be insufficient to handle all cases. Manual verification will be required. # How we teach this From b7189ccf42c5f8c039d268841efdfda296eec411 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 14:34:40 -0800 Subject: [PATCH 03/22] Made deprecation/rename clearer --- text/0000-static-lifecycle-methods.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 18afc1aa..856323a8 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -191,15 +191,15 @@ React does not call this method before the intial render/mount and so it is not ## Deprecated lifecycle methods -### `componentWillMount` +### `componentWillMount` -> `unsafe_componentWillMount` This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillMount` or use the new static `prefetch` method instead. It will be removed entirely in version 17. -### `componentWillUpdate` +### `componentWillUpdate` -> `unsafe_componentWillUpdate` This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillUpdate` or use the new static `prefetch` method instead. It will be removed entirely in version 17. -### `componentWillReceiveProps` +### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillReceiveProps` or use the new static `deriveStateFromProps` method instead. It will be removed entirely in version 17. From aad6845560c562b23de416dd37b4f34aa0bf2472 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 15:53:08 -0800 Subject: [PATCH 04/22] Hopefully clarified the computeMemoizeData() placeholder example --- text/0000-static-lifecycle-methods.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 856323a8..868fccb8 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -33,7 +33,7 @@ class ExampleComponent extends React.Component { addExternalEventListeners(); - computeMemoizeData(nextProps, nextState); + this._computeMemoizeData(nextProps); } componentWillReceiveProps(nextProps) { @@ -57,7 +57,7 @@ class ExampleComponent extends React.Component { nextProps.onChange(nextState.someStatefulValue); } - computeMemoizeData(nextProps, nextState); + this._computeMemoizeData(nextProps); } render() { @@ -127,8 +127,10 @@ class ExampleComponent extends React.Component { render() { // Memoization that doesn't go in state can be done in render. - // It should be idempotent and have no external side effects though. - computeMemoizeData(this.props, this.state); + // It should be idempotent and have no external side effects or mutations. + // Examples include incrementing unique ids, + // Lazily calculating and caching values, etc. + this._computeMemoizeData(this.props); if (this.state.externalData === null) { return
Loading...
; From 99988da86cefdbd05840dbafd71fa4b3eae6f74e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:28:44 -0800 Subject: [PATCH 05/22] Renamed example method for improved clarity --- text/0000-static-lifecycle-methods.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 868fccb8..edbb0225 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -33,7 +33,7 @@ class ExampleComponent extends React.Component { addExternalEventListeners(); - this._computeMemoizeData(nextProps); + this._computeMemoizedInstanceData(nextProps); } componentWillReceiveProps(nextProps) { @@ -57,7 +57,7 @@ class ExampleComponent extends React.Component { nextProps.onChange(nextState.someStatefulValue); } - this._computeMemoizeData(nextProps); + this._computeMemoizedInstanceData(nextProps); } render() { @@ -130,7 +130,7 @@ class ExampleComponent extends React.Component { // It should be idempotent and have no external side effects or mutations. // Examples include incrementing unique ids, // Lazily calculating and caching values, etc. - this._computeMemoizeData(this.props); + this._computeMemoizedInstanceData(); if (this.state.externalData === null) { return
Loading...
; From 4a34d256f6d27aee53805cae96837ba926f20dee Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:29:16 -0800 Subject: [PATCH 06/22] Added note about aEL being unsafe in cWM --- text/0000-static-lifecycle-methods.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index edbb0225..4e83fb99 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -31,6 +31,8 @@ class ExampleComponent extends React.Component { this.setState({ externalData }) ); + // Note that this is not safe; (it can leak) + // But it is a common pattern so I'm showing it here. addExternalEventListeners(); this._computeMemoizedInstanceData(nextProps); From 902d4b33ba0f1057d38829d51f56432883d76121 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:31:25 -0800 Subject: [PATCH 07/22] Fixed typo --- text/0000-static-lifecycle-methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 4e83fb99..abe30a2c 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -35,7 +35,7 @@ class ExampleComponent extends React.Component { // But it is a common pattern so I'm showing it here. addExternalEventListeners(); - this._computeMemoizedInstanceData(nextProps); + this._computeMemoizedInstanceData(this.props); } componentWillReceiveProps(nextProps) { From 3df7ce6b430aa460d5f0ddd8babb58ce8df99522 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:34:54 -0800 Subject: [PATCH 08/22] Fixing typo --- text/0000-static-lifecycle-methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index abe30a2c..f8291ff4 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -119,7 +119,7 @@ class ExampleComponent extends React.Component { componentDidUpdate(prevProps, prevState) { // Callbacks (side effects) are only safe after commit. if (this.state.someStatefulValue !== prevState.someStatefulValue) { - this.state.onChange(this.state.someStatefulValue); + this.props.onChange(this.state.someStatefulValue); } } From 1864c93a4d844964da9839e98066363af60ee0cc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:37:17 -0800 Subject: [PATCH 09/22] Removed Facebook-specific terminology --- text/0000-static-lifecycle-methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index f8291ff4..82e0263f 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -151,7 +151,7 @@ The React team recently added a feature flag to stress-test Facebook components 2. Find and fix async bugs before they impact end-users by intentionally triggering them in a deterministic way. 3. Gain confidence that our existing products could work in async. -I believe this GK confirmed what we suspected about the legacy component API: _It has too many potential pitfalls to be safely used for async rendering._ +I believe the internal experiment confirmed what we suspected about the legacy component API: _It has too many potential pitfalls to be safely used for async rendering._ ## Common problems From 428758b4a7e8ac0ab68ae783b7f921687cba1c77 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:38:54 -0800 Subject: [PATCH 10/22] Tweaked wording for clarity --- text/0000-static-lifecycle-methods.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 82e0263f..cec28bcd 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -183,7 +183,7 @@ This method is invoked before `render` for both the initial render and all subse The purpose of this method is to initiate asynchronous request(s) as early as possible in a component's rendering lifecycle. Such requests will not block `render`. They can be used to pre-prime a cache that is later used in `componentDidMount`/`componentDidUpdate` to trigger a state update. -Avoid introducing any side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. +Avoid introducing any non-idempotent side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. ### `static deriveStateFromProps(props: Props, state: State, prevProps: Props): PartialState | null` From cfcb7e8d69ca77d93b7d82d784e62e446bc61468 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 8 Dec 2017 16:58:26 -0800 Subject: [PATCH 11/22] Reorganization (part 1, more to come) --- text/0000-static-lifecycle-methods.md | 94 +++++++++++++++++---------- 1 file changed, 60 insertions(+), 34 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index cec28bcd..a2bac374 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -10,9 +10,67 @@ Provide a clear migration path for legacy components to become async-ready. # Basic example -## Current API +At a high-level, I propose the following additions/changes to the component API. (The motivations for these changes are explained below.) -The following example combines several patterns that I think are common in React components: +```js +class ExampleComponent extends React.Component { + static deriveStateFromProps(props, state, prevProps) { + // Called before a mounted component receives new props. + // Return an object to update state in response to prop changes. + } + + static prefetch(props, state) { + // Initiate async request(s) as early as possible in rendering lifecycle. + // These requests do not block `render`. + // They can only pre-prime a cache that is used later to update state. + // (This is a micro-optimization and probably not a common use-case.) + } + + unsafe_componentWillMount() { + // New name for componentWillMount() + // Indicates that this method can be unsafe for async rendering. + } + + unsafe_componentWillUpdate(nextProps, nextState) { + // New name for componentWillUpdate() + // Indicates that this method can be unsafe for async rendering. + } + + unsafe_componentWillReceiveProps(nextProps) { + // New name for componentWillReceiveProps() + // Indicates that this method can be unsafe for async rendering. + } +} +``` + +# Motivation + +The React team recently added a feature flag to stress-test Facebook components for potential incompatibilities with our experimental async rendering mode ([facebook/react/pull/11587](https://github.com/facebook/react/pull/11587)). We enabled this feature flag internally so that we could: +1. Identify common problematic coding patterns with the legacy component API to inform a new async component API. +2. Find and fix async bugs before they impact end-users by intentionally triggering them in a deterministic way. +3. Gain confidence that our existing products could work in async. + +I believe this internal experiment confirmed what we suspected about the legacy component API: _It has too many potential pitfalls to be safely used for async rendering._ + +## Common problems + +Some of the most common problematic patterns that were uncovered include: +* **Initializing Flux stores in `componentWillMount`**. It's often unclear whether this is an actual problem or just a potential one (eg if the store or its dependencies change in the future). Because of this uncertainty, it should be avoided. +* **Adding event listeners/subscriptions** in `componentWillMount` and removing them in `componentWillUnmount`. This causes leaks if the initial render is interrupted (or errors) before completion. +* **Non-idempotent external function calls** during `componentWillMount`, `componentWillUpdate`, or `componentWillReceiveProps` (eg registering callbacks that may be invoked multiple times, initializing or configuring shared controllers in such a way as to trigger invariants, etc.) + +## Goal + +The goal of this proposal is to reduce the risk of writing async-compatible React components. I believe that can be accomplished by removing many1 of the potential pitfalls in the current API while retaining important functionality the API enables. This can be done through a combination of: + +1. Choosing lifecycle method names that have a clearer, more limited purpose. +2. Making certain lifecycles static to prevent unsafe access of instance properties. + +1 It is not possible to detect or prevent all side-effects (eg mutations of global/shared objects). + +## Examples + +The following example combines all of the common, potentially-problematic patterns listed above. (Based on initial feedback, I will rewrite this as several smaller, more focused examples shortly.) ```js class ExampleComponent extends React.Component { @@ -72,8 +130,6 @@ class ExampleComponent extends React.Component { } ``` -## Proposed API - This proposal would modify the above component as follows: ```js @@ -141,38 +197,8 @@ class ExampleComponent extends React.Component { // Render real view... } } - ``` -# Motivation - -The React team recently added a feature flag to stress-test Facebook components for potential incompatibilities with our experimental async rendering mode ([facebook/react/pull/11587](https://github.com/facebook/react/pull/11587)). We enabled this feature flag internally so that we could: -1. Identify common problematic coding patterns with the legacy component API to inform a new async component API. -2. Find and fix async bugs before they impact end-users by intentionally triggering them in a deterministic way. -3. Gain confidence that our existing products could work in async. - -I believe the internal experiment confirmed what we suspected about the legacy component API: _It has too many potential pitfalls to be safely used for async rendering._ - -## Common problems - -Some of the most common problematic patterns that were uncovered include: -* **Initializing Flux stores in `componentWillMount`**. It's often unclear whether this is an actual problem or just a potential one (eg if the store or its dependencies change in the future). Because of this uncertainty, it should be avoided. -* **Adding event listeners/subscriptions** in `componentWillMount` and removing them in `componentWillUnmount`. This causes leaks if the initial render is interrupted (or errors) before completion. -* **Non-idempotent external function calls** during `componentWillMount`, `componentWillUpdate`, or `componentWillReceiveProps` (eg registering callbacks that may be invoked multiple times, initializing or configuring shared controllers in such a way as to trigger invariants, etc.) - -The [example above](#basic-example) attempts to illustrate a few of these patterns. - -## Proposal - -This proposal is intended to reduce the risk of writing async-compatible React components. - -It does this by removing many1 of the potential pitfalls in the current API while retaining important functionality the API enables. I believe this can be accomplished through a combination of: - -1. Choosing lifecycle method names that have a clearer, more limited purpose. -2. Making certain lifecycles static to prevent unsafe access of instance properties. - -1 It is not possible to detect or prevent all side-effects (eg mutations of global/shared objects). - # Detailed design ## New static lifecycle methods From 536084db9b3a0f4289a509824f69b9f035612c68 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 9 Dec 2017 08:38:53 -0800 Subject: [PATCH 12/22] Added some more focused examples --- text/0000-static-lifecycle-methods.md | 261 +++++++++++++++++++------- 1 file changed, 191 insertions(+), 70 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index a2bac374..b032f69c 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -17,6 +17,7 @@ class ExampleComponent extends React.Component { static deriveStateFromProps(props, state, prevProps) { // Called before a mounted component receives new props. // Return an object to update state in response to prop changes. + // Return null to indicate no change to state. } static prefetch(props, state) { @@ -70,131 +71,251 @@ The goal of this proposal is to reduce the risk of writing async-compatible Reac ## Examples -The following example combines all of the common, potentially-problematic patterns listed above. (Based on initial feedback, I will rewrite this as several smaller, more focused examples shortly.) +Let's look at some of the common usage patterns mentioned above and how they might be adapted to the new proposed API. + +### Prefetching async data during mount + +The purpose of this pattern is to initiate data loading as early as possible. + +It is worth noting that in both examples below, the data will not finish loading before the initial render (so a second render pass will be required in either case). + +#### Before ```js class ExampleComponent extends React.Component { - constructor(props) { - super(props); - - this.state = { - derivedData: null, - externalData: null, - someStatefulValue: null - }; - } + state = { + externalData: null, + }; componentWillMount() { asyncLoadData(this.props.someId).then(externalData => this.setState({ externalData }) ); - - // Note that this is not safe; (it can leak) - // But it is a common pattern so I'm showing it here. - addExternalEventListeners(); - - this._computeMemoizedInstanceData(this.props); } - componentWillReceiveProps(nextProps) { - this.setState({ - derivedData: computeDerivedState(nextProps) - }); + render() { + if (this.state.externalData === null) { + // Render loading UI... + } else { + // Render real view... + } } +} +``` - componentWillUnmount() { - removeExternalEventListeners(); - } +#### After - componentWillUpdate(nextProps, nextState) { - if (this.props.someId !== nextProps.someId) { - asyncLoadData(nextProps.someId).then(externalData => - this.setState({ externalData }) - ); - } +```js +class ExampleComponent extends React.Component { + state = { + externalData: null, + }; - if (this.state.someStatefulValue !== nextState.someStatefulValue) { - nextProps.onChange(nextState.someStatefulValue); + static prefetch(props, state) { + // Prime an external cache as early as possible. + // (Async request won't complete before render anyway.) + if (state.externalData === null) { + asyncLoadData(props.someId); } + } - this._computeMemoizedInstanceData(nextProps); + componentDidMount() { + // Wait for earlier pre-fetch to complete and update state. + // (This assumes some kind of cache to avoid duplicate requests.) + asyncLoadData(props.someId).then(externalData => + this.setState({ externalData }) + ); } render() { if (this.state.externalData === null) { - return
Loading...
; + // Render loading UI... + } else { + // Render real view... } - - // Render real view... } } ``` -This proposal would modify the above component as follows: +### State derived from props/state + +The purpose of this pattern is to calculate some values derived from props for use during render. + +Typically `componentWillReceiveProps` is used for this, although if the calculation is fast enough it could just be done in `render`. + +#### Before ```js class ExampleComponent extends React.Component { - constructor(props) { - super(props); - - this.state = { - derivedData: null, - externalData: null, - someStatefulValue: null - }; + componentWillReceiveProps(nextProps) { + if (this.props.someValue !== nextProps.someValue) { + this.setState({ + derivedData: computeDerivedState(nextProps) + }); + } } +} +``` +#### After + +```js +class ExampleComponent extends React.Component { static deriveStateFromProps(props, state, prevProps) { - // If derived state is expensive to calculate, - // You can compare props to prevProps and conditionally update. - return { - derivedData: computeDerivedState(props) - }; + if (props.someValue !== prevProps.someValue) { + return { + derivedData: computeDerivedState(props) + }; + } + + // Return null to indicate no change to state. + return null; } +} +``` - static prefetch(props, state) { - // Prime the async cache early. - // (Async request won't complete before render anyway.) - // If you only need to pre-fetch before mount, - // You can conditionally fetch based on state. - asyncLoadData(props.someId); +### Adding event listeners/subscriptions + +The purpose of this pattern is to subscribe a component to external events when it mounts and unsubscribe it when it unmounts. + +The `componentWillMount` lifecycle is often used for this purpose, but this is problematic because any interruption _or_ error during initial mount will cause a memory leak. (The `componentWillUnmount` lifecycle hook is not invoked for a component that does not finish mounting and so there's no safe place to handle unsubscriptions in that case.) + +#### Before + +```js +class ExampleComponent extends React.Component { + componentWillMount() { + // This is not safe; (it can leak). + addExternalEventListeners(); } + componentWillUnmount() { + removeExternalEventListeners(); + } +} +``` + +#### After + +```js +class ExampleComponent extends React.Component { componentDidMount() { // Event listeners are only safe to add after mount, // So they won't leak if mount is interrupted or errors. addExternalEventListeners(); + } - // Wait for earlier pre-fetch to complete and update state. - // (This assumes some kind of cache to avoid duplicate requests.) - asyncLoadData(props.someId).then(externalData => - this.setState({ externalData }) - ); + componentWillUnmount() { + removeExternalEventListeners(); } +} +``` + +### External function calls (side effects, mutations) + +The purpose of this pattern is to send an external signal that something has changed internally (eg in `state`). + +The `componentWillUpdate` lifecycle hook is sometimes used for this but it is not ideal because this method may be called multiple times _or_ called for props that are never committed. `componentDidUpdate` should be used for this purpose rather than `componentWillUpdate`. +#### Before + +```js +class ExampleComponent extends React.Component { + componentWillUpdate(nextProps, nextState) { + if (this.state.someStatefulValue !== nextState.someStatefulValue) { + nextProps.onChange(nextState.someStatefulValue); + } + } +} +``` + +#### After + +```js +class ExampleComponent extends React.Component { componentDidUpdate(prevProps, prevState) { // Callbacks (side effects) are only safe after commit. if (this.state.someStatefulValue !== prevState.someStatefulValue) { this.props.onChange(this.state.someStatefulValue); } } +} +``` - componentWillUnmount() { - removeExternalEventListeners(); +### Memoized values derived from `props` and/or `state` + +The purpose of this pattern is to memoize computed values based on `props` and/or `state`. + +Typically such values are stored in `state`, but in some cases the values require mutation and as such may not seem suited for state (although they could technically still be stored there). An example of this would be an external helper class that calculates and memoizes values interally. + +In other cases the value may be derived from `props` _and_ `state`. + +#### Before + +```js +class ExampleComponent extends React.Component { + componentWillMount() { + this._calculateMemoizedValues(this.props, this.state); } + componentWillUpdate(nextProps, nextState) { + if ( + this.props.someValue !== nextProps.someValue || + this.state.someOtherValue !== nextState.someOtherValue + ) { + this._calculateMemoizedValues(nextProps, nextState); + } + } + + render() { + // Render view using calculated memoized values... + } +} +``` + +#### After + +```js +class ExampleComponent extends React.Component { render() { // Memoization that doesn't go in state can be done in render. // It should be idempotent and have no external side effects or mutations. - // Examples include incrementing unique ids, - // Lazily calculating and caching values, etc. - this._computeMemoizedInstanceData(); + this._calculateMemoizedValues(this.props, this.state); - if (this.state.externalData === null) { - return
Loading...
; - } + // Render view using calculated memoized values... + } +} +``` + +### Initializing Flux stores during mount - // Render real view... +The purpose of this pattern is to initialize some Flux state when a component is mounted. + +This is sometimes done in `componentWillMount` which can be problematic if, for example, the action is not idempotent. From the point of view of the component, it's often unclear whether dispatching the action more than once will cause a problem. It's also possible that it does not cause a problem when the component is authored but later does due to changes in the store. Because of this uncertainty, it should be avoided. + +We recommend using `componentDidMount` for such actions since it will only be invoked once. + +If for some reason your application _requires_ a store to be configured by your component before initial render, and you're certain the action will remain idempotent, this could still be accomplished using either the class constructor or the `render` method itself. It is not a recommended pattern though. + +#### Before + +```js +class ExampleComponent extends React.Component { + componentWillMount() { + FluxStore.dispatchSomeAction(); + } +} + +``` + +#### After + +```js +class ExampleComponent extends React.Component { + componentDidMount() { + // Side effects (like Flux actions) should only be done after mount or update. + // This prevents duplicate actions or certain types of infinite loops. + FluxStore.dispatchSomeAction(); } } ``` @@ -213,7 +334,7 @@ Avoid introducing any non-idempotent side-effects, mutations, or subscriptions i ### `static deriveStateFromProps(props: Props, state: State, prevProps: Props): PartialState | null` -This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. +This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. Note that React may call this method even if the props have not changed. I calculating derived data is expensive, compare new and previous prop values to conditionally handle changes. From a1431a47de77b2ce6a48b47e3dea0a652b758bfd Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 9 Dec 2017 08:59:46 -0800 Subject: [PATCH 13/22] Added a comment about calling setState on a possibly unmounted component --- text/0000-static-lifecycle-methods.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index b032f69c..dcaf1c0e 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -122,9 +122,12 @@ class ExampleComponent extends React.Component { componentDidMount() { // Wait for earlier pre-fetch to complete and update state. // (This assumes some kind of cache to avoid duplicate requests.) - asyncLoadData(props.someId).then(externalData => - this.setState({ externalData }) - ); + asyncLoadData(props.someId).then(externalData => { + // Note that if the component unmounts before this request completes, + // It will trigger a warning, "cannot update an unmounted component". + // You can avoid this by tracking mounted state with an instance var if desired. + this.setState({ externalData }); + }); } render() { From 3c6132edfa004458460eeaefd383f592feb1b004 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 9 Dec 2017 09:22:46 -0800 Subject: [PATCH 14/22] Cancel async request on unmount in example --- text/0000-static-lifecycle-methods.md | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index dcaf1c0e..a8b2b3e6 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -107,6 +107,8 @@ class ExampleComponent extends React.Component { ```js class ExampleComponent extends React.Component { + _asyncRequest = null; + state = { externalData: null, }; @@ -122,12 +124,20 @@ class ExampleComponent extends React.Component { componentDidMount() { // Wait for earlier pre-fetch to complete and update state. // (This assumes some kind of cache to avoid duplicate requests.) - asyncLoadData(props.someId).then(externalData => { - // Note that if the component unmounts before this request completes, - // It will trigger a warning, "cannot update an unmounted component". - // You can avoid this by tracking mounted state with an instance var if desired. - this.setState({ externalData }); - }); + this._asyncRequest = asyncLoadData(props.someId) + .then(externalData => { + this._asyncRequest = null; + + this.setState({ externalData }); + }); + } + + componentWillUnmount() { + // Note that cancelling on unmount is not really related to this proposal. + // I'm just showing it to avoid calling setState on an unmounted component. + if (this._asyncRequest !== null) { + this._asyncRequest.cancel(); + } } render() { From 4425dbe53e5d0eaf72096b183caa60f9d8a14f73 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 9 Dec 2017 09:46:23 -0800 Subject: [PATCH 15/22] Tweaking examples based on Dan's feedback --- text/0000-static-lifecycle-methods.md | 76 +++++++++++++++++++-------- 1 file changed, 53 insertions(+), 23 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index a8b2b3e6..aa952c95 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -107,8 +107,6 @@ class ExampleComponent extends React.Component { ```js class ExampleComponent extends React.Component { - _asyncRequest = null; - state = { externalData: null, }; @@ -124,20 +122,12 @@ class ExampleComponent extends React.Component { componentDidMount() { // Wait for earlier pre-fetch to complete and update state. // (This assumes some kind of cache to avoid duplicate requests.) - this._asyncRequest = asyncLoadData(props.someId) - .then(externalData => { - this._asyncRequest = null; - - this.setState({ externalData }); - }); - } - - componentWillUnmount() { - // Note that cancelling on unmount is not really related to this proposal. - // I'm just showing it to avoid calling setState on an unmounted component. - if (this._asyncRequest !== null) { - this._asyncRequest.cancel(); - } + asyncLoadData(props.someId).then(externalData => { + // Note that if the component unmounts before this request completes, + // It will trigger a warning, "cannot update an unmounted component". + // You can avoid this by tracking mounted state with an instance var if desired. + this.setState({ externalData }); + }); } render() { @@ -160,6 +150,10 @@ Typically `componentWillReceiveProps` is used for this, although if the calculat ```js class ExampleComponent extends React.Component { + state = { + derivedData: computeDerivedState(this.props) + }; + componentWillReceiveProps(nextProps) { if (this.props.someValue !== nextProps.someValue) { this.setState({ @@ -174,6 +168,10 @@ class ExampleComponent extends React.Component { ```js class ExampleComponent extends React.Component { + state = { + derivedData: computeDerivedState(this.props) + }; + static deriveStateFromProps(props, state, prevProps) { if (props.someValue !== prevProps.someValue) { return { @@ -193,18 +191,32 @@ The purpose of this pattern is to subscribe a component to external events when The `componentWillMount` lifecycle is often used for this purpose, but this is problematic because any interruption _or_ error during initial mount will cause a memory leak. (The `componentWillUnmount` lifecycle hook is not invoked for a component that does not finish mounting and so there's no safe place to handle unsubscriptions in that case.) +Using `componentWillMount` for this purpose might also cause problems in the context of server-rendering. + #### Before ```js class ExampleComponent extends React.Component { componentWillMount() { + this.setState({ + subscribedValue: this.props.dataSource.value + }); + // This is not safe; (it can leak). - addExternalEventListeners(); + this.props.dataSource.subscribe(this._onSubscriptionChange); } componentWillUnmount() { - removeExternalEventListeners(); + this.props.dataSource.unsubscribe(this._onSubscriptionChange); } + + render() { + // Render view using subscribed value... + } + + _onSubscriptionChange = subscribedValue => { + this.setState({ subscribedValue }); + }; } ``` @@ -212,15 +224,35 @@ class ExampleComponent extends React.Component { ```js class ExampleComponent extends React.Component { + state = { + subscribedValue: this.props.dataSource.value + }; + componentDidMount() { // Event listeners are only safe to add after mount, // So they won't leak if mount is interrupted or errors. - addExternalEventListeners(); + this.props.dataSource.subscribe(this._onSubscriptionChange); + + // External values could change between render and mount, + // In some cases it may be important to handle this case. + if (this.state.subscribedValue !== this.props.dataSource.value) { + this.setState({ + subscribedValue: this.props.dataSource.value + }); + } } componentWillUnmount() { - removeExternalEventListeners(); + this.props.dataSource.unsubscribe(this._onSubscriptionChange); + } + + render() { + // Render view using subscribed value... } + + _onSubscriptionChange = subscribedValue => { + this.setState({ subscribedValue }); + }; } ``` @@ -308,8 +340,6 @@ This is sometimes done in `componentWillMount` which can be problematic if, for We recommend using `componentDidMount` for such actions since it will only be invoked once. -If for some reason your application _requires_ a store to be configured by your component before initial render, and you're certain the action will remain idempotent, this could still be accomplished using either the class constructor or the `render` method itself. It is not a recommended pattern though. - #### Before ```js @@ -349,7 +379,7 @@ Avoid introducing any non-idempotent side-effects, mutations, or subscriptions i This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. -Note that React may call this method even if the props have not changed. I calculating derived data is expensive, compare new and previous prop values to conditionally handle changes. +Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare new and previous prop values to conditionally handle changes. React does not call this method before the intial render/mount and so it is not called during server rendering. From 67272ce6a9c6cc93610d7775f25a9d8d86f4b83c Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 13 Dec 2017 07:57:51 -0800 Subject: [PATCH 16/22] Renamed deriveStateFromProps to getDerivedStateFromNextProps --- text/0000-static-lifecycle-methods.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index aa952c95..8509a987 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -14,7 +14,7 @@ At a high-level, I propose the following additions/changes to the component API. ```js class ExampleComponent extends React.Component { - static deriveStateFromProps(props, state, prevProps) { + static getDerivedStateFromNextProps(nextProps, prevProps, prevState) { // Called before a mounted component receives new props. // Return an object to update state in response to prop changes. // Return null to indicate no change to state. @@ -172,10 +172,10 @@ class ExampleComponent extends React.Component { derivedData: computeDerivedState(this.props) }; - static deriveStateFromProps(props, state, prevProps) { - if (props.someValue !== prevProps.someValue) { + static getDerivedStateFromNextProps(nextProps, prevProps, prevState) { + if (nextProps.someValue !== prevProps.someValue) { return { - derivedData: computeDerivedState(props) + derivedData: computeDerivedState(nextProps) }; } @@ -375,11 +375,11 @@ The purpose of this method is to initiate asynchronous request(s) as early as po Avoid introducing any non-idempotent side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. -### `static deriveStateFromProps(props: Props, state: State, prevProps: Props): PartialState | null` +### `static getDerivedStateFromNextProps(nextProps: Props, prevProps: Props, prevState: Props): PartialState | null` This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. -Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare new and previous prop values to conditionally handle changes. +Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare next and previous props to conditionally handle changes. React does not call this method before the intial render/mount and so it is not called during server rendering. @@ -395,7 +395,7 @@ This method will log a deprecation warning in development mode recommending that ### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillReceiveProps` or use the new static `deriveStateFromProps` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillReceiveProps` or use the new static `getDerivedStateFromNextProps` method instead. It will be removed entirely in version 17. # Drawbacks From c7f6728398e43f0f19269781b5ea7769ee2d747c Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 13 Dec 2017 07:59:43 -0800 Subject: [PATCH 17/22] Renamed prefetch to optimisticallyPrepareToRender --- text/0000-static-lifecycle-methods.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 8509a987..14a82787 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -20,7 +20,7 @@ class ExampleComponent extends React.Component { // Return null to indicate no change to state. } - static prefetch(props, state) { + static optimisticallyPrepareToRender(props, state) { // Initiate async request(s) as early as possible in rendering lifecycle. // These requests do not block `render`. // They can only pre-prime a cache that is used later to update state. @@ -111,7 +111,7 @@ class ExampleComponent extends React.Component { externalData: null, }; - static prefetch(props, state) { + static optimisticallyPrepareToRender(props, state) { // Prime an external cache as early as possible. // (Async request won't complete before render anyway.) if (state.externalData === null) { @@ -367,14 +367,6 @@ class ExampleComponent extends React.Component { ## New static lifecycle methods -### `static prefetch(props: Props, state: State): void` - -This method is invoked before `render` for both the initial render and all subsequent updates. It is not called during server rendering. - -The purpose of this method is to initiate asynchronous request(s) as early as possible in a component's rendering lifecycle. Such requests will not block `render`. They can be used to pre-prime a cache that is later used in `componentDidMount`/`componentDidUpdate` to trigger a state update. - -Avoid introducing any non-idempotent side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. - ### `static getDerivedStateFromNextProps(nextProps: Props, prevProps: Props, prevState: Props): PartialState | null` This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. @@ -383,15 +375,23 @@ Note that React may call this method even if the props have not changed. If calc React does not call this method before the intial render/mount and so it is not called during server rendering. +### `static optimisticallyPrepareToRender(props: Props, state: State): void` + +This method is invoked before `render` for both the initial render and all subsequent updates. It is not called during server rendering. + +The purpose of this method is to initiate asynchronous request(s) as early as possible in a component's rendering lifecycle. Such requests will not block `render`. They can be used to pre-prime a cache that is later used in `componentDidMount`/`componentDidUpdate` to trigger a state update. + +Avoid introducing any non-idempotent side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. + ## Deprecated lifecycle methods ### `componentWillMount` -> `unsafe_componentWillMount` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillMount` or use the new static `prefetch` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillMount` or use the new static `optimisticallyPrepareToRender` method instead. It will be removed entirely in version 17. ### `componentWillUpdate` -> `unsafe_componentWillUpdate` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillUpdate` or use the new static `prefetch` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillUpdate` or use the new static `optimisticallyPrepareToRender` method instead. It will be removed entirely in version 17. ### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` From bb2d24645ab5f2fc4b5285f975d98c8d4bd40d9d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 14 Dec 2017 13:04:33 -0800 Subject: [PATCH 18/22] Added `render` to a list --- text/0000-static-lifecycle-methods.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 14a82787..d4638ee2 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -58,7 +58,7 @@ I believe this internal experiment confirmed what we suspected about the legacy Some of the most common problematic patterns that were uncovered include: * **Initializing Flux stores in `componentWillMount`**. It's often unclear whether this is an actual problem or just a potential one (eg if the store or its dependencies change in the future). Because of this uncertainty, it should be avoided. * **Adding event listeners/subscriptions** in `componentWillMount` and removing them in `componentWillUnmount`. This causes leaks if the initial render is interrupted (or errors) before completion. -* **Non-idempotent external function calls** during `componentWillMount`, `componentWillUpdate`, or `componentWillReceiveProps` (eg registering callbacks that may be invoked multiple times, initializing or configuring shared controllers in such a way as to trigger invariants, etc.) +* **Non-idempotent external function calls** during `componentWillMount`, `componentWillUpdate`, `componentWillReceiveProps`, or `render` (eg registering callbacks that may be invoked multiple times, initializing or configuring shared controllers in such a way as to trigger invariants, etc.) ## Goal @@ -449,4 +449,4 @@ Leaving `render` as an instance method also provides a mechanism (other than `st ## Other -Are there important use cases that I've overlooked that the new static lifecycles would be insufficient to handle? \ No newline at end of file +Are there important use cases that I've overlooked that the new static lifecycles would be insufficient to handle? From 8618f705485ac1d2c88573e2247ab3e18b2b3fa7 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 19 Dec 2017 13:55:15 -0800 Subject: [PATCH 19/22] Removed static optimisticallyPrepareToRender() in favor of render() --- text/0000-static-lifecycle-methods.md | 46 +++++++++++---------------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index d4638ee2..4dd82724 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -20,26 +20,22 @@ class ExampleComponent extends React.Component { // Return null to indicate no change to state. } - static optimisticallyPrepareToRender(props, state) { - // Initiate async request(s) as early as possible in rendering lifecycle. - // These requests do not block `render`. - // They can only pre-prime a cache that is used later to update state. - // (This is a micro-optimization and probably not a common use-case.) - } - unsafe_componentWillMount() { // New name for componentWillMount() // Indicates that this method can be unsafe for async rendering. + // Prefer componentDidMount() instead. } unsafe_componentWillUpdate(nextProps, nextState) { // New name for componentWillUpdate() // Indicates that this method can be unsafe for async rendering. + // Prefer componentDidUpdate() instead. } unsafe_componentWillReceiveProps(nextProps) { // New name for componentWillReceiveProps() // Indicates that this method can be unsafe for async rendering. + // Prefer static getDerivedStateFromNextProps() instead. } } ``` @@ -75,9 +71,9 @@ Let's look at some of the common usage patterns mentioned above and how they mig ### Prefetching async data during mount -The purpose of this pattern is to initiate data loading as early as possible. +The purpose of this pattern is to initiate data loading as early as possible. Particularly on slower, mobile devices, there may be several hundred miliseconds between the intial call to `render` and the subsequent `componentDidMount`. This pattern eagerly fetches data without waiting for the did-mount callback. -It is worth noting that in both examples below, the data will not finish loading before the initial render (so a second render pass will be required in either case). +It is worth noting that in both examples below, the data will not finish loading before the initial render and so a second render pass will be required in either case. #### Before @@ -111,14 +107,6 @@ class ExampleComponent extends React.Component { externalData: null, }; - static optimisticallyPrepareToRender(props, state) { - // Prime an external cache as early as possible. - // (Async request won't complete before render anyway.) - if (state.externalData === null) { - asyncLoadData(props.someId); - } - } - componentDidMount() { // Wait for earlier pre-fetch to complete and update state. // (This assumes some kind of cache to avoid duplicate requests.) @@ -132,6 +120,10 @@ class ExampleComponent extends React.Component { render() { if (this.state.externalData === null) { + // Prime an external cache as early as possible. + // (Async request would not complete before render anyway.) + asyncLoadData(this.props.someId); + // Render loading UI... } else { // Render real view... @@ -375,27 +367,25 @@ Note that React may call this method even if the props have not changed. If calc React does not call this method before the intial render/mount and so it is not called during server rendering. -### `static optimisticallyPrepareToRender(props: Props, state: State): void` - -This method is invoked before `render` for both the initial render and all subsequent updates. It is not called during server rendering. - -The purpose of this method is to initiate asynchronous request(s) as early as possible in a component's rendering lifecycle. Such requests will not block `render`. They can be used to pre-prime a cache that is later used in `componentDidMount`/`componentDidUpdate` to trigger a state update. - -Avoid introducing any non-idempotent side-effects, mutations, or subscriptions in this method. For those use cases, use `componentDidMount`/`componentDidUpdate` instead. - ## Deprecated lifecycle methods ### `componentWillMount` -> `unsafe_componentWillMount` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillMount` or use the new static `optimisticallyPrepareToRender` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users use `componentDidMount` instead (when possible) or rename to `unsafe_componentWillMount`. + +`componentWillMount` will be removed entirely in version 17. ### `componentWillUpdate` -> `unsafe_componentWillUpdate` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillUpdate` or use the new static `optimisticallyPrepareToRender` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users use `componentDidUpdate` instead (when possible) or rename to `unsafe_componentWillUpdate`. + +`componentWillUpdate` will be removed entirely in version 17. ### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` -This method will log a deprecation warning in development mode recommending that users either rename to `unsafe_componentWillReceiveProps` or use the new static `getDerivedStateFromNextProps` method instead. It will be removed entirely in version 17. +This method will log a deprecation warning in development mode recommending that users use the new static `getDerivedStateFromNextProps` method instead (when possible) or rename to `unsafe_componentWillReceiveProps`. + +`componentWillReceiveProps` will be removed entirely in version 17. # Drawbacks From 8f6c20edd98f16ecc7395b83332944d1757590a4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Jan 2018 09:13:48 -0800 Subject: [PATCH 20/22] Renamed getDerivedStateFromNextProps to getDerivedStateFromProps --- text/0000-static-lifecycle-methods.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 4dd82724..815baaa7 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -14,7 +14,7 @@ At a high-level, I propose the following additions/changes to the component API. ```js class ExampleComponent extends React.Component { - static getDerivedStateFromNextProps(nextProps, prevProps, prevState) { + static getDerivedStateFromProps(nextProps, prevProps, prevState) { // Called before a mounted component receives new props. // Return an object to update state in response to prop changes. // Return null to indicate no change to state. @@ -35,7 +35,7 @@ class ExampleComponent extends React.Component { unsafe_componentWillReceiveProps(nextProps) { // New name for componentWillReceiveProps() // Indicates that this method can be unsafe for async rendering. - // Prefer static getDerivedStateFromNextProps() instead. + // Prefer static getDerivedStateFromProps() instead. } } ``` @@ -164,7 +164,7 @@ class ExampleComponent extends React.Component { derivedData: computeDerivedState(this.props) }; - static getDerivedStateFromNextProps(nextProps, prevProps, prevState) { + static getDerivedStateFromProps(nextProps, prevProps, prevState) { if (nextProps.someValue !== prevProps.someValue) { return { derivedData: computeDerivedState(nextProps) @@ -359,7 +359,7 @@ class ExampleComponent extends React.Component { ## New static lifecycle methods -### `static getDerivedStateFromNextProps(nextProps: Props, prevProps: Props, prevState: Props): PartialState | null` +### `static getDerivedStateFromProps(nextProps: Props, prevProps: Props, prevState: Props): PartialState | null` This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. @@ -383,7 +383,7 @@ This method will log a deprecation warning in development mode recommending that ### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` -This method will log a deprecation warning in development mode recommending that users use the new static `getDerivedStateFromNextProps` method instead (when possible) or rename to `unsafe_componentWillReceiveProps`. +This method will log a deprecation warning in development mode recommending that users use the new static `getDerivedStateFromProps` method instead (when possible) or rename to `unsafe_componentWillReceiveProps`. `componentWillReceiveProps` will be removed entirely in version 17. From e05e317f8a293459273a162e92635807e7598b53 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Jan 2018 09:18:46 -0800 Subject: [PATCH 21/22] Updated when getDerivedStateFromProps is called and what its arguments are --- text/0000-static-lifecycle-methods.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index 815baaa7..ac22e611 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -14,8 +14,8 @@ At a high-level, I propose the following additions/changes to the component API. ```js class ExampleComponent extends React.Component { - static getDerivedStateFromProps(nextProps, prevProps, prevState) { - // Called before a mounted component receives new props. + static getDerivedStateFromProps(nextProps, prevState) { + // Called after a component is instantiated or before it receives new props. // Return an object to update state in response to prop changes. // Return null to indicate no change to state. } @@ -160,14 +160,14 @@ class ExampleComponent extends React.Component { ```js class ExampleComponent extends React.Component { - state = { - derivedData: computeDerivedState(this.props) - }; - - static getDerivedStateFromProps(nextProps, prevProps, prevState) { - if (nextProps.someValue !== prevProps.someValue) { + static getDerivedStateFromProps(nextProps, prevState) { + if ( + !prevState || + prevState.someMirroredValue !== nextProps.someValue + ) { return { - derivedData: computeDerivedState(nextProps) + derivedData: computeDerivedState(nextProps), + someMirroredValue: nextProps.someValue }; } @@ -359,13 +359,13 @@ class ExampleComponent extends React.Component { ## New static lifecycle methods -### `static getDerivedStateFromProps(nextProps: Props, prevProps: Props, prevState: Props): PartialState | null` +### `static getDerivedStateFromProps(nextProps: Props, prevState: State | null): PartialState | null` -This method is invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. +This method is invoked after a component is constructed. Return an object to initialize component state. Note that the value of `prevState` may be null in this case if the constructor did not initialize `this.state`. -Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare next and previous props to conditionally handle changes. +This method is also invoked before a mounted component receives new props. Return an object to update state in response to prop changes. Return null to indicate no change to state. -React does not call this method before the intial render/mount and so it is not called during server rendering. +Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare next and previous props to conditionally handle changes. ## Deprecated lifecycle methods From 7042a2abcbdc90b4a7784ea6a753ce9abac169d9 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Jan 2018 09:19:09 -0800 Subject: [PATCH 22/22] Renamed unsafe_* to UNSAFE_* --- text/0000-static-lifecycle-methods.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/text/0000-static-lifecycle-methods.md b/text/0000-static-lifecycle-methods.md index ac22e611..6a4f9342 100644 --- a/text/0000-static-lifecycle-methods.md +++ b/text/0000-static-lifecycle-methods.md @@ -20,19 +20,19 @@ class ExampleComponent extends React.Component { // Return null to indicate no change to state. } - unsafe_componentWillMount() { + UNSAFE_componentWillMount() { // New name for componentWillMount() // Indicates that this method can be unsafe for async rendering. // Prefer componentDidMount() instead. } - unsafe_componentWillUpdate(nextProps, nextState) { + UNSAFE_componentWillUpdate(nextProps, nextState) { // New name for componentWillUpdate() // Indicates that this method can be unsafe for async rendering. // Prefer componentDidUpdate() instead. } - unsafe_componentWillReceiveProps(nextProps) { + UNSAFE_componentWillReceiveProps(nextProps) { // New name for componentWillReceiveProps() // Indicates that this method can be unsafe for async rendering. // Prefer static getDerivedStateFromProps() instead. @@ -369,21 +369,21 @@ Note that React may call this method even if the props have not changed. If calc ## Deprecated lifecycle methods -### `componentWillMount` -> `unsafe_componentWillMount` +### `componentWillMount` -> `UNSAFE_componentWillMount` -This method will log a deprecation warning in development mode recommending that users use `componentDidMount` instead (when possible) or rename to `unsafe_componentWillMount`. +This method will log a deprecation warning in development mode recommending that users use `componentDidMount` instead (when possible) or rename to `UNSAFE_componentWillMount`. `componentWillMount` will be removed entirely in version 17. -### `componentWillUpdate` -> `unsafe_componentWillUpdate` +### `componentWillUpdate` -> `UNSAFE_componentWillUpdate` -This method will log a deprecation warning in development mode recommending that users use `componentDidUpdate` instead (when possible) or rename to `unsafe_componentWillUpdate`. +This method will log a deprecation warning in development mode recommending that users use `componentDidUpdate` instead (when possible) or rename to `UNSAFE_componentWillUpdate`. `componentWillUpdate` will be removed entirely in version 17. -### `componentWillReceiveProps` -> `unsafe_componentWillReceiveProps` +### `componentWillReceiveProps` -> `UNSAFE_componentWillReceiveProps` -This method will log a deprecation warning in development mode recommending that users use the new static `getDerivedStateFromProps` method instead (when possible) or rename to `unsafe_componentWillReceiveProps`. +This method will log a deprecation warning in development mode recommending that users use the new static `getDerivedStateFromProps` method instead (when possible) or rename to `UNSAFE_componentWillReceiveProps`. `componentWillReceiveProps` will be removed entirely in version 17. @@ -411,9 +411,9 @@ There are no advanced proposals for such a stateful, functional component API th Begin by reaching out to prominent third-party library maintainers to make sure there are no use-cases we have failed to consider. -Assuming we move forward with the proposal, release (at least one) minor 16.x update to add deprecation warnings for the legacy lifecycles and inform users to either rename with the `unsafe_` prefix or use the new static methods instead. We'll then cordinate with library authors to ensure they have enough time to migrate to the new API in advance of the major release that drops support for the legacy lifecycles. +Assuming we move forward with the proposal, release (at least one) minor 16.x update to add deprecation warnings for the legacy lifecycles and inform users to either rename with the `UNSAFE_` prefix or use the new static methods instead. We'll then cordinate with library authors to ensure they have enough time to migrate to the new API in advance of the major release that drops support for the legacy lifecycles. -We will provide a codemod to rename the deprecated lifecycle hooks with the new `unsafe_` prefix. +We will provide a codemod to rename the deprecated lifecycle hooks with the new `UNSAFE_` prefix. We will also provide codemods to assist with the migration to static methods, although given the nature of the change, codemods will be insufficient to handle all cases. Manual verification will be required.