feat(decorators): update @routup/decorators for routup v5#764
Merged
feat(decorators): update @routup/decorators for routup v5#764
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Router as Router (IRouter)
participant Core as defineCoreHandler
participant Args as buildDecoratorMethodArguments
participant Controller as Controller Method
Client->>Router: fetch(Request)
Router->>Core: matched handler(event)
Core->>Args: buildDecoratorMethodArguments({ event }, parameters)
Args-->>Core: Promise<arguments>
Core->>Controller: controllerMethod(...arguments)
Controller-->>Core: return value
Core-->>Router: resolve response
Router-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/decorators/test/unit/header.spec.ts (1)
6-9: Consider centralizingcreateTestRequest()in a shared test util.This helper is repeated across multiple spec files; extracting it once will reduce duplication and keep request construction behavior consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/decorators/test/unit/header.spec.ts` around lines 6 - 9, Extract the createTestRequest function into a single shared test utility module and export it (preserving the same signature: createTestRequest(url: string, options?: RequestInit): Request and the same fullUrl logic), then replace the duplicated local helpers in all spec files (including header.spec.ts) with imports from that shared util; ensure the shared module imports/exports any required DOM/Request types so existing tests compile and update test imports to reference the new exported function name.packages/decorators/src/type.ts (1)
32-35: Model async extractors explicitly in the public type.
buildDecoratorMethodArguments()now awaits extractor results, butParameterExtractFnstill advertises a plainanyreturn. Tightening this to an awaitable return keeps the publicparametercontract aligned with the new async extractor path.♻️ Suggested typing update
export type ParameterExtractFn = ( context: HandlerContext, key?: string, -) => any; +) => Promise<unknown> | unknown;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/decorators/src/type.ts` around lines 32 - 35, ParameterExtractFn currently returns plain any but extractors can be async (buildDecoratorMethodArguments awaits them); change ParameterExtractFn's return type to an awaitable type such as Promise<any> or PromiseLike<any> (or use a MaybePromise<T> alias) so the public parameter contract reflects async extractors; update the type declaration for ParameterExtractFn accordingly and ensure any usages of ParameterExtractFn (e.g., in buildDecoratorMethodArguments) remain compatible.packages/decorators/test/data/get.ts (1)
1-1: Unused import:IRoutupEventThe
IRoutupEventtype is imported but never used in this file. Consider removing it.🧹 Proposed fix
-import type { IRoutupEvent } from 'routup'; import {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/decorators/test/data/get.ts` at line 1, The import IRoutupEvent in get.ts is unused and should be removed; edit the top-level import line that mentions IRoutupEvent and delete that identifier so the file no longer imports IRoutupEvent (keep any other imports intact) to eliminate the unused-type lint error.packages/decorators/test/data/combined.ts (1)
5-5: Unused import:DContext
DContextis imported but not used in any of the controller methods. Consider removing it unless it's needed for future additions.🧹 Proposed fix
import type { HandlerInterface } from '../../src'; import { - DContext, DController, DDelete,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/decorators/test/data/combined.ts` at line 5, Remove the unused import DContext from the file to eliminate dead code: locate the import list that includes DContext (the symbol "DContext" in the combined.ts import block) and delete that specific import entry; ensure no references to DContext exist elsewhere in the file and run tests/lint to confirm the unused-import warning is resolved.packages/decorators/README.md (1)
159-179: Consider clarifying the async nature of body extraction.The
bodyextractor is markedasyncand usesreadRequestBody, whilecookieandqueryextractors are synchronous and useuseRequestCookie/useRequestQuery. This is likely intentional since body parsing requires reading a stream, but a brief comment in the docs could help users understand why onlybodyis async.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/decorators/README.md` around lines 159 - 179, The README shows three extractors—body (async), cookie, and query—where body uses readRequestBody and is async while cookie/query use useRequestCookie/useRequestCookies and useRequestQuery synchronously; add a short clarifying comment in the docs next to the body extractor explaining that readRequestBody consumes a request stream and therefore is asynchronous (hence body is marked async), whereas cookie and query access parsed data synchronously via useRequestCookie/useRequestCookies and useRequestQuery so they do not need async behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/decorators/package.json`:
- Around line 54-60: Bump this package's semver major: update the package.json
"version" field from 3.4.3 to the next major (e.g., 4.0.0) to signal the
breaking v5-compatible API change for `@routup/decorators`, or if you intend CI to
auto-release, add a clear note in package.json or the PR describing that
automated release tooling will perform the major bump; ensure you update any
CHANGELOG/release notes referencing the `@routup/decorators` breaking changes
accordingly.
---
Nitpick comments:
In `@packages/decorators/README.md`:
- Around line 159-179: The README shows three extractors—body (async), cookie,
and query—where body uses readRequestBody and is async while cookie/query use
useRequestCookie/useRequestCookies and useRequestQuery synchronously; add a
short clarifying comment in the docs next to the body extractor explaining that
readRequestBody consumes a request stream and therefore is asynchronous (hence
body is marked async), whereas cookie and query access parsed data synchronously
via useRequestCookie/useRequestCookies and useRequestQuery so they do not need
async behavior.
In `@packages/decorators/src/type.ts`:
- Around line 32-35: ParameterExtractFn currently returns plain any but
extractors can be async (buildDecoratorMethodArguments awaits them); change
ParameterExtractFn's return type to an awaitable type such as Promise<any> or
PromiseLike<any> (or use a MaybePromise<T> alias) so the public parameter
contract reflects async extractors; update the type declaration for
ParameterExtractFn accordingly and ensure any usages of ParameterExtractFn
(e.g., in buildDecoratorMethodArguments) remain compatible.
In `@packages/decorators/test/data/combined.ts`:
- Line 5: Remove the unused import DContext from the file to eliminate dead
code: locate the import list that includes DContext (the symbol "DContext" in
the combined.ts import block) and delete that specific import entry; ensure no
references to DContext exist elsewhere in the file and run tests/lint to confirm
the unused-import warning is resolved.
In `@packages/decorators/test/data/get.ts`:
- Line 1: The import IRoutupEvent in get.ts is unused and should be removed;
edit the top-level import line that mentions IRoutupEvent and delete that
identifier so the file no longer imports IRoutupEvent (keep any other imports
intact) to eliminate the unused-type lint error.
In `@packages/decorators/test/unit/header.spec.ts`:
- Around line 6-9: Extract the createTestRequest function into a single shared
test utility module and export it (preserving the same signature:
createTestRequest(url: string, options?: RequestInit): Request and the same
fullUrl logic), then replace the duplicated local helpers in all spec files
(including header.spec.ts) with imports from that shared util; ensure the shared
module imports/exports any required DOM/Request types so existing tests compile
and update test imports to reference the new exported function name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d7c2ee5-1c6f-464c-82b7-d02b4c8d4154
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
packages/decorators/README.mdpackages/decorators/package.jsonpackages/decorators/src/method/arguments.tspackages/decorators/src/mount.tspackages/decorators/src/parameter/constants.tspackages/decorators/src/parameter/module.tspackages/decorators/src/parameter/type.tspackages/decorators/src/type.tspackages/decorators/src/utils/handler.tspackages/decorators/test/data/combined.tspackages/decorators/test/data/cookie.tspackages/decorators/test/data/get.tspackages/decorators/test/data/header.tspackages/decorators/test/data/middleware.tspackages/decorators/test/data/post.tspackages/decorators/test/data/query.tspackages/decorators/test/unit/body.spec.tspackages/decorators/test/unit/combined.spec.tspackages/decorators/test/unit/cookie.spec.tspackages/decorators/test/unit/header.spec.tspackages/decorators/test/unit/middleware.spec.tspackages/decorators/test/unit/query.spec.ts
💤 Files with no reviewable changes (1)
- packages/decorators/src/parameter/type.ts
Comment on lines
+54
to
+60
| "routup": "^5.0.0-beta.3" | ||
| }, | ||
| "devDependencies": { | ||
| "@routup/body": "^2.4.3", | ||
| "@routup/cookie": "^2.4.3", | ||
| "@routup/query": "^2.4.3", | ||
| "@types/supertest": "^7.2.0", | ||
| "routup": "^4.0.1", | ||
| "supertest": "^7.1.4" | ||
| "routup": "^5.0.0-beta.3" |
There was a problem hiding this comment.
Major-version release step appears incomplete.
This PR introduces a v5 compatibility/breaking API migration, but @routup/decorators is still 3.4.3. Please bump to the next major (or document that automated release tooling will do it), otherwise consumers won’t get proper semver signaling.
Suggested semver bump
- "version": "3.4.3",
+ "version": "4.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/decorators/package.json` around lines 54 - 60, Bump this package's
semver major: update the package.json "version" field from 3.4.3 to the next
major (e.g., 4.0.0) to signal the breaking v5-compatible API change for
`@routup/decorators`, or if you intend CI to auto-release, add a clear note in
package.json or the PR describing that automated release tooling will perform
the major bump; ensure you update any CHANGELOG/release notes referencing the
`@routup/decorators` breaking changes accordingly.
Merged
This was referenced May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #745
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
Chores