fix: discover generated controllers by metadata - #688
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthrough
Changes메타데이터 기반 컨트롤러 발견 및 CLI 로딩 로직 분리
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (openapi-spec / rpc-codegen)
participant Loader as loadControllers / loadRoutes
participant Project as ts-morph Project
participant TmpDir as 임시 emit 디렉터리
participant DC as discoverControllerConstructors
participant Meta as reflect-metadata
CLI->>Loader: loadControllers(glob) / loadRoutes(glob)
Loader->>Project: addSourceFilesFromGlob(glob)
Project-->>Loader: SourceFile[]
Loader->>TmpDir: emit → .js 파일 작성
loop 각 emitted 모듈
Loader->>TmpDir: import(file://*.js)
TmpDir-->>Loader: moduleExports
Loader->>DC: discoverControllerConstructors(moduleExports)
DC->>Meta: Reflect.getMetadata(REST_CONTROLLER_KEY, fn)
Meta-->>DC: 메타데이터 유무
DC-->>Loader: Constructor[]
end
Loader-->>CLI: Controller[] / RouteIR[]
Loader->>TmpDir: rm(tmpDir) [finally]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
📊 Benchmark Results❌ Some benchmarks failed
Updated: 2026-06-14T13:14:01.685Z · Commit: 9b45a4a |
9cf9fbe to
2ba4ecc
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/openapi-spec/src/libs/loadControllers.ts`:
- Around line 48-52: Replace the generic Error instance being thrown when no
REST controllers are found (in the block checking if controllers.length === 0)
with an appropriate Problem subclass. Instead of throwing new Error with the
message about no exported REST controllers found for the glob pattern,
instantiate and throw the appropriate Problem subclass that handles this
scenario, ensuring compliance with RFC 7807 Problem-based error handling
guidelines.
- Around line 23-27: Replace the generic Error being thrown in the
sourceFiles.length === 0 check with an appropriate Problem subclass that
conforms to RFC 7807 Problem-based error handling. Import the suitable Problem
subclass at the top of the file, then instantiate and throw that Problem
subclass instead of the generic Error, while preserving the existing error
message about no exported REST controllers being found for the given glob
pattern.
- Line 8: Replace the `Controller` type definition that currently uses
`Function` with the `Constructor` type imported from `@croco/protocols-core`.
This change improves type safety and consistency. After making this change,
locate the double type assertion `as unknown as Controller` at lines 42-44 and
remove it, since the proper `Constructor` type will eliminate the need for the
intermediate `unknown` type assertion.
In `@packages/openapi-spec/src/tests/loadControllers.spec.ts`:
- Around line 9-10: Add definite assignment assertions (!) to the test instance
variables declared in the describe block. Modify both tempRoot and sourceDir
variable declarations to include the ! operator after the variable name and
before the type annotation, so they read as let tempRoot!: string and let
sourceDir!: string. This follows the coding guideline requirement for test
instance variables declared in describe blocks.
In `@packages/rpc-codegen/src/libs/loadRoutes.ts`:
- Around line 69-83: The getCommonSourceDir function incorrectly handles
absolute paths on Windows by checking if commonDir starts with path.sep and
prepending it if it doesn't. This fails on Windows where absolute paths start
with drive letters (e.g., C:\project) rather than a separator, resulting in
malformed paths like \C:\project. Replace the path.sep check with
path.isAbsolute(commonDir) which correctly identifies absolute paths across
platforms regardless of whether they start with path.sep. This will prevent
incorrectly prepending a separator to paths that are already absolute.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc2e40fe-fa5f-424b-b2ce-7fd3b6207798
📒 Files selected for processing (10)
.changeset/codegen-controller-discovery.mdpackages/openapi-spec/src/cli.tspackages/openapi-spec/src/libs/loadControllers.tspackages/openapi-spec/src/tests/loadControllers.spec.tspackages/protocols-core/src/index.tspackages/protocols-core/src/libs/controllerDiscovery.tspackages/protocols-core/src/tests/controllerDiscovery.spec.tspackages/rpc-codegen/src/cli.tspackages/rpc-codegen/src/libs/loadRoutes.tspackages/rpc-codegen/src/tests/loadRoutes.spec.ts
2ba4ecc to
851d1cf
Compare
851d1cf to
880217b
Compare
Summary
OpenAPI and RPC codegen now discover exported REST controllers from controller metadata instead of importing every class declaration by name, so co-located DTO/helper classes are ignored and controller globs fail clearly when no exported controller is found.
Fixes #663
변경 사항
@croco/protocols-coreusing REST controller metadata.@croco/protocols-core,@croco/openapi-spec, and@croco/rpc-codegen.Verification
pnpm --filter @croco/protocols-core exec vitest run src/tests/controllerDiscovery.spec.tspnpm --filter @croco/openapi-spec exec vitest run src/tests/loadControllers.spec.tspnpm --filter @croco/rpc-codegen exec vitest run src/tests/loadRoutes.spec.tspnpm --filter @croco/openapi-spec testpnpm --filter @croco/rpc-codegen testpnpm --filter @croco/protocols-core testpnpm --filter @croco/protocols-core typecheckpnpm --filter @croco/openapi-spec typecheckpnpm --filter @croco/rpc-codegen typecheckpnpm --filter @croco/protocols-core buildpnpm --filter @croco/openapi-spec buildpnpm --filter @croco/rpc-codegen buildpnpm checkpnpm typecheckpnpm testpnpm buildpnpm exec changeset status --since origin/trunkoxfmt,oxlintauto-changeset,test,typecheckSelf-review
Risk
Users who passed empty or non-controller-only globs to codegen will now receive a hard diagnostic and must point the generator at exported
@Controllerclasses.Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
테스트