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

fix: support router.verb(method, path, controllerString) #12

Merged
merged 2 commits into from
Jun 16, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ jobs:
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
with:
os: 'ubuntu-latest'
version: '18.7.0, 18, 20, 22'
version: '18.19.0, 18, 20, 22'
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@

### ⚠ BREAKING CHANGES

* drop Node.js < 18.7.0 support
* drop Node.js < 18.19.0 support

- Drop generator function support
- Drop Node.js < 18.7.0 support
- Drop Node.js < 18.19.0 support
Comment on lines +15 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify the nature of the breaking changes for better understanding.

- * drop Node.js < 18.19.0 support
- - Drop generator function support
- - Drop Node.js < 18.19.0 support
+ * Dropped support for Node.js versions below 18.19.0
+ - Removed support for generator functions

This edit ensures that each breaking change is distinctly mentioned and avoids repetition. It also clarifies that support for generator functions has been removed, not just deprecated.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* drop Node.js < 18.19.0 support
- Drop generator function support
- Drop Node.js < 18.7.0 support
- Drop Node.js < 18.19.0 support
* Dropped support for Node.js versions below 18.19.0
- Removed support for generator functions
Tools
Markdownlint

17-17: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style


18-18: Expected: asterisk; Actual: dash (MD004, ul-style)
Unordered list style


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
Expand Down
21 changes: 20 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Router core component for [Egg.js](https://github.com/eggjs).
- [Tests](#tests)
- [Breaking changes on v3](#breaking-changes-on-v3)
- [License](#license)
- [Contributors](#contributors)

<a name="exp_module_egg-router--Router"></a>

Expand Down Expand Up @@ -450,8 +451,26 @@ Run tests using `npm test`.
## Breaking changes on v3

- Drop generator function support
- Drop Node.js < 18.7.0 support
- Drop Node.js < 18.19.0 support
fengmk2 marked this conversation as resolved.
Show resolved Hide resolved

## License

[MIT](LICENSE)

<!-- GITCONTRIBUTOR_START -->

## Contributors

|[<img src="https://avatars.githubusercontent.com/u/1112718?v=4" width="100px;"/><br/><sub><b>alexmingoia</b></sub>](https://github.com/alexmingoia)<br/>|[<img src="https://avatars.githubusercontent.com/u/1413330?v=4" width="100px;"/><br/><sub><b>jbielick</b></sub>](https://github.com/jbielick)<br/>|[<img src="https://avatars.githubusercontent.com/u/985607?v=4" width="100px;"/><br/><sub><b>dead-horse</b></sub>](https://github.com/dead-horse)<br/>|[<img src="https://avatars.githubusercontent.com/u/156269?v=4" width="100px;"/><br/><sub><b>fengmk2</b></sub>](https://github.com/fengmk2)<br/>|[<img src="https://avatars.githubusercontent.com/u/1024246?v=4" width="100px;"/><br/><sub><b>wachunei</b></sub>](https://github.com/wachunei)<br/>|[<img src="https://avatars.githubusercontent.com/u/160197?v=4" width="100px;"/><br/><sub><b>dominicbarnes</b></sub>](https://github.com/dominicbarnes)<br/>|
| :---: | :---: | :---: | :---: | :---: | :---: |
|[<img src="https://avatars.githubusercontent.com/u/25254?v=4" width="100px;"/><br/><sub><b>tj</b></sub>](https://github.com/tj)<br/>|[<img src="https://avatars.githubusercontent.com/u/166834?v=4" width="100px;"/><br/><sub><b>aheckmann</b></sub>](https://github.com/aheckmann)<br/>|[<img src="https://avatars.githubusercontent.com/u/385716?v=4" width="100px;"/><br/><sub><b>kilianc</b></sub>](https://github.com/kilianc)<br/>|[<img src="https://avatars.githubusercontent.com/u/98955?v=4" width="100px;"/><br/><sub><b>secretfader</b></sub>](https://github.com/secretfader)<br/>|[<img src="https://avatars.githubusercontent.com/u/474587?v=4" width="100px;"/><br/><sub><b>ilkkao</b></sub>](https://github.com/ilkkao)<br/>|[<img src="https://avatars.githubusercontent.com/u/6873217?v=4" width="100px;"/><br/><sub><b>HeavenDuke</b></sub>](https://github.com/HeavenDuke)<br/>|
|[<img src="https://avatars.githubusercontent.com/u/2842176?v=4" width="100px;"/><br/><sub><b>XadillaX</b></sub>](https://github.com/XadillaX)<br/>|[<img src="https://avatars.githubusercontent.com/u/200876?v=4" width="100px;"/><br/><sub><b>yiminghe</b></sub>](https://github.com/yiminghe)<br/>|[<img src="https://avatars.githubusercontent.com/u/32174276?v=4" width="100px;"/><br/><sub><b>semantic-release-bot</b></sub>](https://github.com/semantic-release-bot)<br/>|[<img src="https://avatars.githubusercontent.com/u/6794386?v=4" width="100px;"/><br/><sub><b>vkhv</b></sub>](https://github.com/vkhv)<br/>|[<img src="https://avatars.githubusercontent.com/u/7627362?v=4" width="100px;"/><br/><sub><b>vikramdurai</b></sub>](https://github.com/vikramdurai)<br/>|[<img src="https://avatars.githubusercontent.com/u/9271565?v=4" width="100px;"/><br/><sub><b>Tankenstein</b></sub>](https://github.com/Tankenstein)<br/>|
|[<img src="https://avatars.githubusercontent.com/u/2822996?v=4" width="100px;"/><br/><sub><b>richardprior</b></sub>](https://github.com/richardprior)<br/>|[<img src="https://avatars.githubusercontent.com/u/1635441?v=4" width="100px;"/><br/><sub><b>joesonw</b></sub>](https://github.com/joesonw)<br/>|[<img src="https://avatars.githubusercontent.com/u/875091?v=4" width="100px;"/><br/><sub><b>ifroz</b></sub>](https://github.com/ifroz)<br/>|[<img src="https://avatars.githubusercontent.com/u/13130706?v=4" width="100px;"/><br/><sub><b>jeynish</b></sub>](https://github.com/jeynish)<br/>|[<img src="https://avatars.githubusercontent.com/u/72027?v=4" width="100px;"/><br/><sub><b>jergason</b></sub>](https://github.com/jergason)<br/>|[<img src="https://avatars.githubusercontent.com/u/227713?v=4" width="100px;"/><br/><sub><b>atian25</b></sub>](https://github.com/atian25)<br/>|
|[<img src="https://avatars.githubusercontent.com/u/130963?v=4" width="100px;"/><br/><sub><b>lagden</b></sub>](https://github.com/lagden)<br/>|[<img src="https://avatars.githubusercontent.com/u/484559?v=4" width="100px;"/><br/><sub><b>fixe</b></sub>](https://github.com/fixe)<br/>|[<img src="https://avatars.githubusercontent.com/u/2671328?v=4" width="100px;"/><br/><sub><b>viliam-jobko</b></sub>](https://github.com/viliam-jobko)<br/>|[<img src="https://avatars.githubusercontent.com/u/2971112?v=4" width="100px;"/><br/><sub><b>mzyy94</b></sub>](https://github.com/mzyy94)<br/>|[<img src="https://avatars.githubusercontent.com/u/687842?v=4" width="100px;"/><br/><sub><b>jeromew</b></sub>](https://github.com/jeromew)<br/>|[<img src="https://avatars.githubusercontent.com/u/6897780?v=4" width="100px;"/><br/><sub><b>killagu</b></sub>](https://github.com/killagu)<br/>|
|[<img src="https://avatars.githubusercontent.com/u/8069753?v=4" width="100px;"/><br/><sub><b>RobertHerhold</b></sub>](https://github.com/RobertHerhold)<br/>|[<img src="https://avatars.githubusercontent.com/u/4619802?v=4" width="100px;"/><br/><sub><b>yudppp</b></sub>](https://github.com/yudppp)<br/>|[<img src="https://avatars.githubusercontent.com/u/3173170?v=4" width="100px;"/><br/><sub><b>thedark1337</b></sub>](https://github.com/thedark1337)<br/>|[<img src="https://avatars.githubusercontent.com/u/6903313?v=4" width="100px;"/><br/><sub><b>x-cold</b></sub>](https://github.com/x-cold)<br/>|[<img src="https://avatars.githubusercontent.com/u/6713367?v=4" width="100px;"/><br/><sub><b>zzuieliyaoli</b></sub>](https://github.com/zzuieliyaoli)<br/>|[<img src="https://avatars.githubusercontent.com/u/81891?v=4" width="100px;"/><br/><sub><b>ryankask</b></sub>](https://github.com/ryankask)<br/>|
|[<img src="https://avatars.githubusercontent.com/u/4810916?v=4" width="100px;"/><br/><sub><b>pschwyter</b></sub>](https://github.com/pschwyter)<br/>|[<img src="https://avatars.githubusercontent.com/u/62940?v=4" width="100px;"/><br/><sub><b>mikefrey</b></sub>](https://github.com/mikefrey)<br/>|[<img src="https://avatars.githubusercontent.com/u/300104?v=4" width="100px;"/><br/><sub><b>dizlexik</b></sub>](https://github.com/dizlexik)<br/>|[<img src="https://avatars.githubusercontent.com/u/2505474?v=4" width="100px;"/><br/><sub><b>jeffijoe</b></sub>](https://github.com/jeffijoe)<br/>|[<img src="https://avatars.githubusercontent.com/u/349336?v=4" width="100px;"/><br/><sub><b>iliakan</b></sub>](https://github.com/iliakan)<br/>|[<img src="https://avatars.githubusercontent.com/u/615334?v=4" width="100px;"/><br/><sub><b>frederickfogerty</b></sub>](https://github.com/frederickfogerty)<br/>|
[<img src="https://avatars.githubusercontent.com/u/2552790?v=4" width="100px;"/><br/><sub><b>t3chnoboy</b></sub>](https://github.com/t3chnoboy)<br/>|[<img src="https://avatars.githubusercontent.com/u/1484279?v=4" width="100px;"/><br/><sub><b>bitinn</b></sub>](https://github.com/bitinn)<br/>|[<img src="https://avatars.githubusercontent.com/u/1441230?v=4" width="100px;"/><br/><sub><b>drGrove</b></sub>](https://github.com/drGrove)<br/>|[<img src="https://avatars.githubusercontent.com/u/12624092?v=4" width="100px;"/><br/><sub><b>CreativeCactus</b></sub>](https://github.com/CreativeCactus)<br/>|[<img src="https://avatars.githubusercontent.com/u/1773785?v=4" width="100px;"/><br/><sub><b>bguiz</b></sub>](https://github.com/bguiz)<br/>
Comment on lines +462 to +472
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add alternative text to images for better accessibility.

Please ensure to add alternative text to all contributor images to improve accessibility. For instance:

<img src="https://avatars.githubusercontent.com/u/1112718?v=4" width="100px;" alt="Profile image of alexmingoia"/><br/><sub><b>alexmingoia</b></sub>

This change helps visually impaired users understand the content better.

Tools
Markdownlint

464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


464-464: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


466-466: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


467-467: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


468-468: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


469-469: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


470-470: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


471-471: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: null (MD045, no-alt-text)
Images should have alternate text (alt text)


472-472: Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe (MD055, table-pipe-style)
Table pipe style


472-472: Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe (MD055, table-pipe-style)
Table pipe style


472-472: Expected: 6; Actual: 5; Too few cells, row will be missing data (MD056, table-column-count)
Table column count


This project follows the git-contributor [spec](https://github.com/xudafeng/git-contributor), auto updated at `Sun Jun 16 2024 12:28:11 GMT+0800`.

<!-- GITCONTRIBUTOR_END -->
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@eggjs/router",
"version": "3.0.1",
"engines": {
"node": ">= 18.7.0"
"node": ">= 18.19.0"
},
"publishConfig": {
"access": "public"
Expand Down
20 changes: 19 additions & 1 deletion src/Router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,25 @@ export class Router {
middlewares: (MiddlewareFunc | string | ResourcesController)[]) {
const options: RegisterOptions = {};
let path: string | RegExp | (string | RegExp)[];
if (typeof pathOrMiddleware === 'string' || pathOrMiddleware instanceof RegExp) {
if (typeof nameOrPath === 'string' && nameOrPath.startsWith('/')) {
// verb(method, path, ...middlewares)
path = nameOrPath;
middlewares = [ pathOrMiddleware as string, ...middlewares ];
if (typeof pathOrMiddleware === 'string') {
// verb(method, path, controllerString)
// set controller name to router name
options.name = pathOrMiddleware;
}
} else if (nameOrPath instanceof RegExp) {
// verb(method, pathRegex, ...middlewares)
path = nameOrPath;
middlewares = [ pathOrMiddleware as string, ...middlewares ];
if (typeof pathOrMiddleware === 'string') {
// verb(method, pathRegex, controllerString)
// set controller name to router name
options.name = pathOrMiddleware;
}
} else if (typeof pathOrMiddleware === 'string' || pathOrMiddleware instanceof RegExp) {
// verb(method, name, path, ...middlewares)
path = pathOrMiddleware;
assert(typeof nameOrPath === 'string', 'route name should be string');
Expand Down
57 changes: 57 additions & 0 deletions test/EggRouter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,63 @@ describe('test/EggRouter.test.ts', () => {
assert(router.stack[1].stack.length === 1);
});

it('should app.verb(url, controllerString) work', () => {
const app = {
controller: {
async foo() { return; },
hello: {
world() { return; },
},
},
};

const router = new EggRouter({}, app);
router.get('/foo', 'foo');
router.post('/hello/world', 'hello.world');

assert.equal(router.stack[0].name, 'foo');
assert.equal(router.stack[0].path, '/foo');
assert.deepEqual(router.stack[0].methods, [ 'HEAD', 'GET' ]);
assert.equal(router.stack[0].stack.length, 1);
assert.equal(router.stack[1].name, 'hello.world');
assert.equal(router.stack[1].path, '/hello/world');
assert.deepEqual(router.stack[1].methods, [ 'POST' ]);
assert.equal(router.stack[1].stack.length, 1);
});

it('should app.verb(urlRegex, controllerString) work', () => {
const app = {
controller: {
async foo() { return; },
hello: {
world() { return; },
},
},
};

const router = new EggRouter({}, app);
router.get(/^\/foo/, 'foo');
router.post(/^\/hello\/world/, 'hello.world');
router.post(/^\/hello\/world2/, () => {}, 'hello.world');

assert.equal(router.stack[0].name, 'foo');
assert(router.stack[0].path instanceof RegExp);
assert.equal(router.stack[0].path.toString(), String(/^\/foo/));
assert.deepEqual(router.stack[0].methods, [ 'HEAD', 'GET' ]);
assert.equal(router.stack[0].stack.length, 1);
assert.equal(router.stack[1].name, 'hello.world');
assert(router.stack[1].path instanceof RegExp);
assert.equal(router.stack[1].path.toString(), String(/^\/hello\/world/));
assert.deepEqual(router.stack[1].methods, [ 'POST' ]);
assert.equal(router.stack[1].stack.length, 1);

assert.equal(router.stack[2].name, undefined);
assert(router.stack[2].path instanceof RegExp);
assert.equal(router.stack[2].path.toString(), String(/^\/hello\/world2/));
assert.deepEqual(router.stack[2].methods, [ 'POST' ]);
assert.equal(router.stack[2].stack.length, 2);
});

it('should app.verb() throw if not found controller', () => {
const app = {
controller: {
Expand Down
Loading