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

Component inheritance hot reload #9

Merged
merged 1 commit into from
Feb 28, 2015

Conversation

BenoitZugmeyer
Copy link
Contributor

Now that components are plain JS classes, it can inherit from other components.

This fix ensure the actual component prototype is patched for hot reload, and not one of its parent.

If the prototype inherits from another component prototype, the
__isAssimilatedByReactHotAPI property may already been defined in the
parent prototype. Thus, we need to make sure this property is owned by
the current prototype.
@gaearon
Copy link
Owner

gaearon commented Feb 28, 2015

It's been a few years and I'm still dumb and don't understand prototypes fully.
Can you please explain to me why this helps?

@BenoitZugmeyer
Copy link
Contributor Author

Okay. Let's assume you have a Page component as a base and a Login component inheriting from Page.

In es3 we could write something like this (this is a basic example, things are a little more complex than that in reality but it illustrates how it works):

function Page() {}
Page.prototype = new Component(); // the React.Component class

function Login() {}
Login.prototype = new Page();

If I set a property on Page.prototype, the Login.prototype (which is a Page instance, see) will inherit the property.

Page.prototype.foo = 42;
console.log(Login.prototype.foo); // 42

So when you set the __isAssimilatedByReactHotAPI property on Page.prototype, Login.prototype will have it too, by inheritance.

Thus, we need to check if this property is the own property of the prototype (= not an inherited property).

console.log(Page.prototype.hasOwnProperty('foo')) // true
console.log(Login.prototype.hasOwnProperty('foo')) // false

Finally, to be a little paranoid, I didn't use hasOwnProperty directly because it could be overriden by another method defined by the class. So I used the Object.prototype.hasOwnProperty directly, which should always be what we want.

gaearon added a commit that referenced this pull request Feb 28, 2015
@gaearon gaearon merged commit 6ff6006 into gaearon:master Feb 28, 2015
@gaearon
Copy link
Owner

gaearon commented Feb 28, 2015

Oh. I get it now, thanks!
Will release in a couple of days.

@gaearon
Copy link
Owner

gaearon commented Mar 9, 2015

On a related note, you might want to check if react-hot-loader@1.2.0 works better with inheritance chains. Hot-reloading changes in base class should work now.

@BenoitZugmeyer
Copy link
Contributor Author

Thanks, I'll give it a try. FYI, I am using react-hot-api directly (without the -loader), to reload react components in an atom-shell application. You might be interested by this use case. See https://github.com/BenoitZugmeyer/chwitt-react/blob/master/bootstrap-jsx.js

@gaearon
Copy link
Owner

gaearon commented Mar 9, 2015

Wow, that's pretty impressive. I didn't know that! Thanks for sharing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants