mobxjs / mobx-react

React bindings for MobX
https://mobx.js.org/react-integration.html
MIT License
4.85k stars 350 forks source link

mobx-react observer broken react-hot-loader #833

Closed wangkechun closed 4 years ago

wangkechun commented 4 years ago

demo: https://github.com/gaearon/react-hot-loader/pull/1418/files pull this github mr and edit examples/typescript/src/Counter.tsx:32 react-hot-loader will not work

ref: https://github.com/gaearon/react-hot-loader/issues/1419

danielkcz commented 4 years ago

Can you help out a bit more? I never used hot loader and I don't know specifics how exactly it works and what is the expected outcome. Can you look at the observer code if you see something suspicious? We are hacking the sCU there, does the hot loader need it somehow?

https://github.com/mobxjs/mobx-react/blob/9b2ffb1d7534c7148339fc1b2180374f0228ccc3/src/observerClass.ts#L19

Edit: Possibly related https://github.com/mobxjs/mobx-react/issues/500

danielkcz commented 4 years ago

Also can you please try it out with mobx-react@5? There was some major refactor with V6, so would be great to know if we have some regression or it's a new issue.

mweststrate commented 4 years ago

FWIW I personally never supported react-hot-loader, as it has too many issues, especially in combination and would be an unending support burden (just reasoning about what should be the correct behavior can already be very tricky in cases). My hopes of the new fast-refresh mechanism are slightly better a it seem to behave much more consistently with known supported and unsupported cases.

genaby commented 4 years ago

@WearyMonkey @FredyC Looks like it works in following way (at least for me): <Observer>{() => <YourComponent/>}</Observer>

@mweststrate for me react-hot-loader provides significant boost in development

danielkcz commented 4 years ago

I think we can close it here. Unless someone can come up with specifics, it doesn't seem to be worth the effort. In my opinion, hot reloading is overrated. There are much better tools for quick development, be it Storybook for designing components or actual tests when verifying logic.

BEGEMOT9I commented 4 years ago

@mweststrate @FredyC Hi. I have the same issue. As a reference, I took the example proposed by @wangkechun. I've tested PureComponent, Component and mobx-react@5.2.8. And none of this works. But functional-component working is properly. Hack with <Observer>{() => <YourComponent/>}</Observer> working properly too.

danielkcz commented 4 years ago

Can you perhaps try react-fast-refresh instead? It was introduced into React Native first where it seems to be working and now it should be available for React as well.

Either way, please open a new issue if it doesn't turn out to be working either.