Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring the leave hook implementation #266

Merged
merged 2 commits into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ examples
src
jest.setup.js
jsconfig.js
webpack.config.js
webpack.config.js
jest
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 8.6.4

Refactoring the leave hook implementation. The leave hook check now run a bit earlier and once per navigating. So far it was running for each of the matched routes. Also the hook now may accept an array of matches. Not only a single Match object.

## 8.6.2, 8.6.3

Fixing a bug with matching a wildcard.
Expand Down
4 changes: 2 additions & 2 deletions DOCUMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ type RouteHooks = {

* The `before` hook receives two arguments. The first one is a function that needs to be called with either no arguments or `false`. The no-argument version means "move forward". `false` stops the resolving and your handler will not be called.
* `after` is called after your handler
* `leave` is called when you are about to leave out of the route. Similarly to the `before` hook accepts a function as first argument and a [Match](#match) object as second. If the function is called with `false` Navigo will stop resolving the new matched route meaning "we cant' go out of the current route".
* `leave` is called when you are about to leave out of the route. Similarly to the `before` hook accepts a function as first argument and a [Match](#match) object (or an array of [Match](#match) objects) as second. If the function is called with `false` Navigo will stop resolving the new matched route meaning "we cant' go out of the current route".
* `already` is called when this is the current route and it matches again

### Defining hooks for specific route
Expand Down Expand Up @@ -693,7 +693,7 @@ type Route = {
type RouteHooks = {
before?: (done: Function, match: Match) => void;
after?: (match: Match) => void;
leave?: (done: Function, match: Match) => void;
leave?: (done: Function, match: Match | Match[]) => void;
already?: (match: Match) => void;
};
```
Expand Down
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type Match = {
};
export type BeforeHook = (done: Function, match: Match) => void;
export type AfterHook = (match: Match) => void;
export type LeaveHook = (done: Function, match: Match) => void;
export type LeaveHook = (done: Function, match: Match | Match[]) => void;
export type AlreadyHook = (match: Match) => void;
export type RouteHooks = {
before?: BeforeHook;
Expand Down
14 changes: 14 additions & 0 deletions jest/setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const { CustomConsole } = require("@jest/console");

global.console = new CustomConsole(
process.stdout,
process.stderr,
simpleFormatter
);

function simpleFormatter(type, message) {
return message
.split(/\n/)
.map((line) => " " + line)
.join("\n");
}
6 changes: 3 additions & 3 deletions lib/es/lifecycles.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import checkForAlreadyHook from "./middlewares/checkForAlreadyHook";
import checkForNotFoundHandler from "./middlewares/checkForNotFoundHandler";
import errorOut from "./middlewares/errorOut";
import flushCurrent from "./middlewares/flushCurrent";
export var foundLifecycle = [checkForAlreadyHook, checkForLeaveHook, checkForBeforeHook, callHandler, checkForAfterHook];
export var notFoundLifeCycle = [checkForNotFoundHandler, Q["if"](function (_ref) {
export var foundLifecycle = [checkForAlreadyHook, checkForBeforeHook, callHandler, checkForAfterHook];
export var notFoundLifeCycle = [checkForLeaveHook, checkForNotFoundHandler, Q["if"](function (_ref) {
var notFoundHandled = _ref.notFoundHandled;
return notFoundHandled;
}, foundLifecycle, [errorOut, checkForLeaveHook]), flushCurrent];
}, foundLifecycle, [errorOut]), flushCurrent];
4 changes: 0 additions & 4 deletions lib/es/middlewares/callHandler.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { undefinedOrTrue } from "../utils";
export default function callHandler(context, done) {
if (undefinedOrTrue(context.navigateOptions, "updateState")) {
context.instance._setCurrent(context.matches);
}

if (undefinedOrTrue(context.navigateOptions, "callHandler")) {
context.match.route.handler(context.match);
}
Expand Down
13 changes: 8 additions & 5 deletions lib/es/middlewares/checkForLeaveHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,23 @@ export default function checkForLeaveHook(context, done) {
return;
}

Q([].concat(instance.lastResolved().map(function (oldMatch) {
Q(instance.lastResolved().map(function (oldMatch) {
return function (_, leaveLoopDone) {
// no leave hook
if (!oldMatch.route.hooks || !oldMatch.route.hooks.leave) {
leaveLoopDone();
return;
} // no match or different path or callHooks=false
}

var someOfTheLastOnesMatch = context.matches ? context.matches.find(function (match) {
return oldMatch.route.path === match.route.path;
}) : false;

if (undefinedOrTrue(context.navigateOptions, "callHooks") && (!context.match || !instance.matchLocation(oldMatch.route.path, context.match.url))) {
if (undefinedOrTrue(context.navigateOptions, "callHooks") && !someOfTheLastOnesMatch) {
Q(oldMatch.route.hooks.leave.map(function (f) {
// just so we match the Q interface
return function (_, d) {
return f(d, context.match);
return f(d, context.matches && context.matches.length > 0 ? context.matches.length === 1 ? context.matches[0] : context.matches : undefined);
};
}).concat([function () {
return leaveLoopDone();
Expand All @@ -31,7 +34,7 @@ export default function checkForLeaveHook(context, done) {
leaveLoopDone();
}
};
})), {}, function () {
}), {}, function () {
return done();
});
}
15 changes: 8 additions & 7 deletions lib/es/middlewares/processMatches.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ function _extends() { _extends = Object.assign || function (target) { for (var i

import Q from "../Q";
import { foundLifecycle } from "../lifecycles";
import updateState from "./updateState";
import checkForLeaveHook from "./checkForLeaveHook";
export default function processMatches(context, done) {
var idx = 0; // console.log(
// "_processMatches matches=" +
// (context.matches ? context.matches.length : 0)
// );
var idx = 0;

(function nextMatch() {
function nextMatch() {
if (idx === context.matches.length) {
done();
updateState(context, done);
return;
}

Expand All @@ -20,5 +19,7 @@ export default function processMatches(context, done) {
idx += 1;
nextMatch();
});
})();
}

checkForLeaveHook(context, nextMatch);
}
8 changes: 8 additions & 0 deletions lib/es/middlewares/updateState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { undefinedOrTrue } from "../utils";
export default function callHandler(context, done) {
if (undefinedOrTrue(context.navigateOptions, "updateState")) {
context.instance._setCurrent(context.matches);
}

done();
}
62 changes: 43 additions & 19 deletions lib/navigo.amd.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/navigo.amd.js.map

Large diffs are not rendered by default.

Loading