-
Notifications
You must be signed in to change notification settings - Fork 50.6k
RFC 6: Deprecate unsafe lifecycles #12028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
e709e30
404944e
bd300b3
2868176
8679926
64f27d7
1047182
035c220
8d0e001
b71ca93
09c39d0
286df77
b699543
68f2fe7
2d9f75d
8f125b7
1d3e3d5
d95ec49
b940938
6cd0a8e
7572667
361a2cf
8178d52
8d67e27
53770c3
3772ee2
4dfa6e1
5609031
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Also added a new set of tests focused on server side lifecycle hooks.
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| /** | ||
| * Copyright (c) 2013-present, Facebook, Inc. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| * @emails react-core | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils'); | ||
|
|
||
| let React; | ||
| let ReactDOMServer; | ||
|
|
||
| function initModules() { | ||
| // Reset warning cache. | ||
| jest.resetModuleRegistry(); | ||
| React = require('react'); | ||
| ReactDOMServer = require('react-dom/server'); | ||
|
|
||
| // Make them available to the helpers. | ||
| return { | ||
| ReactDOMServer, | ||
| }; | ||
| } | ||
|
|
||
| const {resetModules} = ReactDOMServerIntegrationUtils(initModules); | ||
|
|
||
| describe('ReactDOMServerLifecycles', () => { | ||
| beforeEach(() => { | ||
| resetModules(); | ||
| }); | ||
|
|
||
| it('should invoke the correct lifecycle hooks', () => { | ||
| const log = []; | ||
|
|
||
| class Outer extends React.Component { | ||
| unsafe_componentWillMount() { | ||
| log.push('outer componentWillMount'); | ||
| } | ||
| render() { | ||
| log.push('outer render'); | ||
| return <Inner />; | ||
| } | ||
| } | ||
|
|
||
| class Inner extends React.Component { | ||
| unsafe_componentWillMount() { | ||
| log.push('inner componentWillMount'); | ||
| } | ||
| render() { | ||
| log.push('inner render'); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| ReactDOMServer.renderToString(<Outer />); | ||
| expect(log).toEqual([ | ||
| 'outer componentWillMount', | ||
| 'outer render', | ||
| 'inner componentWillMount', | ||
| 'inner render', | ||
| ]); | ||
| }); | ||
|
|
||
| it('should warn about deprecated lifecycle hooks', () => { | ||
| class Component extends React.Component { | ||
| componentWillMount() {} | ||
| render() { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| expect(() => ReactDOMServer.renderToString(<Component />)).toWarnDev( | ||
| 'Warning: Component: componentWillMount() is deprecated and will be removed ' + | ||
| 'in the next major version. Please use unsafe_componentWillMount() instead.', | ||
| ); | ||
| }); | ||
|
|
||
| it('should update instance.state with value returned from getDerivedStateFromProps', () => { | ||
| class Grandparent extends React.Component { | ||
| state = { | ||
| foo: 'foo', | ||
| }; | ||
| render() { | ||
| return ( | ||
| <div> | ||
| {`Grandparent: ${this.state.foo}`} | ||
| <Parent /> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class Parent extends React.Component { | ||
| state = { | ||
| bar: 'bar', | ||
| baz: 'baz', | ||
| }; | ||
| static getDerivedStateFromProps(props, prevState) { | ||
| return { | ||
| bar: `not ${prevState.bar}`, | ||
| }; | ||
| } | ||
| render() { | ||
| return ( | ||
| <div> | ||
| {`Parent: ${this.state.bar}, ${this.state.baz}`} | ||
| <Child />; | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class Child extends React.Component { | ||
| static getDerivedStateFromProps() { | ||
| return { | ||
| qux: 'qux', | ||
| }; | ||
| } | ||
| render() { | ||
| return `Child: ${this.state.qux}`; | ||
| } | ||
| } | ||
|
|
||
| const markup = ReactDOMServer.renderToString(<Grandparent />); | ||
| expect(markup).toContain('Grandparent: foo'); | ||
| expect(markup).toContain('Parent: not bar, baz'); | ||
| expect(markup).toContain('Child: qux'); | ||
| }); | ||
|
|
||
| it('should warn if getDerivedStateFromProps returns undefined', () => { | ||
| class Component extends React.Component { | ||
| static getDerivedStateFromProps() {} | ||
| render() { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| expect(() => ReactDOMServer.renderToString(<Component />)).toWarnDev( | ||
| 'Component.getDerivedStateFromProps(): A valid state object (or null) must ' + | ||
| 'be returned. You may have returned undefined.', | ||
| ); | ||
|
|
||
| // De-duped | ||
| ReactDOMServer.renderToString(<Component />); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,6 +123,7 @@ let didWarnDefaultTextareaValue = false; | |
| let didWarnInvalidOptionChildren = false; | ||
| const didWarnAboutNoopUpdateForComponent = {}; | ||
| const didWarnAboutBadClass = {}; | ||
| const didWarnAboutUndefinedDerivedState = {}; | ||
| const valuePropNames = ['value', 'defaultValue']; | ||
| const newlineEatingTags = { | ||
| listing: true, | ||
|
|
@@ -421,6 +422,33 @@ function resolve( | |
|
|
||
| if (shouldConstruct(Component)) { | ||
| inst = new Component(element.props, publicContext, updater); | ||
|
|
||
| if (typeof Component.getDerivedStateFromProps === 'function') { | ||
| partialState = Component.getDerivedStateFromProps( | ||
| element.props, | ||
| inst.state, | ||
| ); | ||
|
|
||
| if (__DEV__) { | ||
| if (partialState === undefined) { | ||
| const componentName = getComponentName(Component) || 'Unknown'; | ||
|
|
||
| if (!didWarnAboutUndefinedDerivedState[componentName]) { | ||
| warning( | ||
| false, | ||
| '%s.getDerivedStateFromProps(): A valid state object (or null) must be returned. ' + | ||
| 'You may have returned undefined.', | ||
|
||
| componentName, | ||
| ); | ||
| didWarnAboutUndefinedDerivedState[componentName] = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (partialState != null) { | ||
| inst.state = Object.assign({}, inst.state, partialState); | ||
| } | ||
| } | ||
| } else { | ||
| if (__DEV__) { | ||
| if ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be
null, right? Do we want to pass{}in this case for type safety?I don't remember what this debate settled on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be
null, yup. I assume this was desirable since it can be used to detect that the state has not yet been initialized. Alternately if you've initialized the state via the constructor it will be whatever that value us.