React-hot-loader: React Hot Loader 3.0 might prevent views updates

[loicfrering]

Views are no more updated when we connect a container to an externally defined simple component. See this commit loicfrering/redux-devtools@d387bc9b6dd517e91f6d4c1008490cd4b55720a9 for reference.

When I start the app, actions are correctly dispatched (as we can see in the devtools) but the views are not updating anymore.

And when I remove react-hot-loader/patch from webpack.config.js, everything is working as expected.

Tested with react-hot-loader 3.0.0-alpha.13. I'll try to investigate more over the weekend, thanks!

[gaearon]

Can you share the exact sequence of steps? I tried your commit, and I’m able to update every component.

[loicfrering]

I just checked and reproduced it on another machine with these exact steps:

1. git clone https://github.com/loicfrering/redux-devtools.git
2. cd redux-devtools
3. npm install (linting fails for some reason)
4. cd examples/todomvc
5. npm install
6. npm start

Then when I navigate to http://localhost:3000 I get the behavior I described: actions are correctly dispatched as shown by the devtools but views are not updated, the new todo is not added to the list.

Please let me know if I can help you with more details, thanks!

[gaearon]

Oh, I thought you were saying that views are not updated when you edit them.
Thanks for reporting, this definitely sounds like a bug.

[gaearon]

It’s https://github.com/reactjs/react-redux/issues/163#issuecomment-192556637 showing its head all over again. 😞

[gaearon]

Okay, I’ve got a solution. We need to get rid of setting _source and do a global call instead (e.g. __REACT_HOT_LOADER_MAP__.set(uniqueID, type). This would both solve this problem and (I think) the router issues.

[nfcampos]

@gaearon I think a better solution might be to keep it as is but instead of setting __source we set [Symbol.for('rhl-source')]. That will never get hoisted so it's safe

[gaearon]

We can’t assume the user has Symbols. Also this fixes issues with React Router because we don’t need createElement to run. I think this would be a better solution, at least until browsers provide an equivalent of _source.

[nfcampos]

We don't need createElement to run? Then how do we know which of the tagged functions/exports are react components and which are just regular functions?
Nuno Campos

_____________________________

From: Dan Abramov [email protected]
Sent: Sunday, May 1, 2016 2:40 PM
Subject: Re: [gaearon/react-hot-loader] React Hot Loader 3.0 might prevent views updates (#266)
To: gaearon/react-hot-loader [email protected]
Cc: Comment [email protected], Nuno Campos nuno.[email protected]

We can’t assume the user has Symbols. Also this fixes issues with React Router because we don’t need createElement to run. I think this would be a better solutions, at least not until browsers provide an equivalent of _source.

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

[nfcampos]

Also, is it a problem for a development tool to depend on the existence of Symbol?

[gaearon]

We call .set() from module definition so _I think_ the new type will be in the map even if the router calls createElement with previous types. The problem was that new types weren’t getting createElement-ed, but with approach in #272, it doesn’t matter whether you call it with previous or new version because we have entries in idsByComponent for both.

[gaearon]

Also, is it a problem for a development tool to depend on the existence of Symbol?

Not a big problem but a good thing to avoid when easily possible. But this is not the main reason why Symbol doesn't seem like a solution to me. React Redux could copy symbol properties too, if it wanted—this is still easy to break. And the bigger issue is the problems with stale types that #272 so neatly avoids. The logic in resolveType was really brittle and full of edge cases but now it’s easy to grok. I think it’s an important improvement. (And RR support too.)

[nfcampos]

I see. Yes that sounds like it will work :) (And yes I forgot Object.getOwnPropertySymbols existed)

Nuno Campos

_____________________________

From: Dan Abramov [email protected]
Sent: Sunday, May 1, 2016 2:48 PM
Subject: Re: [gaearon/react-hot-loader] React Hot Loader 3.0 might prevent views updates (#266)
To: gaearon/react-hot-loader [email protected]
Cc: Comment [email protected], Nuno Campos nuno.[email protected]

Also, is it a problem for a development tool to depend on the existence of Symbol?

Not a big problem but a good thing to avoid when easily possible. But this is not the main reason why Symbol doesn't seem like a solution to me. React Redux could copy symbol properties too, if it wanted—this is still easy to break. And the bigger issue is the problems with stale types that #272 so neatly avoids. The logic in resolveType was really brittle and full of edge cases but now it’s easy to grok. I think it’s an important improvement. (And RR support too.)

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

[gaearon]

Fixed in v3.0.0-beta.1.

[loicfrering]

Thanks @gaearon for your fantastic work!

[gaearon]

Really appreciate that you reported this and provided easy instructions to reproduce!