Skip to content

Commit

Permalink
fix: ensure search and hash in context, location, and lifecycle
Browse files Browse the repository at this point in the history 
Connected to #56
  • Loading branch information
@platosha
platosha committed Sep 27, 2019
1 parent 9059c05 commit 1154d88
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 19 deletions.
39 changes: 24 additions & 15 deletions src/router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
}
});
Expand Down Expand Up @@ -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--) {
Expand Down
59 changes: 58 additions & 1 deletion test/router/lifecycle-events.spec.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -1656,6 +1655,64 @@
await router.render('/client');
expect(outlet.innerHTML.toLowerCase()).to.be.equal('<span><a>layout-link</a><h1>client</h1></span>');
});

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', () => {
Expand Down
58 changes: 55 additions & 3 deletions test/router/router.spec.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'},
Expand Down Expand Up @@ -1214,15 +1248,33 @@
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');

const commandsParam = action.args[0][1];
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() => {
Expand Down

0 comments on commit 1154d88

Please sign in to comment.