Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9818f57
Initial proposal for async-friendly, static lifecycle hooks
bvaughn Dec 8, 2017
839547f
Added a note about the prefix
bvaughn Dec 8, 2017
b7189cc
Made deprecation/rename clearer
bvaughn Dec 8, 2017
aad6845
Hopefully clarified the computeMemoizeData() placeholder example
bvaughn Dec 8, 2017
99988da
Renamed example method for improved clarity
bvaughn Dec 9, 2017
4a34d25
Added note about aEL being unsafe in cWM
bvaughn Dec 9, 2017
902d4b3
Fixed typo
bvaughn Dec 9, 2017
3df7ce6
Fixing typo
bvaughn Dec 9, 2017
1864c93
Removed Facebook-specific terminology
bvaughn Dec 9, 2017
428758b
Tweaked wording for clarity
bvaughn Dec 9, 2017
cfcb7e8
Reorganization (part 1, more to come)
bvaughn Dec 9, 2017
536084d
Added some more focused examples
bvaughn Dec 9, 2017
a1431a4
Added a comment about calling setState on a possibly unmounted component
bvaughn Dec 9, 2017
3c6132e
Cancel async request on unmount in example
bvaughn Dec 9, 2017
4425dbe
Tweaking examples based on Dan's feedback
bvaughn Dec 9, 2017
67272ce
Renamed deriveStateFromProps to getDerivedStateFromNextProps
bvaughn Dec 13, 2017
c7f6728
Renamed prefetch to optimisticallyPrepareToRender
bvaughn Dec 13, 2017
bb2d246
Added `render` to a list
bvaughn Dec 14, 2017
8618f70
Removed static optimisticallyPrepareToRender() in favor of render()
bvaughn Dec 19, 2017
8f6c20e
Renamed getDerivedStateFromNextProps to getDerivedStateFromProps
bvaughn Jan 18, 2018
e05e317
Updated when getDerivedStateFromProps is called and what its argument…
bvaughn Jan 18, 2018
7042a2a
Renamed unsafe_* to UNSAFE_*
bvaughn Jan 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Removed static optimisticallyPrepareToRender() in favor of render()
  • Loading branch information
bvaughn committed Dec 19, 2017
commit 8618f705485ac1d2c88573e2247ab3e18b2b3fa7
46 changes: 18 additions & 28 deletions text/0000-static-lifecycle-methods.md
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other parts of the React API that have underscores in their naming? Wondering about simply camel-casing:
unsafeComponentWillMount().

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s designed to make you think twice before using it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unstable_deferredUpdates, unstable_batchedUpdates, unstable_AsyncComponent, etc.

// 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.
}
}
```
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.)
Expand All @@ -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);
Copy link

@namuol namuol Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize why this is being done, but should side effects in render really be encouraged for such a common pattern?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully it will be pretty rare, and componentDidMount will work for most use cases.

But I wonder, why don't you begin loading async data in the constructor? Would that be encouraged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant to show this pattern on the RFC, because it's only intended for rare, advanced use cases (eg a library like Relay). I'm a little bummed that it's drawn a bit of negative attention, but I understand why. I should have probably labeled it much more clearly with a disclaimer or something.

That being said, you could trigger the load in the constructor. Side effects in the constructor feel worse to me than side effects in render, but neither is great. Timing wise though, it would be about the same. (render will be called synchronously, shortly after the constructor.)


// Render loading UI...
} else {
// Render real view...
Expand Down Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be ideal IMHO if we could somehow coalesce this use case for initial mount and updating props. It's common for me to pull out a function like this already so that I can call it both during mount and componentWillReceiveProps, because you often need to do the same derivation during mount. I'd argue that's the more common case.

I haven't read through this in its entirety though, maybe that's addressed elsewhere.

There are also some cases (much rarer) where I use an instance variable during this derivation process. So it'd be unfortunate not to have that there, but with async maybe there are pitfalls that I'm not aware of (I should read all of this through).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree I’d like to have this called on initial mount too. Curious to learn more why this was decided against here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As proposed, the name for this method isn't clear about if it will render on mount or not. The componentWillReceiveProps hook seems clear because a component will always initially render with props, so it wouldn't need a life cycle hook. I foresee some confusion with the current name.

Having it run on initial render would be a great feature add. Just today a dev on my team asked why we end up having so much similar logic in ComponentDidMount and ComponentReceiveProps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The componentWillReceiveProps hook seems clear because a component will always initially render with props, so it wouldn't need a life cycle hook. I foresee some confusion with the current name.

It’s clear in retrospect because you already know it. In practice I see engineers expect it to fire on initial mount very often.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's something I wouldn't have guessed. Thanks for the insight

Copy link
Collaborator Author

@bvaughn bvaughn Dec 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting an instance property (state) in a constructor isn't a side effect? Isn't that the only thing a constructor is meant for? Maybe I misunderstanding what you mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I frequently see things in the constructor being side-effectful, like Promises, event emitters, accessing globals etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but initializing state via the constructor (or just as a class property, which is nicer still IMO) isn't a side effect. Again, maybe I'm misunderstanding what you're saying.

Are you suggesting that we shouldn't recommend using the constructor for anything (even state) because people might use it for other things? If so, I don't think I agree.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m suggesting we make the initilization of state pure by returning from a function given some props passed in as arguments. The constructor still needs to exist for side-effectual, non-pure operations such as binding methods of the class, creating class methods as instanve variables and calling super(). It would also really help the compilation effort, as we can evaluate pure functions ahead of time given it has to return the initial state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the benefit of removing state initialization from the constructor if other initialization si done there. How does this help compilation?


### `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

Expand Down