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

Remove react-hot-loader #4494

Closed
rauchg opened this issue May 29, 2018 · 9 comments · Fixed by #4500
Closed

Remove react-hot-loader #4494

rauchg opened this issue May 29, 2018 · 9 comments · Fixed by #4500
Assignees

Comments

@rauchg
Copy link
Member

rauchg commented May 29, 2018

It causes way more issues than it fixes.

It's a matter of removing it from babel options

    plugins: [dev && !isServer && hotLoaderItem, dev && reactJsxSourceItem].filter(Boolean)

becomes

    plugins: [dev && reactJsxSourceItem].filter(Boolean)
@rauchg rauchg added good first issue Easy to fix issues, good for newcomers Priority: High labels May 29, 2018
@timneutkens timneutkens removed the good first issue Easy to fix issues, good for newcomers label May 29, 2018
@timneutkens
Copy link
Member

It’s a little more involved than just removing the babel plugin. Also requires to rewrite the provider component they have to handle errors with componentDidCatch. I’m planning to fix this after a bug fix when using _app.js

@ArmorDarks
Copy link

I wonder, which exactly issues does it cause?

@Everettss
Copy link

Everettss commented May 30, 2018

@ArmorDarks Some changes made in RHL 4.1.0 cause this problem: #4232

timneutkens added a commit that referenced this issue May 31, 2018
@ex3ndr
Copy link
Contributor

ex3ndr commented Jun 16, 2018

What's alternatives? How we should auto-refresh page on changes?

@timneutkens
Copy link
Member

timneutkens commented Jun 16, 2018

Note that react-hot-loader !== hot reloading. Hot reloading still works. The only thing react-hot-loader did was keeping state, but there were too many cases in which it caused weird and unexpected behaviour. For example shouldComponentUpdate being overriden etc.

lependu pushed a commit to lependu/next.js that referenced this issue Jun 19, 2018
@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jun 19, 2018

I confirm, it works like a charm with @material-ui/styles. I have also the feeling that the end-to-end time for hot reloading has greatly improved 🚀 .

@timneutkens
Copy link
Member

Good to hear @oliviertassinari 😌

@msand
Copy link

msand commented Jul 21, 2018

This seems to have decreased hot reloading from a worst case of minutes to seconds for a handful of views, now it seems to perform as good as next.js v4, or even better. And some strange edge cases in development have disappeared. Great work.

@timneutkens
Copy link
Member

Great 👍

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants