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

Proof of concept: simple hot reloading #2304

Closed
wants to merge 19 commits into from

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented May 21, 2017

Couldn't resist. Integrated with our error handling. (So errors cause refresh on next save.)

I only want to enable this for files that export a single functional component defined with JSX.
The goal is not to be comprehensive but to always work well, and create trust.

@Timer
Copy link
Contributor

Timer commented May 23, 2017

Per request, I made the babel plugin. I've never made one before so I dunno if I did things wrong, but it works. 🤷‍♀️

You might need to wipe your babel caches (echo y | ./node_modules/.bin/lerna clean && npm install); they don't play well with hot reloading.
I had to add a safety check in there (typeof data.acceptNext === 'function') which prevents the caches from causing runtime errors when funky things happen.

Turning off cacheDirectory fixes the glitches. Maybe it'll work better with cache-loader.

Here's what I mean by the caches act weird, notice it not swapping back to an older component after save / hard refreshing next:

Works as desired with cacheDirectory off:

function decorateFunctionName(t, name) {
return t.TryStatement(
t.BlockStatement([
t.ExpressionStatement(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simpler to do with "templates".

@Timer
Copy link
Contributor

Timer commented May 23, 2017

Code has been updated to use babel-template and now highlights nodes that are reloaded.

@marcofugaro
Copy link
Contributor

That looks nice! Did you have more time to work on it?

@Timer
Copy link
Contributor

Timer commented Jul 14, 2017

@marcofugaro we're currently waiting on some browser bugs to be fixed.

@rvion
Copy link

rvion commented Aug 13, 2017

we're currently waiting on some browser bugs to be fixed.

still waiting ?

@bestwestern
Copy link

Could someone please explain how to get HMR (even though it may still be a bit buggy)

eirikurn added a commit to aranja/tux that referenced this pull request Nov 16, 2017
We should not support it since by default it's an aggressive and flaky
HMR handler. Where it says it can handle more file changes than it
actually can. Developers usually learn not to trust it and refresh
manually.

It also breaks JS error handling in arrow functions:

gaearon/react-proxy#76

After this change, HMR is still properly configured, including the
module.hot api and client/server communications. By default JS changes
refresh and CSS changes appear without refresh.

For the future, we should monitor the progress of this PR:

facebook/create-react-app#2304
@ro-savage
Copy link
Contributor

Hopefully @aslushnikov will be able to updated on the source map bug in Chrome DevTools. Once thats fixed it should be ready to merge.

@aslushnikov
Copy link

@ro-savage: the fix for crbug.com/438251 has landed and will be available on tomorrow's Canary.

@gaearon
Copy link
Contributor Author

gaearon commented Jan 22, 2018

This is amazing, thank you @aslushnikov!

We should start getting this up to date.

@onpaws
Copy link

onpaws commented Mar 12, 2018

Curious, is this working now?
What needs to happen to close #2407?

Happy to help if someone can please point me in the right direction.

@mnemanja
Copy link

Hi @gaearon, any news regarding the hot reloading integration progress?

@gaearon
Copy link
Contributor Author

gaearon commented May 11, 2018

There are things with higher priority that are currently being worked on and are blockers for v2.
This one is not a blocker. I think it'll get in this year though.

@gaearon gaearon changed the base branch from master to next May 11, 2018 20:08
@Timer Timer changed the base branch from next to master October 19, 2018 15:44
@eddiemonge
Copy link
Contributor

@gaearon this still on track for this year?

@stale
Copy link

stale bot commented Nov 24, 2018

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Nov 24, 2018
@gausie
Copy link

gausie commented Nov 24, 2018

Please bot do not stale 🙏🤖

@stale stale bot removed the stale label Nov 24, 2018
@Timer Timer added this to the 3.0 milestone Nov 24, 2018
@ahmad2smile
Copy link

ahmad2smile commented Nov 28, 2018

Fingers crossed 🤞

@lstkz
Copy link

lstkz commented Mar 26, 2019

This is the most annoying to me "Editing after compile error should still be hot".
Related to #6694

@gaearon
Copy link
Contributor Author

gaearon commented Jun 15, 2019

Ok so I have something you can try.

https://twitter.com/dan_abramov/status/1139876172903395329

https://twitter.com/dan_abramov/status/1139699841204809729 (But use 0.0.5 rather than 0.0.3)

@mrmckeb
Copy link
Contributor

mrmckeb commented Jul 24, 2019

Closing in favour of #5958.

@mrmckeb mrmckeb closed this Jul 24, 2019
@lock lock bot locked and limited conversation to collaborators Jul 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.