-
Notifications
You must be signed in to change notification settings - Fork 26
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
Polyfill for getSnapshotBeforeUpdate #1
Changes from 7 commits
b0688e3
ab7a7ab
366211f
d1ac34c
ce78769
a00cf2e
668cbdf
16aa7fe
ff70f44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,27 @@ function componentWillReceiveProps(nextProps) { | |
} | ||
} | ||
|
||
function componentWillUpdate(nextProps, nextState) { | ||
try { | ||
var prevProps = this.props; | ||
var prevState = this.state; | ||
this.props = nextProps; | ||
this.state = nextState; | ||
this.__reactInternalSnapshot = this.getSnapshotBeforeUpdate( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the kind of thing for which a Have you tried at least making it non-enumerable, or even maybe also a polyfill-internal Symbol instead of a string name, or are those also out when supporting 0.14? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that's necessary. React already stores other "__reactInternal" attributes on class components. And relying on a symbol might cause problems for older browsers (supported by older versions of React) that don't support symbols. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice if a future version of React would use a |
||
prevProps, | ||
prevState | ||
); | ||
} finally { | ||
this.props = prevProps; | ||
this.state = prevState; | ||
} | ||
} | ||
|
||
// React may warn about cWM/cWRP/cWU methods being deprecated. | ||
// Add a flag to suppress these warnings for this special case. | ||
componentWillMount.__suppressDeprecationWarning = true; | ||
componentWillReceiveProps.__suppressDeprecationWarning = true; | ||
componentWillUpdate.__suppressDeprecationWarning = true; | ||
|
||
module.exports = function polyfill(Component) { | ||
if (!Component.prototype || !Component.prototype.isReactComponent) { | ||
|
@@ -38,18 +55,49 @@ module.exports = function polyfill(Component) { | |
|
||
if (typeof Component.getDerivedStateFromProps === 'function') { | ||
if (typeof Component.prototype.componentWillMount === 'function') { | ||
throw new Error('Cannot polyfill if componentWillMount already exists'); | ||
throw new Error( | ||
'Cannot polyfill getDerivedStateFromProps() for components that define componentWillMount()' | ||
); | ||
} else if ( | ||
typeof Component.prototype.componentWillReceiveProps === 'function' | ||
) { | ||
throw new Error( | ||
'Cannot polyfill if componentWillReceiveProps already exists' | ||
'Cannot polyfill getDerivedStateFromProps() for components that define componentWillReceiveProps()' | ||
); | ||
} | ||
|
||
Component.prototype.componentWillMount = componentWillMount; | ||
Component.prototype.componentWillReceiveProps = componentWillReceiveProps; | ||
} | ||
|
||
if (typeof Component.prototype.getSnapshotBeforeUpdate === 'function') { | ||
if (typeof Component.prototype.componentWillUpdate === 'function') { | ||
throw new Error( | ||
'Cannot polyfill getSnapshotBeforeUpdate() for components that define componentWillUpdate()' | ||
); | ||
} | ||
if (typeof Component.prototype.componentDidUpdate !== 'function') { | ||
throw new Error( | ||
'Cannot polyfill getSnapshotBeforeUpdate() for components that do not define componentDidUpdate()' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add "on the prototype" to these messages? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
); | ||
} | ||
|
||
Component.prototype.componentWillUpdate = componentWillUpdate; | ||
|
||
var componentDidUpdate = Component.prototype.componentDidUpdate; | ||
|
||
Component.prototype.componentDidUpdate = function componentDidUpdatePolyfill( | ||
prevProps, | ||
prevState | ||
) { | ||
componentDidUpdate.call( | ||
this, | ||
prevProps, | ||
prevState, | ||
this.__reactInternalSnapshot | ||
); | ||
}; | ||
} | ||
|
||
return Component; | ||
}; |
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.
Won't direct assignments to these warn?
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.
No. React sets/updates this values in a similar way before calling lifecycles.