Commit d8ac7666 authored by Robert Knight's avatar Robert Knight

Ignore "popstate" event delivered by browser on page load

Fix an issue where the route was set before the initial profile and
groups fetch had completed, which could result in the WebSocket
connection being created and then immediately torn down and re-created
on startup.

The route was set too soon because Safari and Chrome emit a "popstate"
event on page load, which triggered a call to `router.sync()` before the
call to `sync()` made by `setupRoute` in src/sidebar/index.js after the
initial profile and groups load.

The solution is to only add the "popstate" listener after the initial
call to `sync()`.
parent 55356682
......@@ -61,13 +61,31 @@ export default function router($window, store) {
return url;
}
let didRegisterPopstateListener = false;
/**
* Synchronize the route name and parameters in the store with the current
* URL.
*
* The first call to this method also registers a listener for future back/forwards
* navigation in the browser.
*/
function sync() {
const { route, params } = currentRoute();
store.changeRoute(route, params);
// Setup listener for back/forward navigation. We do this in `sync()` to avoid
// the route being changed by eg. a "popstate" emitted by the browser on
// document load (which Safari and Chrome do).
if (!didRegisterPopstateListener) {
$window.addEventListener('popstate', () => {
// All the state we need to update the route is contained in the URL, which
// has already been updated at this point, so just sync the store route
// to match the URL.
sync();
});
didRegisterPopstateListener = true;
}
}
/**
......@@ -81,13 +99,5 @@ export default function router($window, store) {
sync();
}
// Handle back/forward navigation.
$window.addEventListener('popstate', () => {
// All the state we need to update the route is contained in the URL, which
// has already been updated at this point, so just sync the store route
// to match the URL.
sync();
});
return { sync, navigate };
}
......@@ -101,7 +101,9 @@ describe('router', () => {
context('when a browser history navigation happens', () => {
fixtures.forEach(({ path, search, route, params }) => {
it('updates the active route in the store', () => {
createService();
const svc = createService();
svc.sync();
fakeStore.changeRoute.resetHistory();
updateUrl(path, search);
fakeWindow.emit('popstate');
......@@ -109,5 +111,17 @@ describe('router', () => {
assert.calledWith(fakeStore.changeRoute, route, params);
});
});
it('does nothing if the initial call to `sync()` has not happened', () => {
createService();
// Simulate a "popstate" event being triggered by the browser before
// the app is ready to initialize the router by calling `sync()`.
//
// Safari and Chrome do this when the page loads. Firefox does not.
fakeWindow.emit('popstate');
assert.notCalled(fakeStore.changeRoute);
});
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment