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

feat: support cjs and esm both #10

Merged
merged 13 commits into from
Jun 11, 2024
Merged

feat: support cjs and esm both #10

merged 13 commits into from
Jun 11, 2024

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Jun 8, 2024

BREAKING CHANGE: drop Node.js < 18.7.0 support

  • Drop generator function support
  • Drop Node.js < 18.7.0 support

Summary by CodeRabbit

  • New Features

    • Introduced EggRouter class for defining RESTful routes and handling HTTP verbs.
    • Added new utility functions and type definitions to support enhanced routing and middleware functionalities.
  • Bug Fixes

    • Updated test cases to ensure compatibility with new routing and middleware functionalities.
  • Documentation

    • Updated examples in the README.md to reflect TypeScript syntax and ES module imports.
    • Mentioned breaking changes for version 3, including dropping support for generator functions and Node.js versions below 18.7.0.
  • Breaking Changes

    • Dropped support for generator functions.
    • Dropped support for Node.js versions below 18.7.0.
  • Chores

    • Updated Node.js versions in the GitHub Actions workflow.
    • Modified .gitignore to include additional patterns.
    • Updated dependencies and dev dependencies in package.json.
    • Added new scripts for linting, testing, and pre-publish actions in package.json.
    • Introduced a new tsconfig.json for strict TypeScript settings.

BREAKING CHANGE: drop Node.js < 18.7.0 support

- Drop generator function support
- Drop Node.js < 18.7.0 support
@fengmk2 fengmk2 added the enhancement New feature or request label Jun 8, 2024
@fengmk2 fengmk2 self-assigned this Jun 8, 2024
Copy link

coderabbitai bot commented Jun 8, 2024

Walkthrough

The updates enhance the ESLint configuration for TypeScript and node prefix enforcement, refine Node.js versioning in workflows, and extend .gitignore patterns. The README.md and source files have been updated to use TypeScript and ES modules, introduce new features, and enforce stricter Node.js version requirements. Additionally, new tests, scripts, and dependencies have been added to support these changes, improving the overall functionality and maintainability of the project.

Changes

