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

Opt-in to disable page reload when using HMR #418

Closed
gaearon opened this issue Aug 21, 2014 · 60 comments
Closed

Opt-in to disable page reload when using HMR #418

gaearon opened this issue Aug 21, 2014 · 60 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Aug 21, 2014

When HMR update fails, it forces a page reload.

This may be desirable for consistency, but most of the time this is caused by a simple syntax error, and I'd like to error to just appear in console. When I fix the error, another HMR update will come and "fix" the system anyway.

When react-hot-loader used a pitch loader, it just put HMR-updated require inside a try-catch.

Now that I refactored it to be a simple loader (no pitching), I no longer have the ability to catch syntax errors in the breaking module.

Can we have a way to disable page reloads altogether? I guess making it opt-in and logging something like "The system is in inconsistent state" is fine with me.

cc @syranide

@syranide
Copy link
Contributor

👍

EDIT FROM SCRATCH: Thought it through a bit more and I think it largely comes down to if you're working with a stateful or stateless project. If you're working with a very stateful project you likely don't want it to reload automatically at all (at least I don't), whereas changes that can be hot-patched should be patched. But if you're working with a rather stateless project you want it to reload all the time, not reloading (when possible) is a bonus.

So I propose an option for whether or not "reloading" is at all allowed.

@sokra
Copy link
Member

sokra commented Aug 21, 2014

What you really want is webpack/webpack-dev-server#42...

@syranide
Copy link
Contributor

@sokra Definitely, but I'm pretty sure I also would want to be able to turn off reloading entirely, I do not want it to reload if I make an irrelevant change to a non-hot-replaceable file.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 22, 2014

I agree; it's also a matter of preference, as most of my team hate auto reloading. They're used to refreshing when they feel like it. For stateful project, auto reloading is quite irritating. For them, HMR is a nice bonus when it's possible, but not at the cost of constantly losing their state due to irrelevant edits.

@jhnns
Copy link
Member

jhnns commented Aug 22, 2014

Agreed. HMR is great (e.g. for editing styles), but auto-reload is not always desired.

@sokra
Copy link
Member

sokra commented Aug 22, 2014

Currently we cannot recover from an exception during hot update, but you can disable refreshing by writing you own HMR management code instead of using the prepared webpack/hot/dev-server (as workaround). Hey, integrate it with your app GUI.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 22, 2014

Bah! I never realized it's that small. OK :-)

@sokra
Copy link
Member

sokra commented Aug 22, 2014

And it's mostly logging ^^

@jhnns
Copy link
Member

jhnns commented Aug 22, 2014

Yep, I've already written my own 😀

@syranide
Copy link
Contributor

For posterity, this is what I use for the moment:

function check() {
  module.hot.check(function(err, updatedModules) {
    if (updatedModules) {
      check();
    }
  });
}
window.onmessage = function(event) {
  if (event.data === 'webpackHotUpdate' && module.hot.status() === 'idle') {
    check();
  }
};

@gaearon
Copy link
Contributor Author

gaearon commented Aug 22, 2014

I have similar code.

The only problem so far is that after one failed update (that I ignore inside my server), updates can't be applied anymore.

Apparently module.hot.status() becomes fail after the first update and I don't see how to make it recover.

@jhnns
Copy link
Member

jhnns commented Aug 25, 2014

So you don't restart your app on failure (like with window.location.reload())?

@gaearon
Copy link
Contributor Author

gaearon commented Aug 25, 2014

@jhnns

I don't—it's inconvenient to lose all state due to a simple typo. I may have style modifications or useful logs in Chrome devtools that will be lost because of that.

@jhnns
Copy link
Member

jhnns commented Aug 26, 2014

That's true, but unless you have stateless views it's really hard to just swap out modules. Seems like one of your updated modules didn't provide an update handler via module.hot.accept and thus can't handle the update. For example take a look at the style-loader.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 26, 2014

@jhnns Not sure I'm following you.

unless you have stateless views it's really hard to just swap out modules

But it already works! This is precisely the problem react-hot-loader is solving (for React views).

I just don't want an occasional typo to either reload the page or disable HMR. I want the typo to be ignored till next HMR.

@sokra
Copy link
Member

sokra commented Aug 26, 2014

