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.

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 (2) #856

Merged
merged 3 commits into from
Jan 22, 2017
Merged
Changes from 1 commit
Commits
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
65 changes: 49 additions & 16 deletions client/patch-react.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,23 @@ export default (handleError = () => {}) => {
React.createElement = function (Component, ...rest) {
if (typeof Component === 'function') {
const { prototype } = Component
if (prototype && prototype.render) {
prototype.render = wrapRender(prototype.render)

// assumes it's a class component if render method exists.
const isClassComponent = !!(prototype && prototype.render) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Using !! is seems pretty weird. Either don't use them or use Boolean(xxx)

// subclass of React.Component or PureComponent with no render method.
prototype instanceof React.Component ||
prototype instanceof React.PureComponent
Copy link
Member

Choose a reason for hiding this comment

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

Will this not make it work with class properties and alt-reacts like Preact? cc @developit

Copy link
Member

Choose a reason for hiding this comment

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

I guess the React compat could make the instanceof pass, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The alias will make this work, yup.


if (isClassComponent) {
// There's no render method in prototype
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment correct? (or in the correct place?)

// when it's created with class-properties.
if (prototype.render) {
prototype.render = wrapRender(prototype.render)
}

// wrap the render method in runtime when the component initialized
// for class-properties.
Component = wrap(Component, withWrapOwnRender)
} else {
// stateless component
Component = wrapRender(Component)
Expand All @@ -39,24 +54,42 @@ export default (handleError = () => {}) => {
}

function wrapRender (render) {
if (render.__wrapped) {
return render.__wrapped
}
return wrap(render, withHandleError)
}

const _render = function (...args) {
try {
return render.apply(this, args)
} catch (err) {
handleError(err)
return null
}
function withHandleError (fn, ...args) {
try {
return fn.apply(this, args)
} catch (err) {
handleError(err)
return null
}
}

// copy all properties
Object.assign(_render, render)
function withWrapOwnRender (fn, ...args) {
const result = fn.apply(this, args)
if (this.render && this.hasOwnProperty('render')) {
this.render = wrapRender(this.render)
}
return result
}
}

render.__wrapped = _render.__wrapped = _render
function wrap (fn, around) {
if (fn.__wrapped) {
return fn.__wrapped
}

return _render
const _fn = function (...args) {
return around.call(this, fn, ...args)
}

// copy all properties
Object.assign(_fn, fn)

_fn.prototype = fn.prototype

_fn.__wrapped = fn.__wrapped = _fn

return _fn
}