File/Path Change Summary
.eslintrc Extended ESLint configuration for TypeScript and node prefix enforcement.
.github/workflows/nodejs.yml Updated Node.js versions in the workflow to specific versions.
.gitignore Added patterns .tshy* and dist to ignore list.
README.md Updated examples to TypeScript syntax, noted breaking changes for version 3.
bench/run Updated script to run server using server.cjs instead of server.js.
bench/server.cjs Switched to Application from @eggjs/koa, updated imports and commented out nesting lines.
package.json Updated version, dependencies, added new scripts and configurations.
src/EggRouter.ts, src/Router.ts Introduced new classes and methods for enhanced routing and RESTful support.
src/Layer.ts Added Layer class with methods for path matching, URL generation, and parameter handling.
src/index.ts Updated exports to include new modules and set Router as default export.
src/types.ts Introduced new type definitions for middleware functions and resource controllers.
test/*.ts Updated and added tests for EggRouter, Layer, and overall module functionality.
tsconfig.json Added TypeScript configuration with strict typing and ES2022 target.

Poem

In code's embrace, TypeScript shines bright,
Node's versions aligned, workflows take flight.
Routers refined, new paths to explore,
Tests and configs, enhancing more.
With every commit, we reach new heights,
Together we build, in these digital nights.
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

socket-security bot commented Jun 8, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@eslint-community/eslint-utils@4.4.0, npm/@eslint-community/regexpp@4.10.1, npm/@eslint/eslintrc@2.1.4, npm/@eslint/js@8.57.0, npm/@humanwhocodes/config-array@0.11.14, npm/@humanwhocodes/module-importer@1.0.1, npm/@humanwhocodes/object-schema@2.0.3, npm/@types/json5@0.0.29, npm/@types/node@20.14.2, npm/@typescript-eslint/eslint-plugin@6.21.0, npm/@typescript-eslint/parser@6.21.0, npm/@typescript-eslint/scope-manager@6.21.0, npm/@typescript-eslint/type-utils@6.21.0, npm/@typescript-eslint/types@6.21.0, npm/@typescript-eslint/typescript-estree@6.21.0, npm/@typescript-eslint/utils@6.21.0, npm/@typescript-eslint/visitor-keys@6.21.0, npm/@ungap/structured-clone@1.2.0, npm/ansi-colors@4.1.1, npm/argparse@2.0.1, npm/brace-expansion@2.0.1, npm/c8@9.1.0, npm/camelcase@6.3.0, npm/chokidar@3.5.3, npm/cliui@8.0.1, npm/comment-parser@1.4.1, npm/debug@4.3.4, npm/decamelize@4.0.0, npm/diff@5.0.0, npm/egg-bin@6.9.0, npm/egg-ts-helper@2.1.0, npm/escape-string-regexp@4.0.0, npm/eslint-config-egg@13.1.0, npm/eslint-plugin-eggache@2.0.0, npm/eslint-plugin-jsdoc@46.10.1, npm/eslint-scope@7.2.2, npm/eslint-visitor-keys@3.4.3, npm/eslint@8.57.0, npm/espree@9.6.1, npm/execa@5.1.1, npm/file-entry-cache@6.0.1, npm/flat-cache@3.2.0, npm/flat@5.0.2, npm/flatted@3.3.1, npm/for-each@0.3.3, npm/foreground-child@3.1.1, npm/get-stream@6.0.1, npm/glob-parent@6.0.2, npm/glob@8.1.0, npm/globals@13.24.0, npm/graphemer@1.4.0, npm/has-bigints@1.0.2, npm/http-errors@2.0.0, npm/human-signals@2.1.0, npm/inflection@3.0.0, npm/is-async-function@2.0.0, npm/is-bigint@1.0.4, npm/is-boolean-object@1.1.2, npm/is-finalizationregistry@1.0.2, npm/is-map@2.0.3, npm/is-number-object@1.0.7, npm/is-path-inside@3.0.3, npm/is-set@2.0.3, npm/is-type-of@2.1.0, npm/is-weakmap@2.0.2, npm/is-weakset@2.0.3, npm/jest-changed-files@29.7.0, npm/js-yaml@4.1.0, npm/json-buffer@3.0.1, npm/json5@1.0.2, npm/keyv@4.5.4, npm/levn@0.4.1, npm/lodash.merge@4.6.2, npm/log-symbols@4.1.0, npm/minimatch@5.0.1, npm/minimatch@5.1.6, npm/minimatch@9.0.3, npm/mocha@10.4.0, npm/optionator@0.9.4, npm/prelude-ls@1.2.1, npm/punycode@2.3.1, npm/serialize-javascript@6.0.0, npm/signal-exit@4.1.0, npm/spdx-expression-parse@4.0.0, npm/statuses@2.0.1, npm/strip-json-comments@3.1.1, npm/supports-color@8.1.1, npm/tsconfig-paths@4.2.0, npm/type-check@0.4.0, npm/type-fest@0.20.2, npm/typescript@5.4.5, npm/undici-types@5.26.5, npm/unescape@1.0.1, npm/utility@2.1.0, npm/which-boxed-primitive@1.0.2, npm/which-collection@1.0.2, npm/workerpool@6.2.1, npm/yargs-parser@20.2.4, npm/yargs-parser@21.1.1, npm/yargs-unparser@2.0.0, npm/yargs@17.7.2

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 99.40441% with 10 lines in your changes missing coverage. Please review.

Project coverage is 99.40%. Comparing base (2d71fd8) to head (edf455d).

Files Patch % Lines
src/Router.ts 99.24% 8 Missing ⚠️
src/EggRouter.ts 99.40% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
- Coverage   99.46%   99.40%   -0.07%     
==========================================
  Files           5        4       -1     
  Lines         374     1679    +1305     
  Branches       84      216     +132     
==========================================
+ Hits          372     1669    +1297     
- Misses          2       10       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fengmk2
Copy link
Member Author

fengmk2 commented Jun 10, 2024

@SocketSecurity ignore-all

@fengmk2 fengmk2 marked this pull request as ready for review June 11, 2024 07:04
@fengmk2 fengmk2 merged commit 55149bc into master Jun 11, 2024
7 of 9 checks passed
@fengmk2 fengmk2 deleted the next-major branch June 11, 2024 07:16
fengmk2 pushed a commit that referenced this pull request Jun 11, 2024
[skip ci]

## [3.0.0](v2.0.1...v3.0.0) (2024-06-11)

### ⚠ BREAKING CHANGES

* drop Node.js < 18.7.0 support

- Drop generator function support
- Drop Node.js < 18.7.0 support

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced `EggRouter` class for defining RESTful routes and handling
HTTP verbs.
- Added new utility functions and type definitions to support enhanced
routing and middleware functionalities.

- **Bug Fixes**
- Updated test cases to ensure compatibility with new routing and
middleware functionalities.

- **Documentation**
- Updated examples in the `README.md` to reflect TypeScript syntax and
ES module imports.
- Mentioned breaking changes for version 3, including dropping support
for generator functions and Node.js versions below 18.7.0.

- **Breaking Changes**
  - Dropped support for generator functions.
  - Dropped support for Node.js versions below 18.7.0.

- **Chores**
  - Updated Node.js versions in the GitHub Actions workflow.
  - Modified `.gitignore` to include additional patterns.
  - Updated dependencies and dev dependencies in `package.json`.
- Added new scripts for linting, testing, and pre-publish actions in
`package.json`.
  - Introduced a new `tsconfig.json` for strict TypeScript settings.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

### Features

* support cjs and esm both ([#10](#10)) ([55149bc](55149bc))
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Outside diff range and nitpick comments (5)
bench/server.cjs (1)

38-39: Consider re-enabling or removing the commented-out route configurations if they are no longer needed. Keeping them might confuse new developers or clutter the codebase.

test/EggRouter.test.ts (2)

Line range hint 36-81: Tests for basic routing functionality are correctly implemented. Consider refactoring function expressions to arrow functions for consistency and to reduce complexity.

- router.get('/foo', function() {
+ router.get('/foo', () => {

Line range hint 161-210: Tests for middleware integration in routing are correctly implemented. Consider using arrow functions for middleware to reduce complexity.

- const asyncMiddleware1 = async function() {
+ const asyncMiddleware1 = async () => {
Tools
Biome

[error] 161-161: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 162-162: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 163-163: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

README.md (1)

Line range hint 71-80: The documentation for HTTP verb methods is clear and concise. However, consider adding a note about the removal of generator function support in these methods.

+ Note: As of version 3, generator functions are no longer supported in route handlers.
Tools
LanguageTool

[duplication] ~27-~27: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ... - router.param(param, middleware) ⇒ Router - [Router.url(path, params [, options]) ⇒ Strin...


[uncategorized] ~72-~72: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ethods, where verb is one of the HTTP verbs such as router.get() or `router.post(...

test/Router.test.ts (1)

Line range hint 19-22: Consider converting function expressions to arrow functions for consistency and improved readability.

The static analysis tool suggests converting function expressions to arrow functions where this is not used. This can make the code more concise and easier to understand. Here's an example of how you can refactor one of the functions:

- router1.get('/', function(ctx, next) {
+ router1.get('/', (ctx, next) => {
    ctx.foo = 'bar';
    return next();
  });

Apply similar changes to other function expressions throughout the file.

Also applies to: 23-27, 41-43, 44-46, 47-49, 51-54, 67-71, 85-88, 84-89, 83-90, 92-95, 91-96, 90-97, 97-99, 115-117, 118-120, 121-123, 125-127, 145-150, 144-151

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2d71fd8 and ad4cd64.

Files selected for processing (17)
  • .eslintrc (1 hunks)
  • .github/workflows/nodejs.yml (1 hunks)
  • .gitignore (1 hunks)
  • README.md (23 hunks)
  • bench/run (1 hunks)
  • bench/server.cjs (2 hunks)
  • package.json (3 hunks)
  • src/EggRouter.ts (1 hunks)
  • src/Layer.ts (1 hunks)
  • src/Router.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/types.ts (1 hunks)
  • test/EggRouter.test.ts (7 hunks)
  • test/Layer.test.ts (1 hunks)
  • test/Router.test.ts (40 hunks)
  • test/index.test.ts (1 hunks)
  • tsconfig.json (1 hunks)
Files not summarized due to errors (1)
  • test/Router.test.ts: Error: Message exceeds token limit
Files not reviewed due to errors (1)
  • src/EggRouter.ts (no review received)
Files skipped from review due to trivial changes (5)
  • .eslintrc
  • .github/workflows/nodejs.yml
  • .gitignore
  • src/index.ts
  • tsconfig.json
Additional context used
Biome
test/Layer.test.ts

[error] 90-95: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 105-110: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 110-115: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 144-144: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 156-158: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 181-188: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 189-196: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 208-208: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

test/EggRouter.test.ts

[error] 20-20: This generator function doesn't contain yield. (lint/correctness/useYield)


[error] 161-161: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 162-162: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 163-163: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 200-200: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

src/Layer.ts

[error] 244-253: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

src/Router.ts

[error] 316-319: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 334-337: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 305-305: Shouldn't redeclare 'allowedMethods'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)

'allowedMethods' is defined here:

test/Router.test.ts

[error] 19-22: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 23-27: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 41-43: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 44-46: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 47-49: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 51-54: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 67-71: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 85-88: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 84-89: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 83-90: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 92-95: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 91-96: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 90-97: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 97-99: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 115-117: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 118-120: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 121-123: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 125-127: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 145-150: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 144-151: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

LanguageTool
README.md

[duplication] ~21-~21: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ... - router.use([path], middleware) ⇒ Router - [router.prefix(prefix) ⇒ Router](#routerprefixp...


[duplication] ~22-~22: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...outer) - router.prefix(prefix) ⇒ Router - [router.allowedMethods([options]) ⇒ function]...


[duplication] ~24-~24: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...direct(source, destination, [code]) ⇒ Router](#routerredirectsource-destination-code--router) - [router.route(name) ⇒ Layer | false](#routerrou...


[duplication] ~27-~27: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ... - router.param(param, middleware) ⇒ Router - [Router.url(path, params [, options]) ⇒ Strin...


[uncategorized] ~72-~72: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ethods, where verb is one of the HTTP verbs such as router.get() or `router.post(...


[misspelling] ~384-~384: This word is normally spelled as one. (EN_COMPOUNDS_AUTO_LOADING)
Context: ... for named route parameters. Useful for auto-loading or validation. Kind: instance meth...

Markdownlint
README.md

194-194: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines


216-216: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading


245-245: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading


268-268: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading


355-355: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading


394-394: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading


430-430: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading

GitHub Check: codecov/patch
src/EggRouter.ts

[warning] 91-94: src/EggRouter.ts#L91-L94
Added lines #L91 - L94 were not covered by tests


[warning] 98-101: src/EggRouter.ts#L98-L101
Added lines #L98 - L101 were not covered by tests


[warning] 112-115: src/EggRouter.ts#L112-L115
Added lines #L112 - L115 were not covered by tests


[warning] 119-122: src/EggRouter.ts#L119-L122
Added lines #L119 - L122 were not covered by tests


[warning] 133-136: src/EggRouter.ts#L133-L136
Added lines #L133 - L136 were not covered by tests


[warning] 140-143: src/EggRouter.ts#L140-L143
Added lines #L140 - L143 were not covered by tests


[warning] 285-286: src/EggRouter.ts#L285-L286
Added lines #L285 - L286 were not covered by tests

src/Router.ts

[warning] 371-372: src/Router.ts#L371-L372
Added lines #L371 - L372 were not covered by tests


[warning] 380-381: src/Router.ts#L380-L381
Added lines #L380 - L381 were not covered by tests


[warning] 610-613: src/Router.ts#L610-L613
Added lines #L610 - L613 were not covered by tests


[warning] 825-828: src/Router.ts#L825-L828
Added lines #L825 - L828 were not covered by tests

Additional comments not posted (30)
test/index.test.ts (1)

1-11: LGTM! The tests are well-structured and verify the exposure of Router, KoaRouter, and EggRouter as expected.

bench/run (1)

11-11: Ensure the server script server.cjs is correctly handling startup errors. Consider adding error handling for the server startup command.

src/types.ts (1)

1-15: The type definitions and interface are well-defined and cover a broad range of middleware functionalities. Ensure these types are consistently used throughout the codebase to maintain type safety.

package.json (5)

4-6: Ensure compatibility with Node.js version >= 18.7.0.


7-9: Public access for npm packages is correctly set.


63-91: License, files, and module type settings are correctly updated. Ensure the exports configuration aligns with the project's module resolution strategy.


36-50: New development dependencies added. Ensure they are necessary and correctly configured.


26-33: Dependencies have been updated to their latest versions. Verify compatibility with the rest of the project.

test/Layer.test.ts (5)

8-25: Tests for middleware composition and URL path parameter capturing are correctly implemented.


44-57: Test for handling URL encoding errors is well-implemented.


101-119: Test for optional URL segments is correctly implemented.

Tools
Biome

[error] 105-110: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 110-115: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


121-149: Test for error handling when middleware is not provided is well-implemented.

Tools
Biome

[error] 144-144: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


206-219: Test for URL generation is correctly implemented.

Tools
Biome

[error] 208-208: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

test/EggRouter.test.ts (4)

Line range hint 6-34: Tests for router initialization and error handling for generator functions are correctly implemented.


58-81: Tests for routing with multiple URLs and named routes are correctly implemented.


204-210: Test for resource routing is correctly implemented.


218-233: Test for URL generation and path resolution is correctly implemented.

src/Layer.ts (4)

91-100: The match method is correctly implemented.


102-122: The params method is correctly implemented for extracting URL parameters.


124-135: The captures method is correctly implemented for capturing URL segments.


137-204: The url method is correctly implemented for generating URLs. Consider using arrow functions for internal function expressions to reduce complexity.

- stack.some(function(fn, i) {
+ stack.some((fn, i) => {

Likely invalid or redundant comment.

README.md (9)

53-67: Ensure the example uses the correct import statements and reflects the new module structure if necessary.

Verification successful

The import statements in the README.md are correct and reflect the valid module structure.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the example in the README.md uses the correct import statements.

# Test: Search for the import statements in the example. Expect: Correct module paths.
rg --type markdown "import .* from '@eggjs/.*';" README.md

Length of output: 264



Script:

#!/bin/bash
# Description: Verify the existence and correctness of the '@eggjs/koa' and '@eggjs/router' modules.

# Search for '@eggjs/koa' and '@eggjs/router' in package.json to confirm they are valid dependencies.
rg '"@eggjs/koa"' package.json
rg '"@eggjs/router"' package.json

Length of output: 124


146-147: The nested router example is well-explained. Ensure that the code snippets are updated to reflect any changes in syntax or functionality due to the new module system.

Verification successful

To ensure the nested router example in the README.md is up-to-date with the current module system, we need to verify the correct syntax for creating new routers in the codebase. Let's search for the Router class or function definition and its usage in the codebase to confirm the current syntax.


The search results indicate that the syntax new Router() is used consistently throughout the codebase, including in the README.md file. This suggests that the current syntax for creating new routers is indeed new Router(), and it aligns with the examples provided in the README.md.

  • README.md:
    • const router = new Router();
    • const forums = new Router();
    • const posts = new Router();

Given this consistency, it appears that the nested router example in the README.md is up-to-date with the current module system.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the nested router example in the README.md is up-to-date.

# Test: Search for outdated syntax or functionality in the nested router example. Expect: Current syntax and functionality.
rg --type markdown "new Router()" README.md

Length of output: 289



Script:

#!/bin/bash
# Description: Verify the current syntax for creating new routers in the codebase.

# Search for the definition of the Router class or function.
ast-grep --lang javascript --pattern 'class Router { $$$ }'

# Search for instances where new Router is used in the codebase.
rg 'new Router()'

Length of output: 6131


343-343: The router.url method documentation is clear and informative. Ensure that the URL generation functionality works as described.


201-201: The middleware documentation is comprehensive. Ensure that all middleware examples are compatible with the new module system and Node.js version requirements.


331-331: The router.route method documentation is accurate. Verify that the route lookup functionality works as described.

Verification successful

The test cases for router.route and router.routes are comprehensive and cover various scenarios, including middleware registration, parameter handling, nested routes, and error handling. These tests confirm that the route lookup functionality works as described in the README.md.

  • The router.route method is tested for route registration and middleware functionality.
  • The router.routes method is tested for middleware composition, route matching, and integration with other middleware.

The functionality appears to be well-documented and verified through these tests.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the route lookup functionality in the README.md works as described.

# Test: Search for issues related to route lookup functionality in the routing system. Expect: No issues found.
rg --type markdown "router.route" README.md

Length of output: 396



Script:

#!/bin/bash
# Search for the definition of `router.route` in the codebase
ast-grep --lang javascript --pattern 'router.route($_)' 

# Search for the definition of `router.routes` in the codebase
ast-grep --lang javascript --pattern 'router.routes($_)' 

Length of output: 116



Script:

#!/bin/bash
# Search for the term `router.route` in the codebase
rg 'router.route'

# Search for the term `router.routes` in the codebase
rg 'router.routes'

Length of output: 9800



Script:

#!/bin/bash
# Extract test cases for `router.route` from the test files
rg -A 10 'router.route' test/

# Extract test cases for `router.routes` from the test files
rg -A 10 'router.routes' test/

Length of output: 72726


302-302: The router.redirect method documentation is clear. Ensure that the functionality works as described with the new routing system.

Verification successful

The router.redirect method is implemented in src/Router.ts and is referenced in the README.md as well as in the test file test/Router.test.ts. The implementation and tests indicate that the functionality is as described in the documentation.

  • src/Router.ts: Contains the implementation of router.redirect.
  • test/Router.test.ts: Contains tests verifying the router.redirect functionality.

Locations to review:

  • src/Router.ts
  • test/Router.test.ts
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the `router.redirect` functionality in the README.md works as described.

# Test: Search for issues related to `router.redirect` functionality in the routing system. Expect: No issues found.
rg --type markdown "router.redirect" README.md

Length of output: 270



Script:

#!/bin/bash
# Locate the implementation of `router.redirect` in the codebase
ast-grep --lang javascript --pattern 'router.redirect($_, $_, $_)' 

Length of output: 68



Script:

#!/bin/bash
# Locate the `redirect` method within any class or object named `router`
ast-grep --lang javascript --pattern 'class $_ {
  $$$
  redirect($_, $_, $_) {
    $$$
  }
  $$$
}' 

ast-grep --lang javascript --pattern 'const router = {
  $$$
  redirect($_, $_, $_) {
    $$$
  }
  $$$
}' 

Length of output: 211



Script:

#!/bin/bash
# Search for any occurrence of `redirect` in the codebase to understand its context and implementation
rg 'redirect' 

Length of output: 1223


236-236: The documentation for router.prefix is clear. Ensure that the functionality described is consistent with the new routing system's capabilities.


254-254: The router.allowedMethods method is well-documented. Verify that the options parameter is handled correctly in the new routing system.


418-418: The Router.url static method documentation is accurate. Ensure that the static URL generation functionality works as described.

Verification successful

The Router.url static method is implemented in src/Router.ts and is tested in test/Router.test.ts. The implementation and tests confirm that the method generates URLs as described in the README.md, including handling parameters and query options.

  • Implementation in src/Router.ts:

    static url(path: string, params?: string | number | object,
      ...paramsOrOptions: (string | number | object | LayerURLOptions)[]): string {
      return Layer.prototype.url.call({ path }, params, ...paramsOrOptions);
    }
  • Tests in test/Router.test.ts:

    describe('Static Router#url()', () => {
      it('generates route URL', () => {
        const url = Router.url('/:category/:title', { category: 'programming', title: 'how-to-node' });
        assert.equal(url, '/programming/how-to-node');
      });
    
      it('escapes using encodeURIComponent()', () => {
        const url = Router.url('/:category/:title', { category: 'programming', title: 'how to node' });
        assert.equal(url, '/programming/how%20to%20node');
      });
    
      it('generates route URL with params and query params', () => {
        let url = Router.url('/books/:category/:id', 'programming', 4, {
          query: { page: 3, limit: 10 },
        });
        assert.equal(url, '/books/programming/4?page=3&limit=10');
        url = Router.url('/books/:category/:id',
          { category: 'programming', id: 4 },
          { query: { page: 3, limit: 10 } },
        );
        assert.equal(url, '/books/programming/4?page=3&limit=10');
        url = Router.url('/books/:category/:id',
          { category: 'programming', id: 4 },
          { query: 'page=3&limit=10' },
        );
        assert.equal(url, '/books/programming/4?page=3&limit=10');
      });
    
      it('generates router URL without params and with with query params', () => {
        const url = Router.url('/category', {
          query: { page: 3, limit: 10 },
        });
        assert.equal(url, '/category?page=3&limit=10');
      });
    });

The static URL generation functionality works as described in the README.md.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/binbash
# Description: Verify that the static URL generation functionality in the README.md works as described.

# Test: Search for issues related to static URL generation functionality in the routing system. Expect: No issues found.
rg --type markdown "Router.url" README.md

Length of output: 414



Script:

#!/bin/bash
# Locate the implementation of the Router.url method in the codebase
ast-grep --lang javascript --pattern 'class Router {
  $$$
  static url($_, $_, $_) {
    $$$
  }
  $$$
}'

Length of output: 108



Script:

#!/bin/bash
# Locate the implementation of the Router.url method in the codebase with a broader search pattern
ast-grep --lang javascript --pattern 'class Router {
  $$$
  static url($$$) {
    $$$
  }
  $$$
}'

Length of output: 101



Script:

#!/bin/bash
# Search for references to Router.url in the codebase
rg 'Router.url'

Length of output: 1524



Script:

#!/bin/bash
# Extract the implementation of Router.url from src/Router.ts
rg 'Router.url' -A 10 src/Router.ts

# Extract the tests for Router.url from test/Router.test.ts
rg 'Router.url' -A 10 test/Router.test.ts

Length of output: 2381

allowed[method] = method;
});
});
const allowedMethods = Object.keys(allowed);
Copy link

Choose a reason for hiding this comment

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

Consider renaming the local variable allowedMethods to avoid shadowing the method name.

The variable allowedMethods on line 305 shadows the method name allowedMethods defined from line 292. This can lead to confusion and potential bugs. Consider renaming the variable to something more specific like allowedMethodsList.

Tools
Biome

[error] 305-305: Shouldn't redeclare 'allowedMethods'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)

'allowedMethods' is defined here:

Comment on lines +316 to +319
} else {
ctx.status = 501;
ctx.set('Allow', allowedMethods.join(', '));
}
Copy link

Choose a reason for hiding this comment

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

Remove unnecessary else clauses to simplify control flow.

The else clauses on lines 316-319 and 334-337 are unnecessary because the preceding if statements end with throw statements, which exit the block. Removing these else clauses can simplify the control flow and improve readability.

-        } else {
+        }

Also applies to: 334-337

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 316-319: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

Comment on lines +371 to +372
throw routeUrl;
}
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The new error handling paths introduced in lines 371-372, 380-381, 610-613, and 825-828 in src/Router.ts are not covered by tests. It's crucial to ensure that these paths behave as expected under error conditions.

  • Lines 371-372: throw routeUrl
  • Lines 380-381: throw new Error
  • Lines 610-613: throw new Error
  • Lines 825-828: throw new Error

Please add test coverage for these error handling paths.

Analysis chain

Add test coverage for new error handling paths.

The new error handling paths introduced in lines 371-372, 380-381, 610-613, and 825-828 are not covered by tests. It's crucial to ensure that these paths behave as expected under error conditions.

Also applies to: 380-381, 610-613, 825-828

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Check test coverage for specific lines in src/Router.ts

# Test: Search for test cases covering the specified lines. Expect: Test cases found.
rg --type python -C 3 "src/Router.ts" | grep -E "test.*(371|372|380|381|610|613|825|828)"

Length of output: 120



Script:

#!/bin/bash
# Description: Check test coverage for specific lines in src/Router.ts

# Test: Search for test cases covering the specified lines. Expect: Test cases found.
rg --type ts -C 3 "src/Router.ts" | grep -E "test.*(371|372|380|381|610|613|825|828)"

Length of output: 85



Script:

#!/bin/bash
# Description: Search for test files that might indirectly cover the specified lines in src/Router.ts

# Find all test files
test_files=$(fd --type f --extension ts --extension tsx --glob '*test*')

# Search for references to error handling paths or related functions in test files
for file in $test_files; do
  echo "Searching in $file"
  rg --type ts -C 3 "throw routeUrl" "$file"
  rg --type ts -C 3 "throw new Error" "$file"
done

Length of output: 1025

Tools
GitHub Check: codecov/patch

[warning] 371-372: src/Router.ts#L371-L372
Added lines #L371 - L372 were not covered by tests

README.md Outdated
@@ -27,44 +27,48 @@ Router core component for [Egg.js](https://github.com/eggjs).
- [router.param(param, middleware) ⇒ Router](#routerparamparam-middleware--router)
- [Router.url(path, params \[, options\]) ⇒ String](#routerurlpath-params--options--string)
- [Tests](#tests)
- [Breaking changes on v3](#breaking-changes-on-v3)
Copy link

Choose a reason for hiding this comment

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

Update the "Breaking changes on v3" section to include a brief explanation of why generator functions and Node.js versions below 18.7.0 are no longer supported.

Would you like me to draft a brief explanation for these breaking changes?

README.md Outdated
@@ -167,7 +171,7 @@

Named route parameters are captured and added to `ctx.params`.

```javascript
```ts
Copy link

Choose a reason for hiding this comment

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

The explanation of URL parameters is accurate. Consider adding examples of how these parameters can be used in middleware for clarity.

+ Example: Using URL parameters in middleware to log request details.
+ router.get('/:category/:title', (ctx, next) => {
+   console.log(`Category: ${ctx.params.category}, Title: ${ctx.params.title}`);
+   next();
+ });
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
```ts
```ts
Example: Using URL parameters in middleware to log request details.
router.get('/:category/:title', (ctx, next) => {
console.log(`Category: ${ctx.params.category}, Title: ${ctx.params.title}`);
next();
});

README.md Outdated
<a name="module_egg-router--Router+param"></a>

#### router.param(param, middleware) ⇒ <code>Router</code>

Copy link

Choose a reason for hiding this comment

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

The router.param method documentation is clear. Consider adding more examples to illustrate its use in different scenarios.

+ Example: Using `router.param` to load a user object.
+ router.param('user', async (id, ctx, next) => {
+   ctx.user = await User.findById(id);
+   if (!ctx.user) ctx.throw(404);
+   await next();
+ });
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
Example: Using `router.param` to load a user object.
router.param('user', async (id, ctx, next) => {
ctx.user = await User.findById(id);
if (!ctx.user) ctx.throw(404);
await next();
});

Comment on lines +153 to +203
it('composes middleware for param fn', async () => {
const app = new Application();
const router = new Router();
const route = new Layer('/users/:user', [ 'GET' ], [ function(ctx) {
ctx.body = ctx.user;
} ]);
route.param('user', (id, ctx, next) => {
ctx.user = { name: 'alex' };
if (!id) {
ctx.status = 404;
return;
}
return next();
});
router.stack.push(route);
app.use(router.middleware());
const res = await request(app.callback())
.get('/users/3')
.expect(200);
assert.equal(res.body.name, 'alex');
});

it('ignores params which are not matched', async () => {
const app = new Application();
const router = new Router();
const route = new Layer('/users/:user', [ 'GET' ], [ ctx => {
ctx.body = ctx.user;
} ]);
route.param('user', function(id, ctx, next) {
ctx.user = { name: 'alex' };
if (!id) {
ctx.status = 404;
return;
}
return next();
});
route.param('title', function(id, ctx, next) {
ctx.user = { name: 'mark' };
if (!id) {
ctx.status = 404;
return;
}
return next();
});
router.stack.push(route);
app.use(router.middleware());
const res = await request(app.callback())
.get('/users/3')
.expect(200);
assert.equal(res.body.name, 'alex');
});
Copy link

Choose a reason for hiding this comment

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

Tests for parameter-specific middleware are correctly implemented. Consider using arrow functions for consistency.

- route.param('user', function(id, ctx, next) {
+ route.param('user', (id, ctx, next) => {

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 156-158: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 181-188: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 189-196: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

Comment on lines +59 to +99
it('populates ctx.captures with regexp captures', async () => {
const app = new Application();
const router = new Router();
app.use(router.routes());
router.get(/^\/api\/([^\/]+)\/?/i, (ctx, next) => {
assert(ctx.captures);
assert(Array.isArray(ctx.captures));
assert.equal(ctx.captures.length, 1);
assert.equal(ctx.captures[0], '1');
return next();
}, ctx => {
assert(ctx.captures);
assert(Array.isArray(ctx.captures));
assert.equal(ctx.captures.length, 1);
assert.equal(ctx.captures[0], '1');
ctx.status = 204;
});
await request(app.callback())
.get('/api/1')
.expect(204);
});

it('return original ctx.captures when decodeURIComponent throw error', async () => {
const app = new Application();
const router = new Router();
app.use(router.routes());
router.get(/^\/api\/([^\/]+)\/?/i, (ctx, next) => {
assert(Array.isArray(ctx.captures));
assert.equal(ctx.captures.length, 1);
assert.equal(ctx.captures[0], '101%');
return next();
}, function(ctx) {
assert(Array.isArray(ctx.captures));
assert.equal(ctx.captures.length, 1);
assert.equal(ctx.captures[0], '101%');
ctx.status = 204;
});
await request(app.callback())
.get('/api/101%')
.expect(204);
});
Copy link

Choose a reason for hiding this comment

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

Tests for capturing regular expression groups in URLs are correctly implemented. Consider refactoring function expressions to arrow functions for consistency and to reduce complexity.

- router.get(/^\/api\/([^\/]+)\/?/i, (ctx, next) => {
+ router.get(/^\/api\/([^\/]+)\/?/i, (ctx, next) => {

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 90-95: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

src/Layer.ts Outdated
Comment on lines +30 to +89
export class Layer {
readonly opts: LayerOptions;
readonly name?: string;
readonly methods: string[] = [];
readonly stack: MiddlewareFuncWithParamProperty[];
path: string | RegExp;
regexp: RegExp;
paramNames: Key[] = [];

/**
* Initialize a new routing Layer with given `method`, `path`, and `middleware`.
*
* @param {String|RegExp} path Path string or regular expression.
* @param {Array} methods Array of HTTP verbs.
* @param {Array|Function} middlewares Layer callback/middleware or series of.
* @param {Object=} opts optional params
* @param {String=} opts.name route name
* @param {String=} opts.sensitive case sensitive (default: false)
* @param {String=} opts.strict require the trailing slash (default: false)
* @private
*/
constructor(path: string | RegExp, methods: string[], middlewares: MiddlewareFunc | MiddlewareFunc[],
opts?: LayerOptions | string) {
if (typeof opts === 'string') {
// new Layer(path, methods, middlewares, name);
opts = { name: opts };
}
this.opts = opts ?? {};
this.opts.prefix = this.opts.prefix ?? '';
this.name = this.opts.name;
this.stack = Array.isArray(middlewares) ? middlewares : [ middlewares ];

for (const method of methods) {
const l = this.methods.push(method.toUpperCase());
if (this.methods[l - 1] === 'GET') {
this.methods.unshift('HEAD');
}
}

// ensure middleware is a function
this.stack.forEach(fn => {
const type = typeof fn;
if (type !== 'function') {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: `middleware` '
+ 'must be a function, not `' + type + '`',
);
}
if (isGeneratorFunction(fn)) {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: Please use async function instead of generator function',
);
}
});

