POC: createElement patch + prototype methods patch

[nfcampos]

open questions that i can think of:

• still needs some way of assigning unique IDs to react components (for the POC i did that manually)
• still have to test whether this works for components wrapped in a HOC it appears to work
• patching React.createElement doesn't play well with code like this https://github.com/reactjs/react-router/blob/master/modules/RouterContext.js#L29 (unless there's some foolproof way of ensuring the patch happens before any line like that is ran) Making it work with react router currently requires providing the createElement prop like this <Router history={browserHistory} createElement={React.createElement}> (and that still doesn't work if route components are not classes, which I think is related to the issue below)
• for a tree composed only of SFCs I have no idea how to forceUpdate it after hot reloading, since I can't get a ref to it

notes:

• I ignored components components created with React.createClass() but they shouldn't be any different
• I was surprised by this but it appears to work on components that are not module exports, not sure it works always though because I wasn't really expecting this (basically it works because even though you might not have changed the render method of the exported component, it gets replaced and the replacement holds references to the new non-exported components, which themselves get replaced in te call to React.createElement()

[nfcampos]

@gaearon have you had any chance to look at this?

[gaearon]

Nope, no chance yet.
But I will!

On 2 Apr 2016, at 10:09, Nuno Campos notifications@github.com wrote:

@gaearon have you had any chance to look at this?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[gaearon]

Can you remind me why we’re patching manually rather than via react-proxy?

[pepjo]

Not sure if I should post this here;

I've tried to manually copy your code into my fresh new project that uses redux and react-router. And I get this error on the browser console when I modify any file:

[HMR] bundle rebuilding
client.js:126 [HMR] bundle rebuilt in 642ms
process-update.js:27 [HMR] Checking for updates on the server...
browser.jsx:121 update Root
process-update.js:115 [HMR] Cannot check for update (Full reload needed)
process-update.js:116 [HMR] TypeError: Cannot read property '_reactInternalInstance' of null
    at deepForceUpdate (webpack:///./~/react-deep-force-update/lib/index.js?:39:34)
    at eval (webpack:///./src/browser.jsx?:186:42)
    at Object.hotApply [as apply] (http://localhost:5000/scripts/main.js:485:16)
    at cb (webpack:///(webpack)-hot-middleware/process-update.js?:52:36)
    at hotUpdateDownloaded (http://localhost:5000/scripts/main.js:301:13)
    at hotAddUpdateChunk (http://localhost:5000/scripts/main.js:273:13)
    at webpackHotUpdateCallback (http://localhost:5000/scripts/main.js:5:12)
    at http://localhost:5000/scripts/0.6055dcaab5a09d633ed0.hot-update.js:1:1

Maybe it's related to your point:

• patching React.createElement doesn't play well with code like this https://github.com/reactjs/react-router/blob/master/modules/RouterContext.js#L29 (unless there's some foolproof way of ensuring the patch happens before any line like that is ran)

since I'm using the Router I mean, or it's not, do you have any clue why this happens?

[nfcampos]

@pepjo I'd really recommend not using this anywhere, it hasn't been tested at all. But it sounds like maybe your root component is not a class component

[nfcampos]

@gaearon we're patching manually because we had this idea that just replacing the methods on the prototype would be enough (and would be simpler than going through react-proxy). It does appear to work but since it requires patching the bind method of every method maybe it's not that much simpler and it's just reinventing the wheel
The above is true for class components. for SFCs I should have just used react-proxy actually (if it supports SFCs?), instead of rolling my own proxy

[pepjo]

@nfcampos It's just a project I'm messing with, nothing serious :P
You were right! My top component was a pure function, my bad 😞
Now it works great with classes and functions unless they are anywhere down inside a react router route. If they are I get this:

browser.js:49 Warning: [react-router] You cannot change <Router routes>; it will be ignored

I'm guessing thats what you ment when you said that createElement was messing with react router.

Anyway, thanks for this (y)

[nfcampos]

@pepjo yeah this still only works for really simple toy setups, eventually it might come to work in real apps

[gaearon]

I forgot about overriding bind(), my bad.
Thanks for reminding!

[gaearon]

Hey, thanks for your work. I played with this for a while but ultimately I went with react-proxy since it’s covered by tests and it was easier to ensure I had the exact same behavior I wanted. That said, huge thanks for encouraging me to use an ID rather than a method call. This made things so much easier. React Hot Loader 3 tags functions with __source: { fileName, localName } and it works well. You can try it and learn more here: gaearon/react-hot-boilerplate#61.

[nfcampos]

Thanks for looking at it. I've started to play around with RHL 3 and I've left an issue here gaearon/react-hot-loader#240