Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Warning message no longer recommends using deprecated lifecycle method. #12760

Closed
wants to merge 5 commits into from
Closed

Warning message no longer recommends using deprecated lifecycle method. #12760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d9f63e
warning message is now align with new lifecycle paradigm
iliran11 May 7, 2018
1e32c87
Merge remote-tracking branch 'facebook/master'
iliran11 Jun 6, 2018
ab9b01d
updated warning message
iliran11 Jun 6, 2018
6bfbb47
updated another test.
iliran11 Jun 6, 2018
27297b8
prettier
iliran11 Jun 6, 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
7 changes: 3 additions & 4 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,9 @@ describe('ReactCompositeComponent', () => {
expect(() => {
instance = ReactDOM.render(<Component />, container);
}).toWarnDev(
'Cannot update during an existing state transition (such as within ' +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add "(such as within render...)" part back? I don't see why it needs to be changed.

Copy link
Author

Choose a reason for hiding this comment

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

was just thinking it was lengthy.
maybe perhaps instead of printing "Cannot update during an existing state transition" , It is better to be concise and print "Cannot update state during render lifecycle method" ?

"`render` or another component's constructor). Render methods should " +
'be a pure function of props and state; constructor side-effects are ' +
'an anti-pattern, but can be moved to `componentWillMount`.',
'Cannot update the state during an existing state transition. ' +
'Render methods should be a pure function of props and state. ' +
'Either assign the initial state in constructor, or move the setState call to componentDidMount',
);

// The setState call is queued and then executed as a second pass. This
Expand Down
4 changes: 3 additions & 1 deletion packages/react-dom/src/__tests__/ReactUpdates-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,7 +1225,9 @@ describe('ReactUpdates', () => {

const container = document.createElement('div');
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
'Cannot update during an existing state transition',
'Cannot update the state during an existing state transition. ' +
'Render methods should be a pure function of props and state. ' +
'Either assign the initial state in constructor, or move the setState call to componentDidMount',
);
expect(ops).toEqual(['base: 0, memoized: 0', 'base: 1, memoized: 1']);
});
Expand Down
7 changes: 3 additions & 4 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,9 @@ if (__DEV__) {
}
warning(
false,
'Cannot update during an existing state transition (such as within ' +
"`render` or another component's constructor). Render methods should " +
'be a pure function of props and state; constructor side-effects are ' +
'an anti-pattern, but can be moved to `componentWillMount`.',
'Cannot update the state during an existing state transition. ' +
'Render methods should be a pure function of props and state. ' +
'Either assign the initial state in constructor, or move the setState call to componentDidMount',
);
didWarnAboutStateTransition = true;
break;
Expand Down