Feature: useBeforeUnload by default for browser history

[jamiehodge]

In order to enable useBeforeUnload, one currently needs to import createHistory and useBeforeUnload from history. If I'm not mistaken, listening for unload events would be a sane default for browser history, so why not change the browserHistory export to some variation of useBeforeUnload(createHistory)()?

[taion]

Is there any cost to attaching a no-op unload listener that we don't use?

This would be a breaking change, anyway.

[timdorr]

It doesn't attach the listener until the first listenBeforeUnload hook is added. So, no cost that I see.

[timdorr]

Also, is it a breaking change? It only adds an extra API and doesn't change the original history behavior. In fact, it really looks usable without history at all. useBeforeUnload(() => ({}))() should work just fine.

[taion]

The change is that leave hooks will now always trigger on hard-leaving the page, which is not the current behavior.

[timdorr]

Oh, I didn't even know we handled that.

I don't think it's a huge change. What are most people using onLeave for? I don't use it myself.

[taion]

Route leave hooks, not onLeave.

[timdorr]

Well, it looks like our example and this doc are wrong/incomplete without this being added.

I can't think of a reason not to add it by default.

[taion]

Well, the example works. It just only works when staying within the router, and not for e.g. hard leaving the page or closing the tab.

[agundermann]

beforeunload isn't totally free of side effects. It disables BFCache (Firefox, IE). This could actually be a good thing though, since BFCache can break the router and it seems like we can't support BFCache in a clean way using pageshow.

[taion]

I'd be fine either way, but I'm not convinced we can make this change without a major version bump.

[timdorr]

@taurose Thanks for that insight. Yeah, this sounding more like a straight up improvement.

@taion At worst, it's a minor bump. We're simply improving an API to cover more cases of "leaving" a route. There is no backwards incompatible change. Existing leave hooks will continue to fire on the same events as always.

[taion]

I'm not totally convinced, but I don't care enough either way.

[timdorr]

Closing as per #3118