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

Wrap render method created using class properties. #850

Closed
wants to merge 2 commits into from
Closed

Wrap render method created using class properties. #850

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b84c7cd
Wrap render method created using class properties.
arunoda Jan 21, 2017
0ed6fbe
Remove unwated codebase.
arunoda Jan 21, 2017
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
13 changes: 12 additions & 1 deletion client/patch-react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ export default (handleError = () => {}) => {
const { prototype } = Component
if (prototype && prototype.render) {
prototype.render = wrapRender(prototype.render)
} else if (prototype && prototype.constructor) {
// Still a React component instance, but there's no render method in
Copy link
Contributor

Choose a reason for hiding this comment

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

btw there may be render but might override it on constructor.

class Foo extends React.Component {
  render () {}
}
class Bar extends Foo {
  render = () => {};
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we do this on the componentWillMount. I assume this is fine.
Isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, we don't monkeypatch componentWillMount if prototype.render exists.

// prototype. This happens when the render method created with class-properties.
// With this fix, we'll wrap the render method in runtime when the component initialized
const originalComponentWillMount = prototype.componentWillMount
prototype.componentWillMount = function (...args) {
if (originalComponentWillMount) {
originalComponentWillMount.apply(this, args)
}

this.render = wrapRender(this.render)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should redefine constructor instead of using componentDidMount ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm quite not sure about that. This constructor function is something generated by babel. I just used that to detect a React component.

} else {
// stateless component
Component = wrapRender(Component)
Expand Down Expand Up @@ -54,7 +66,6 @@ export default (handleError = () => {}) => {

// copy all properties
Object.assign(_render, render)

render.__wrapped = _render.__wrapped = _render

return _render
Expand Down