this.path = path;
this.regexp = pathToRegExp(path, this.paramNames, this.opts);

debug('defined route %s %s', this.methods, this.opts.prefix + this.path);
}
Copy link

Choose a reason for hiding this comment

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

The Layer class constructor is correctly implemented. Consider using arrow functions for middleware to ensure consistency and reduce complexity.

- this.stack.forEach(function(fn) {
+ this.stack.forEach((fn) => {
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
export class Layer {
readonly opts: LayerOptions;
readonly name?: string;
readonly methods: string[] = [];
readonly stack: MiddlewareFuncWithParamProperty[];
path: string | RegExp;
regexp: RegExp;
paramNames: Key[] = [];
/**
* Initialize a new routing Layer with given `method`, `path`, and `middleware`.
*
* @param {String|RegExp} path Path string or regular expression.
* @param {Array} methods Array of HTTP verbs.
* @param {Array|Function} middlewares Layer callback/middleware or series of.
* @param {Object=} opts optional params
* @param {String=} opts.name route name
* @param {String=} opts.sensitive case sensitive (default: false)
* @param {String=} opts.strict require the trailing slash (default: false)
* @private
*/
constructor(path: string | RegExp, methods: string[], middlewares: MiddlewareFunc | MiddlewareFunc[],
opts?: LayerOptions | string) {
if (typeof opts === 'string') {
// new Layer(path, methods, middlewares, name);
opts = { name: opts };
}
this.opts = opts ?? {};
this.opts.prefix = this.opts.prefix ?? '';
this.name = this.opts.name;
this.stack = Array.isArray(middlewares) ? middlewares : [ middlewares ];
for (const method of methods) {
const l = this.methods.push(method.toUpperCase());
if (this.methods[l - 1] === 'GET') {
this.methods.unshift('HEAD');
}
}
// ensure middleware is a function
this.stack.forEach(fn => {
const type = typeof fn;
if (type !== 'function') {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: `middleware` '
+ 'must be a function, not `' + type + '`',
);
}
if (isGeneratorFunction(fn)) {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: Please use async function instead of generator function',
);
}
});
this.path = path;
this.regexp = pathToRegExp(path, this.paramNames, this.opts);
debug('defined route %s %s', this.methods, this.opts.prefix + this.path);
}
// ensure middleware is a function
this.stack.forEach((fn) => {
const type = typeof fn;
if (type !== 'function') {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: `middleware` '
+ 'must be a function, not `' + type + '`',
);
}
if (isGeneratorFunction(fn)) {
throw new TypeError(
methods.toString() + ' `' + (this.opts.name || path) + '`: Please use async function instead of generator function',
);
}
});

src/Layer.ts Outdated
Comment on lines +207 to +273
/**
* Run validations on route named parameters.
*
* @example
*
* ```javascript
* router
* .param('user', function (id, ctx, next) {
* ctx.user = users[id];
* if (!user) return ctx.status = 404;
* next();
* })
* .get('/users/:user', function (ctx, next) {
* ctx.body = ctx.user;
* });
* ```
*
* @param {String} param param string
* @param {Function} fn middleware function
* @return {Layer} layer instance
* @private
*/
param(param: string, fn: ParamMiddlewareFunc): Layer {
const stack = this.stack;
const params = this.paramNames;
const middleware: MiddlewareFuncWithParamProperty = function(this: any, ctx, next) {
return fn.call(this, ctx.params[param], ctx, next);
};
middleware.param = param;

const names = params.map(p => {
return p.name;
});

const x = names.indexOf(param);
if (x > -1) {
// iterate through the stack, to figure out where to place the handler fn
stack.some(function(fn, i) {
// param handlers are always first, so when we find an fn w/o a param property, stop here
// if the param handler at this part of the stack comes after the one we are adding, stop here
if (!fn.param || names.indexOf(fn.param) > x) {
// inject this param handler right before the current item
stack.splice(i, 0, middleware);
return true; // then break the loop
}
return false;
});
}

return this;
}

/**
* Prefix route path.
*
* @param {String} prefix prefix string
* @return {Layer} layer instance
* @private
*/
setPrefix(prefix: string): Layer {
if (this.path) {
this.path = prefix + this.path;
this.paramNames = [];
this.regexp = pathToRegExp(this.path, this.paramNames, this.opts);
}
return this;
}
Copy link

Choose a reason for hiding this comment

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

The param and setPrefix methods are correctly implemented. Consider using arrow functions for internal function expressions to reduce complexity.

- stack.some(function(fn, i) {
+ stack.some((fn, i) => {
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
/**
* Run validations on route named parameters.
*
* @example
*
* ```javascript
* router
* .param('user', function (id, ctx, next) {
* ctx.user = users[id];
* if (!user) return ctx.status = 404;
* next();
* })
* .get('/users/:user', function (ctx, next) {
* ctx.body = ctx.user;
* });
* ```
*
* @param {String} param param string
* @param {Function} fn middleware function
* @return {Layer} layer instance
* @private
*/
param(param: string, fn: ParamMiddlewareFunc): Layer {
const stack = this.stack;
const params = this.paramNames;
const middleware: MiddlewareFuncWithParamProperty = function(this: any, ctx, next) {
return fn.call(this, ctx.params[param], ctx, next);
};
middleware.param = param;
const names = params.map(p => {
return p.name;
});
const x = names.indexOf(param);
if (x > -1) {
// iterate through the stack, to figure out where to place the handler fn
stack.some(function(fn, i) {
// param handlers are always first, so when we find an fn w/o a param property, stop here
// if the param handler at this part of the stack comes after the one we are adding, stop here
if (!fn.param || names.indexOf(fn.param) > x) {
// inject this param handler right before the current item
stack.splice(i, 0, middleware);
return true; // then break the loop
}
return false;
});
}
return this;
}
/**
* Prefix route path.
*
* @param {String} prefix prefix string
* @return {Layer} layer instance
* @private
*/
setPrefix(prefix: string): Layer {
if (this.path) {
this.path = prefix + this.path;
this.paramNames = [];
this.regexp = pathToRegExp(this.path, this.paramNames, this.opts);
}
return this;
}
/**
* Run validations on route named parameters.
*
* @example
*
* ```javascript
* router
* .param('user', function (id, ctx, next) {
* ctx.user = users[id];
* if (!user) return ctx.status = 404;
* next();
* })
* .get('/users/:user', function (ctx, next) {
* ctx.body = ctx.user;
* });
* ```
*
* @param {String} param param string
* @param {Function} fn middleware function
* @return {Layer} layer instance
* @private
*/
param(param: string, fn: ParamMiddlewareFunc): Layer {
const stack = this.stack;
const params = this.paramNames;
const middleware: MiddlewareFuncWithParamProperty = function(this: any, ctx, next) {
return fn.call(this, ctx.params[param], ctx, next);
};
middleware.param = param;
const names = params.map(p => {
return p.name;
});
const x = names.indexOf(param);
if (x > -1) {
// iterate through the stack, to figure out where to place the handler fn
stack.some((fn, i) => {
// param handlers are always first, so when we find an fn w/o a param property, stop here
// if the param handler at this part of the stack comes after the one we are adding, stop here
if (!fn.param || names.indexOf(fn.param) > x) {
// inject this param handler right before the current item
stack.splice(i, 0, middleware);
return true; // then break the loop
}
return false;
});
}
return this;
}
/**
* Prefix route path.
*
* @param {String} prefix prefix string
* @return {Layer} layer instance
* @private
*/
setPrefix(prefix: string): Layer {
if (this.path) {
this.path = prefix + this.path;
this.paramNames = [];
this.regexp = pathToRegExp(this.path, this.paramNames, this.opts);
}
return this;
}
Tools
Biome

[error] 244-253: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.

@fengmk2 fengmk2 mentioned this pull request Jun 17, 2024
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant