[147] Ensure path sanity checks provide clear errors during registrat… #154
Conversation
|
this is an adjusted version of #147 with unit tests added for correctness |
|
@wengeezhang could you check it? |
|
@peterver I suppose it's not good approach make PR from |
lib/router.js
Outdated
| }); | ||
| // Sanity check to ensure we have a viable path candidate (eg: string|regex|non-empty array) | ||
| if ( | ||
| typeof path !== "string" && |
There was a problem hiding this comment.
nit: In project usually use single quotes not double
lib/router.js
Outdated
| this.register(path, methods, middleware, { name }); | ||
| // Sanity check to ensure we have a viable path candidate (eg: string|regex|non-empty array) | ||
| if ( | ||
| typeof path !== "string" && |
There was a problem hiding this comment.
In project usually use single quotes not double
test/lib/router.js
Outdated
| @@ -993,7 +993,30 @@ describe('Router', function () { | |||
| done(); | |||
| }); | |||
| }); | |||
|
|
|||
| it ('correctly returns an error when not passed a path for verb-specific registration (gh-148)', function () { | |||
| const app = new Koa(); | |||
There was a problem hiding this comment.
Hi,
In this test you have some unused variables, you can fix it like this:
const router = new Router();
try {
router.get(function () {});
} catch (e) {
expect(e.message).to.be('You have to provide a path when adding a get handler');
}
test/lib/router.js
Outdated
|
|
||
| it ('correctly returns an error when not passed a path for "all" registration (gh-148)', function () { |
There was a problem hiding this comment.
In this test you have some unused variables, you can fix it like this:
const router = new Router();
try {
router.all(function () {});
} catch (e) {
expect(e.message).to.be('You have to provide a path when adding an all handler');
}
Apologies, first contribution here and want to get going :D I'm used to working in a git-flow approach (develop/master) and wasn't sure about the proper practice here :). Will adhere for future PR's thanks! |
|
We have just published https://github.com/koajs/router/releases/tag/v11.0.0 This project is maintained by Forward Email and Lad. |
…ion checks
Checklist