Permalink
Browse files

fix: Prevent running CanDeactivatePreviousStep twice in same navigation

To handle this, I've added a new parameter to the router called `couldDeactivate` which is set to true whenever the CanDeactivatePreviousStep completes and is reset on every navigation. If a redirect is returned from the CanActivateNextStep, then this value will be true and it is used to build a special pipeline that skips the CanDeactivatePreviousStep on the next pipeline run.

fixes #45
  • Loading branch information...
davismj committed Aug 15, 2018
1 parent c6c4832 commit 54b3654fbd2cde5c165d72e2c199d87f0b8a3843
Showing with 38 additions and 21 deletions.
  1. +3 −1 src/activation.js
  2. +2 −1 src/app-router.js
  3. +6 −2 src/pipeline-provider.js
  4. +6 −0 src/router.js
  5. +21 −17 test/activation.spec.js
@@ -25,7 +25,7 @@ export class ActivateNextStep {
}
}
function processDeactivatable(navigationInstruction: NavigationInstruction, callbackName: string, next: Funcion, ignoreResult: boolean) {
function processDeactivatable(navigationInstruction: NavigationInstruction, callbackName: string, next: Function, ignoreResult: boolean) {
const plan = navigationInstruction.plan;
let infos = findDeactivatable(plan, callbackName);
let i = infos.length; //query from inside out
@@ -49,6 +49,8 @@ function processDeactivatable(navigationInstruction: NavigationInstruction, call
}
}
navigationInstruction.router.couldDeactivate = true;
return next();
}
@@ -156,7 +156,7 @@ export class AppRouter extends Router {
throw new Error('Maximum navigation attempts exceeded. Giving up.');
}
let pipeline = this.pipelineProvider.createPipeline();
let pipeline = this.pipelineProvider.createPipeline(!this.couldDeactivate);
return pipeline
.run(instruction)
@@ -230,6 +230,7 @@ function resolveInstruction(instruction, result, isInnerInstruction, router) {
router.isNavigatingRefresh = false;
router.isNavigatingForward = false;
router.isNavigatingBack = false;
router.couldDeactivate = false;
let eventName;
@@ -52,9 +52,13 @@ export class PipelineProvider {
/**
* Create the navigation pipeline.
*/
createPipeline(): Pipeline {
createPipeline(useCanDeactivateStep: boolean = true): Pipeline {
let pipeline = new Pipeline();
this.steps.forEach(step => pipeline.addStep(this.container.get(step)));
this.steps.forEach(step => {
if (useCanDeactivateStep || step !== CanDeactivatePreviousStep) {
pipeline.addStep(this.container.get(step));
}
});
return pipeline;
}
@@ -84,6 +84,11 @@ export class Router {
*/
isNavigatingRefresh: boolean;
/**
* True if the previous instruction successfully completed the CanDeactivatePreviousStep in the current navigation.
*/
couldDeactivate: boolean;
/**
* The currently active navigation tracker.
*/
@@ -150,6 +155,7 @@ export class Router {
this.isNavigatingRefresh = false;
this.isNavigatingForward = false;
this.isNavigatingBack = false;
this.couldDeactivate = false;
this.navigation = [];
this.currentInstruction = null;
this.viewPortDefaults = {};
@@ -19,42 +19,46 @@ describe('activation', () => {
};
};
let instructionFactory = (instruction) => {
return Object.assign({ router: {} }, instruction);
}
beforeEach(() => {
step = new CanDeactivatePreviousStep();
state = createPipelineState();
});
it('should return true for context that canDeactivate', () => {
let instruction = { plan: { first: viewPortFactory(() => (true)) } };
let instruction = instructionFactory({ plan: { first: viewPortFactory(() => (true)) } });
step.run(instruction, state.next);
expect(state.result).toBe(true);
});
it('should return true for context that canDeactivate with activationStrategy.replace', () => {
let instruction = { plan: { first: viewPortFactory(() => (true), activationStrategy.replace) } };
let instruction = instructionFactory({ plan: { first: viewPortFactory(() => (true), activationStrategy.replace) } });
step.run(instruction, state.next);
expect(state.result).toBe(true);
});
it('should cancel for context that cannot Deactivate', () => {
let instruction = { plan: { first: viewPortFactory(() => (false)) } };
let instruction = instructionFactory({ plan: { first: viewPortFactory(() => (false)) } });
step.run(instruction, state.next);
expect(state.rejection).toBeTruthy();
});
it('should return true for context that cannot Deactivate with unknown strategy', () => {
let instruction = { plan: { first: viewPortFactory(() => (false), 'unknown') } };
let instruction = instructionFactory({ plan: { first: viewPortFactory(() => (false), 'unknown') } });
step.run(instruction, state.next);
expect(state.result).toBe(true);
});
it('should return true for context that canDeactivate with a promise', (done) => {
let instruction = { plan: {first: viewPortFactory(() => (Promise.resolve(true))) } };
let instruction = instructionFactory({ plan: {first: viewPortFactory(() => (Promise.resolve(true))) } });
step.run(instruction, state.next).then(() => {
expect(state.result).toBe(true);
@@ -63,7 +67,7 @@ describe('activation', () => {
});
it('should cancel for context that cantDeactivate with a promise', (done) => {
let instruction = { plan: {first: viewPortFactory(() => (Promise.resolve(false))) } };
let instruction = instructionFactory({ plan: {first: viewPortFactory(() => (Promise.resolve(false))) } });
step.run(instruction, state.next).then(() => {
expect(state.rejection).toBeTruthy();
@@ -72,7 +76,7 @@ describe('activation', () => {
});
it('should cancel for context that throws in canDeactivate', (done) => {
let instruction = { plan: {first: viewPortFactory(() => { throw new Error('oops'); }) } };
let instruction = instructionFactory({ plan: {first: viewPortFactory(() => { throw new Error('oops'); }) } });
step.run(instruction, state.next).then(() => {
expect(state.rejection).toBeTruthy();
@@ -81,30 +85,30 @@ describe('activation', () => {
});
it('should return true when all plans return true', () => {
let instruction = { plan: { first: viewPortFactory(() => (true)), second: viewPortFactory(() => (true))} };
let instruction = instructionFactory({ plan: { first: viewPortFactory(() => (true)), second: viewPortFactory(() => (true))} });
step.run(instruction, state.next);
expect(state.result).toBe(true);
});
it('should cancel when some plans return false', () => {
let instruction = { plan: {first: viewPortFactory(() => (true)), second: viewPortFactory(() => (false))} };
let instruction = instructionFactory({ plan: {first: viewPortFactory(() => (true)), second: viewPortFactory(() => (false))} });
step.run(instruction, state.next);
expect(state.rejection).toBeTruthy();
});
it('should pass a navigationInstruction to the callback function', () => {
const instruction = { plan: { first: viewPortFactory(() => (true)) } };
const instruction = instructionFactory({ plan: { first: viewPortFactory(() => (true)) } });
const viewModel = instruction.plan.first.prevComponent.viewModel;
spyOn(viewModel, 'canDeactivate').and.callThrough();
step.run(instruction, state.next);
expect(viewModel.canDeactivate).toHaveBeenCalledWith(instruction);
expect(viewModel.canDeactivate).toHaveBeenCalledWith(instruction);
});
describe('with a childNavigationInstruction', () => {
let viewPort = viewPortFactory(() => (true));
let instruction = { plan: { first: viewPort } };
let instruction = instructionFactory({ plan: { first: viewPort } });
describe('when navigating on the parent', () => {
@@ -116,18 +120,18 @@ describe('activation', () => {
it('should return true when the currentInstruction can deactivate', () => {
let viewPort = viewPortFactory(() => (true), activationStrategy.replace);
let currentInstruction = { viewPortInstructions: { first: viewPortInstructionFactory(() => (true)) } };
let currentInstruction = instructionFactory({ viewPortInstructions: { first: viewPortInstructionFactory(() => (true)) } });
viewPort.prevComponent.childRouter = { currentInstruction };
let instruction = { plan: { first: viewPort } };
let instruction = instructionFactory({ plan: { first: viewPort } });
step.run(instruction, state.next);
expect(state.result).toBe(true);
});
it('should cancel when router instruction cannot deactivate', () => {
let viewPort = viewPortFactory(() => (true), activationStrategy.replace);
let currentInstruction = { viewPortInstructions: { first: viewPortInstructionFactory(() => (false)) } };
let currentInstruction = instructionFactory({ viewPortInstructions: { first: viewPortInstructionFactory(() => (false)) } });
viewPort.prevComponent.childRouter = { currentInstruction };
let instruction = { plan: { first: viewPort } };
let instruction = instructionFactory({ plan: { first: viewPort } });
step.run(instruction, state.next);
expect(state.rejection).toBeTruthy();
});
@@ -204,7 +208,7 @@ describe('activation', () => {
done();
}
}
instruction.addViewPortInstruction('my-view-port', 'ignored', 'ignored', { viewModel });
step.run(instruction, state.next);
});

0 comments on commit 54b3654

Please sign in to comment.