It possible to catch the syntax errors in the accept handlers...

Example:

// Accepting another module
module.hot.accept("module", function() {
  try {
    require("module");
  } catch(e) {
    // Some fallback
  }
});
// Accepting self
module.hot.accept(function(err) {
  // Some fallback
});

@syranide
Copy link
Contributor

@jhnns There are tons of cases where it will "fall out of sync" and error in practice, we're not "arguing" against that. But if I spot a minor mistake or grammatical error in my getTimeLeft-utility function while working on something entirely different, I don't want everything to reload, I just want it to keep running as-is.

If I add/change/remove a non-hot-updateable require, then sure, stop hot updates and tell me (please don't reload!). But if I make an insignificant change in a non-hot-replaceable file, please just ignore it and continue as normal.

To put it differently, assume that style-loader didn't support hot-replacing, I wouldn't want it to reload just because I added/modified/removed styles, I want it to keep working as-is without the style changes, because they're mostly irrelevant. When I want to preview the new styles, I reload manually.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 26, 2014

@sokra

The first example is how I used to do it before I rewrote the loader to avoid pitch stage. It allowed me to catch the error.

In your second example, how would you prevent HMR entering "fail" stage and refusing to accept future updates?

@sokra
Copy link
Member

sokra commented Aug 26, 2014

It only goes into fail for unhandled exceptions. Once you have an exception handler it continues as normal...

@gaearon
Copy link
Contributor Author

gaearon commented Aug 26, 2014

Ah, so there's a difference between accept() and accept(noop)? I didn't realize that..

@sokra
Copy link
Member

sokra commented Aug 26, 2014

accept() goes into fail when the require throws.
accept(noop) ignores the exception from the require.
accept(function(err) { console.log(err) }) prints the exception but doesn't go into fail.
accept(function(err) { console.log(err); throw err }) prints the exception and does into fail.

@jhnns
Copy link
Member

jhnns commented Aug 26, 2014

@sokra do your examples refer to "accepting itself" or "accepting another module"?
@syranide but if you don't want your app to be reloaded, why do you use HMR?

@gaearon
Copy link
Contributor Author

gaearon commented Aug 26, 2014

@jhnns

I think these examples refer to accepting itself—I'll give it a try as it seems to solve my problem (although this issue is about webpack-wide support for disabling page reload).

As for why @syranide uses HMR, I think it's for the same reasons that I do: with react-hot-loader you get no-refresh hot reload for React components (like in this video). The whole point is avoiding refreshing.

@syranide
Copy link
Contributor

@jhnns I see HMR as an awesome feature to avoid having to do lots of changes in the blind and then reload, not as a feature to reduce the time between visual updates (by avoiding reloads). I think neither is wrong, but the second makes no sense to my workflow for complex apps; I do not want to throw away all my app state because I made an insignificant change that I don't need to preview right now.

I.e. if I'm working on something mostly visual and stateless, please just reload for every change. But if I'm working on stateful application code then I absolutely don't want it to reload unless I say so, I accept the consequences of making potentially incompatible changes and reloading myself when necessary.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 26, 2014

@sokra Thanks, that's what I did: gaearon/react-hot-loader@f3485dc

@jhnns
Copy link
Member

jhnns commented Aug 26, 2014

@syranide I think you're right... maybe that's a better default behavior than reloading the app. How should the feedback to the developer then look like

@sokra what do you think?

@syranide
Copy link
Contributor

@jhnns It's covered by the "copy-pasteable" server code, that's where the reloading takes place. It's easy enough to disable reload and/or tailor to your own liking, but module.hot.status() === 'abort' so no further updates are accepted and applied. So if I make a change that cannot be hot-updated, then no further hot-updateable changes will be applied until reload. Simply being able to ignore non-hot-updateable changes sounds fine to me (if it breaks horribly when requires are added/moved/removed, that's fine, if it could be detected then that's great but not at all important IMHO).

@sokra sokra closed this as completed in b8fef9a Aug 29, 2014
@syranide
Copy link
Contributor

@sokra You're the best! 👍

@gaearon
Copy link
Contributor Author

gaearon commented Sep 3, 2014

I see! Really appreciate you watching this thread and making changes to bring us hot reloading nirvana.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 4, 2014

I just tried NoErrorsPlugin with hot/only-dev-server and it is officially awesome.
Works like magic.
Thank you.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 18, 2014

@sokra Just wanted to thank you for this again. This hugely improved workflow for our designer who spends a lot of time in CSS and JSX and often makes small syntax errors. He rarely reloads the page now and iterates much faster. Also thanks to @syranide and @chanon for advocating this!

@sokra
Copy link
Member

sokra commented Sep 18, 2014

@gaearon 😄

@gaearon
Copy link
Contributor Author

gaearon commented May 22, 2015

It seems like I've been doing a silly thing for a long time. If you use React Hot Loader with hot/only-dev-server, there's no need for NoErrorsPlugin. If you make a syntax error, it'll print it in the console, but the next reload will still work:

screen shot 2015-05-23 at 1 23 05

Which is awesome because with NoErrorsPlugin it just prints "Nothing hot updated." which can be misleading when you have an error. (Yeah I know I was misusing it, sorry!)

@gaearon
Copy link
Contributor Author

gaearon commented May 22, 2015

Oh well, I was wrong. HMR breaks if you don't use NoErrorsPlugin and misspell a dependency name. Bummer. I'll probably create an issue because I'm not sure why it happens this way.

@sdtsui
Copy link

sdtsui commented Aug 4, 2016

Just wanted to say this discussion is awesome, helped me understand webpack better, and saved me a few hours at work. Thanks all.

@koddo
Copy link

koddo commented Oct 13, 2016

The NoErrorsPlugin doesn't work for me. When I have an error in my code, the webpack still tries to send something to the browser, like http://localhost:3000/b206461c30a0ae51ec9c.hot-update.json.
But this file doesn't exist and I have historyApiFallback=true, so the server just sends the index.html, then the browser says Update check failed: SyntaxError: Unexpected token < in JSON at position 0 at XMLHttpRequest.request.onreadystatechange.
And after that the HMR stops working.

@koddo
Copy link

koddo commented Oct 15, 2016

I'm not alone having the issue with the noErrorsPlugin: webpack/webpack-dev-server#655

When using the noErrorsPlugin, after making an error and then fixing it, module.hot.status() returns "check", when it should return "idle". So, basically it gets stuck somewhere, I can't figure out.

@mikeengland
Copy link

@koddo sadly the fix for this issue only went into Webpack 2 beta. I do understand bug fixes going into 2 so it gets out the door more quickly but I am reliant on webpack v1 and so are a lot of people who feel more comfortable on a stable version.

@mikeengland
Copy link

I haven't looked at the commit that fixes it in version 2, so I'm not sure how complex backporting the fix would be.

@koddo
Copy link

koddo commented Oct 16, 2016

@mikeengland, I've seen it, and that would be hard, for sure. But my monkey patch works for me. Will use it until v2.

@ghost
Copy link

ghost commented Jun 20, 2017

I know it's immature but I temporarily solved it by placing a return in the beginning of reloadApp() :P

@gaearon
Copy link
Contributor Author

gaearon commented May 7, 2019

Four years later, @sokra showed me a solution that seems to do exactly what I wanted:

function hotErrorHandler(err) {
  require.cache[module.id].hot.accept(hotErrorHandler);
}
module.hot.accept(hotErrorHandler);

😄

@gaearon
Copy link
Contributor Author

gaearon commented May 8, 2019

@sokra
Copy link
Member

sokra commented May 8, 2019

It's never too late...

@nealoke
Copy link

nealoke commented Aug 12, 2019

@gaearon I've been trying for a couple of hours to get this working but I can't. Is there no way to have an onError handler for the code below?

if (module.hot) {
	module.hot.accept("./server", () => {
		serverInstance.close();
		serverInstance = createServer();
		return serverInstance;
	});
}

@nealoke
Copy link

nealoke commented Aug 12, 2019

For people trying to figure out how to correctly catch the error on a node server. It basically tries to import the updated file and when it doesn't work simply logs the error and carries on like normal.

let serverInstance = require("./server").createServer();

if (module.hot) {
	module.hot.accept("./server", () => {
		try {
			const updatedImport = require("./server");
			serverInstance.close();
			serverInstance = updatedImport.createServer();
		}
		catch (err) {
			console.log(err);
		}
	});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants