From 1154d88f8cf68bf122378716bfeff6f4ed4fd386 Mon Sep 17 00:00:00 2001 From: Anton Platonov Date: Thu, 26 Sep 2019 15:51:38 +0300 Subject: [PATCH] fix: ensure search and hash in context, location, and lifecycle Connected to #56 --- src/router.js | 39 ++++++++++------- test/router/lifecycle-events.spec.html | 59 +++++++++++++++++++++++++- test/router/router.spec.html | 58 +++++++++++++++++++++++-- 3 files changed, 137 insertions(+), 19 deletions(-) diff --git a/src/router.js b/src/router.js index 7537d273..51ac6fe9 100644 --- a/src/router.js +++ b/src/router.js @@ -442,14 +442,21 @@ export class Router extends Resolver { */ render(pathnameOrContext, shouldUpdateHistory) { const renderId = ++this.__lastStartedRenderId; - const { - pathname, - search, - hash - } = isString(pathnameOrContext) ? {pathname: pathnameOrContext, search: '', hash: ''} : pathnameOrContext; + const context = Object.assign( + { + search: '', + hash: '' + }, + isString(pathnameOrContext) + ? {pathname: pathnameOrContext} + : pathnameOrContext, + { + __renderId: renderId + } + ); // Find the first route that resolves to a non-empty result - this.ready = this.resolve({pathname, search, hash, __renderId: renderId}) + this.ready = this.resolve(context) // Process the result of this.resolve() and handle all special commands: // (redirect / prevent / component). If the result is a 'component', @@ -505,11 +512,11 @@ export class Router extends Resolver { .catch(error => { if (renderId === this.__lastStartedRenderId) { if (shouldUpdateHistory) { - this.__updateBrowserHistory({pathname, search, hash}); + this.__updateBrowserHistory(context); } removeDomNodes(this.__outlet && this.__outlet.children); - this.location = createLocation({pathname, resolver: this}); - fireRouterEvent('error', {router: this, error, pathname}); + this.location = createLocation(Object.assign(context, {resolver: this})); + fireRouterEvent('error', Object.assign({router: this, error}, context)); throw error; } }); @@ -607,24 +614,26 @@ export class Router extends Resolver { // Skip re-attaching and notifications if element and chain do not change newContext.__skipAttach = - // Same route chain - newChain.length === previousChain.length && newContext.__divergedChainIndex == newChain.length && - // Same element - this.__isReusableElement(newContext.result, previousContext.result); + // Same route chain + newChain.length === previousChain.length && newContext.__divergedChainIndex == newChain.length && + // Same element + this.__isReusableElement(newContext.result, previousContext.result); if (newContext.__skipAttach) { // execute onBeforeLeave for changed segment element when skipping attach for (let i = newChain.length - 1; i >= 0; i--) { - if (previousChain[i].path !== newChain[i].path) { + if (previousChain[i].path !== newChain[i].path || newContext.search !== previousContext.search) { callbacks = this.__runOnBeforeLeaveCallbacks(callbacks, newContext, {prevent}, previousChain[i]); } } // execute onBeforeEnter for changed segment element when skipping attach for (let i = 0; i < newChain.length; i++) { - if (previousChain[i].path !== newChain[i].path) { + if (previousChain[i].path !== newChain[i].path || newContext.search !== previousContext.search) { callbacks = this.__runOnBeforeEnterCallbacks(callbacks, newContext, {prevent, redirect}, newChain[i]); } + previousChain[i].element.location = createLocation(newContext, previousChain[i].route); } + } else { // execute onBeforeLeave when NOT skipping attach for (let i = previousChain.length - 1; i >= newContext.__divergedChainIndex; i--) { diff --git a/test/router/lifecycle-events.spec.html b/test/router/lifecycle-events.spec.html index 2c802364..73a728fe 100644 --- a/test/router/lifecycle-events.spec.html +++ b/test/router/lifecycle-events.spec.html @@ -1618,7 +1618,6 @@ ]); }); - it('should not remove layout contents when it is reused (#392)', async() => { // A reusable layout with some content const layout = document.createElement('span'); @@ -1656,6 +1655,64 @@ await router.render('/client'); expect(outlet.innerHTML.toLowerCase()).to.be.equal('layout-link

client

'); }); + + it('should skip lifecycle when path remains same and search string remains empty', async() => { + router.setRoutes([ + {path: '/a', component: 'x-spy'} + ], true); + + // Pathname only means empty search string + await router.render('/a'); + const view = outlet.children[0]; + + callbacksLog = []; + + // No search in context means empty search string + await router.render({pathname: '/a'}); + expect(outlet.children[0]).to.be.equal(view); + + // Search remains empty, skip lifecycle + verifyCallbacks([ + ]); + + // Explicit empty search string + await router.render({pathname: '/a', search: ''}); + expect(outlet.children[0]).to.be.equal(view); + + // Search remains empty, skip lifecycle + verifyCallbacks([ + ]); + }); + + it('should call lifecycle when path remains same and search string changes', async() => { + router.setRoutes([ + {path: '/a', component: 'x-spy'} + ], true); + + // Pathname only means empty search string + await router.render('/a'); + const view = outlet.children[0]; + + callbacksLog = []; + await router.render({pathname: '/a', search: '?foo=bar'}); + expect(outlet.children[0]).to.be.equal(view); + + // Search string changed, call short lifecycle without reattach + verifyCallbacks([ + 'x-spy.onBeforeLeave', + 'x-spy.onBeforeEnter', + ]); + + callbacksLog = []; + await router.render({pathname: '/a', search: '?foo=baz'}); + expect(outlet.children[0]).to.be.equal(view); + + // Search string changed again, call short lifecycle without reattach + verifyCallbacks([ + 'x-spy.onBeforeLeave', + 'x-spy.onBeforeEnter', + ]); + }); }); describe('lifecycle events with async action', () => { diff --git a/test/router/router.spec.html b/test/router/router.spec.html index ad03d339..59fa8c4a 100644 --- a/test/router/router.spec.html +++ b/test/router/router.spec.html @@ -1121,14 +1121,48 @@ ]); await router.render({ pathname: '/non-existent/path', - search: 'foo=bar', + search: '?foo=bar', hash: '#baz' }); expect(outlet.children[0]).to.have.deep.property('location.pathname', '/non-existent/path'); - expect(outlet.children[0]).to.have.deep.property('location.search', 'foo=bar'); + expect(outlet.children[0]).to.have.deep.property('location.search', '?foo=bar'); expect(outlet.children[0]).to.have.deep.property('location.hash', '#baz'); }); + it('should update the `location` properties on the route component', async() => { + router.setRoutes([ + {path: '(.*)', component: 'x-not-found-view'}, + ]); + await router.render({ + pathname: '/non-existent/path', + search: '?foo=bar', + hash: '#baz' + }); + await router.render({ + pathname: '/non-existent/path', + search: '?foo=qux', + hash: '#quz' + }); + expect(outlet.children[0]).to.have.deep.property('location.pathname', '/non-existent/path'); + expect(outlet.children[0]).to.have.deep.property('location.search', '?foo=qux'); + expect(outlet.children[0]).to.have.deep.property('location.hash', '#quz'); + }); + + it('should update the `location` properties on the route component on router.render(pathname)', async() => { + router.setRoutes([ + {path: '(.*)', component: 'x-not-found-view'}, + ]); + await router.render({ + pathname: '/non-existent/path', + search: '?foo=bar', + hash: '#baz' + }); + await router.render('/non-existent/path'); + expect(outlet.children[0]).to.have.deep.property('location.pathname', '/non-existent/path'); + expect(outlet.children[0]).to.have.deep.property('location.search', ''); + expect(outlet.children[0]).to.have.deep.property('location.hash', ''); + }); + it('should keep route parameters when redirecting to different route', async() => { router.setRoutes([ {path: '/', component: 'x-home-view'}, @@ -1214,8 +1248,10 @@ expect(action).to.have.been.called.once; expect(action.args[0].length).to.equal(2); - const contextParam = action.args[0][0]; + let contextParam = action.args[0][0]; expect(contextParam.pathname).to.equal('/'); + expect(contextParam.search).to.equal(''); + expect(contextParam.hash).to.equal(''); expect(contextParam.route.path).to.equal('/'); expect(contextParam.route.component).to.equal('x-home-view'); @@ -1223,6 +1259,22 @@ expect(commandsParam.prevent).to.be.an('undefined'); expect(commandsParam.redirect).to.be.a('function'); expect(commandsParam.component).to.be.a('function'); + + action.reset(); + await router.render({pathname: '/'}); + + contextParam = action.args[0][0]; + expect(contextParam.pathname).to.equal('/'); + expect(contextParam.search).to.equal(''); + expect(contextParam.hash).to.equal(''); + + action.reset(); + await router.render({pathname: '/', search: '?foo=bar', hash: '#baz'}); + + contextParam = action.args[0][0]; + expect(contextParam.pathname).to.equal('/'); + expect(contextParam.search).to.equal('?foo=bar'); + expect(contextParam.hash).to.equal('#baz'); }); it('action.this points to the route that defines action', async() => {