From aa684c9127a0eedaf820eec619897be224677dcc Mon Sep 17 00:00:00 2001 From: Mark <1446916+markcellus@users.noreply.github.com> Date: Sun, 30 May 2021 18:08:09 -0400 Subject: [PATCH] Fix back button render issue Resolves an issue where pressing back button after clicking on a new route caused previous route not to be cleared from DOM. --- src/router-component.ts | 19 +++---- tests/router-component-tests.ts | 92 ++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 44 deletions(-) diff --git a/src/router-component.ts b/src/router-component.ts index 2d47c9e..f5ac923 100644 --- a/src/router-component.ts +++ b/src/router-component.ts @@ -73,12 +73,14 @@ export class RouterComponent extends HTMLElement { if (!triggerRouteChange) { delete state.triggerRouteChange; } - this.previousLocation = { ...this.location }; method.call(history, state, title, url); - - if (triggerRouteChange) { - this.showRoute(url); + if (!triggerRouteChange) { + return; + } + if (this.previousLocation) { + this.hideRoute(this.previousLocation.pathname); } + this.showRoute(url); }; } this.showRoute(this.getFullPathname(this.location)); @@ -154,7 +156,7 @@ export class RouterComponent extends HTMLElement { if (!location) return; const [pathname, hashString] = location.split('#'); const routeElement = this.getRouteElementByPath(pathname); - + this.previousLocation = { ...this.location }; if (!routeElement) { return console.warn( `Navigated to path "${pathname}" but there is no matching element with a path ` + @@ -316,10 +318,9 @@ export class RouterComponent extends HTMLElement { private async popStateChanged() { const path = this.getFullPathname(this.location); - if (this.location.href !== this.previousLocation.href) { - this.hideRoute(this.previousLocation.pathname); - } - this.showRoute(path); + // although popstate was called we still need to trigger + // replaceState so all stateful operations can be performed + window.history.replaceState({}, document.title, path); } private setupElement(routeElement: Element) { diff --git a/tests/router-component-tests.ts b/tests/router-component-tests.ts index 130147d..f7000a3 100644 --- a/tests/router-component-tests.ts +++ b/tests/router-component-tests.ts @@ -1,5 +1,5 @@ import sinon from 'sinon'; -import { expect, fixture, html } from '@open-wc/testing'; +import { expect, fixture, html, nextFrame } from '@open-wc/testing'; // eslint-disable-next-line no-unused-vars import { extractPathParams, RouterComponent } from '../src/router-component'; @@ -98,40 +98,6 @@ describe('', async () => { expect(document.body.querySelector('second-page')).to.be.null; }); - it('switches to the child that has the path that matches the current location after popstate has been called', async () => { - await fixture(html` - - - - - `); - - window.history.pushState({}, document.title, '/page1'); - window.history.pushState({}, document.title, '/page2'); - const popstate = new PopStateEvent('popstate', { state: {} }); - window.dispatchEvent(popstate); - expect(document.body.querySelector('first-page')).to.be.null; - expect(document.body.querySelector('second-page')).to.not.be.null; - }); - - it('shows a warning when attempting to go to a route that is not handled after popstate is called', async () => { - const component: RouterComponent = await fixture(html` - - - - - `); - - window.history.pushState({}, document.title, '/page1'); - const newPath = 'nope'; - consoleWarn.resetHistory(); - component.show(newPath); - expect(consoleWarn.args[0]).to.deep.equal([ - `Navigated to path "${newPath}" but there is no matching ` + - `element with a path that matches. Maybe you should implement a catch-all route with the path attribute of ".*"?`, - ]); - }); - it('shows the child whose path matches the catch all url', async () => { await fixture(html` @@ -382,6 +348,62 @@ describe('', async () => { expect(disconnectedCallbackSpy.callCount).to.equal(1); }); + describe('when popstate is triggered', () => { + it('switches to the child that has the path that matches the current location', async () => { + await fixture(html` + + + + + `); + + window.history.pushState({}, document.title, '/page1'); + window.history.pushState({}, document.title, '/page2'); + const popstate = new PopStateEvent('popstate', { state: {} }); + window.dispatchEvent(popstate); + expect(document.body.querySelector('first-page')).to.be.null; + expect(document.body.querySelector('second-page')).to.not.be.null; + }); + + it('shows a warning when attempting to go to a route that is not handled', async () => { + const component: RouterComponent = await fixture(html` + + + + + `); + + window.history.pushState({}, document.title, '/page1'); + const newPath = 'nope'; + consoleWarn.resetHistory(); + component.show(newPath); + expect(consoleWarn.args[0]).to.deep.equal([ + `Navigated to path "${newPath}" but there is no matching ` + + `element with a path that matches. Maybe you should implement a catch-all route with the path attribute of ".*"?`, + ]); + }); + + it('removes previous route element after clicking back button', async () => { + await fixture(html` + + + To page 2 + + + + `); + + window.history.pushState({}, document.title, '/page1'); + const firstPageLink: HTMLAnchorElement = + document.querySelector('first-page a'); + firstPageLink.click(); // go to second + window.history.back(); + await nextFrame(); + expect(document.body.querySelector('first-page')).to.not.be.null; + expect(document.body.querySelector('second-page')).to.be.null; + }); + }); + describe('when pushState or replaceState is overridden', () => { const origPushState = window.history.pushState; const origReplaceState = window.history.replaceState;