feat: split swagger package in ui, generator and move preset#783
Merged
feat: split swagger package in ui, generator and move preset#783
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR splits the monolithic ChangesPackage Split & Architecture Refactor
Sequence DiagramsequenceDiagram
actor Dev
participant Generator as "@routup/swagger-generator"
participant Decorators as "@routup/decorators/preset"
participant TRAPI as "@trapi/swagger"
participant UI as "@routup/swagger-ui"
Dev->>Generator: generate(options)
Generator->>Generator: merge DEFAULT_DATA with options.data
alt metadata.preset missing
Generator->>Decorators: buildPreset()
Decorators-->>Generator: Preset (name="@routup/decorators")
end
Generator->>TRAPI: generateSwagger({version, metadata, data})
TRAPI-->>Generator: OpenAPI spec (v2/v3)
Generator-->>Dev: OutputForVersion<V>
Dev->>UI: swaggerUI(specOrUrl, options)
UI->>UI: createUIHandler(...)
UI-->>Dev: Handler (mountable on Router)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
tada5hi
changed the title
…r-ui
UI-only consumers no longer pull `@trapi/metadata` and `@trapi/swagger`
(both transitively need TypeScript), and generator-only consumers (CI
build steps) no longer pull `swagger-ui-dist` and its ~10MB of static
assets. Each half is independently versionable; they're kept aligned
via a release-please linked-versions group.
Layout:
- `@routup/swagger-generator` — `generate()`, `Version`, `buildPreset()`,
bundled metadata preset (`packages/swagger-generator/src/{generator,preset}/`).
No runtime routup dep.
- `@routup/swagger-ui` — `swaggerUI()`, `createUIHandler()`, URL helpers,
`swagger-ui-dist` assets (`packages/swagger-ui/src/`,
`packages/swagger-ui/assets/template.tpl`).
The internal preset name moved from `@routup/swagger` to
`@routup/swagger-generator` (only relevant if you cache by preset name).
BREAKING CHANGE: `@routup/swagger` is removed. Replace
`import { generate, Version, buildPreset } from '@routup/swagger'` with
`@routup/swagger-generator`, and `import { swaggerUI } from '@routup/swagger'`
with `@routup/swagger-ui`. Install whichever halves you need.
… API
The metadata preset now lives where it's most useful — next to the
decorators it decodes — and is published as the `./preset` subpath
export of `@routup/decorators`. Any trapi-based tool (the `trapi` CLI,
custom generators) can consume it directly via
`--preset @routup/decorators/preset` without depending on
`@routup/swagger-generator`.
`@routup/decorators` gains `@trapi/metadata` as an *optional* peer
dependency (peerDependenciesMeta.optional = true) — runtime-only users
who never touch the preset don't need it installed.
`@routup/swagger-generator` is now a thin wrapper:
- `generate()` takes `SwaggerGenerateOptions` directly
(`{ version, metadata, data }`), the same shape as
`@trapi/swagger`'s `generateSwagger()` — no more `GeneratorContext`
wrapper, no more `options.metadata` / `options.<data fields>` mix.
- `buildPreset()` is re-exported from `@routup/decorators/preset` so
it's still a single import for typical consumers, and the rest of
`@trapi/swagger`'s public API (`Version`, `SpecV2`/`V3`,
`saveSwagger`, types) is re-exported via `export * from
'@trapi/swagger'`.
- The wrapper applies the same defaults as before (entryPoint =
`<cwd>/src/**/*.ts`, ignore node_modules, preset = buildPreset(),
version = V3_2, default data name/description/consumes/produces).
BREAKING CHANGE: `@routup/swagger-generator` no longer exports
`GeneratorContext`, `GeneratorOptionsInput`, or `GeneratorOutput`, and
`generate()`'s call shape changed. Migrate from
`generate({ version, options: { metadata, ...data } })` to
`generate({ version, metadata, data })`. The internal preset name
moved from `@routup/swagger-generator` to `@routup/decorators` (only
relevant if you cache by preset name).
tada5hi
force-pushed
the
feat/swagger-bundle-preset
branch
from
3b3f7e4 to
f7a6a87
Compare
There was a problem hiding this comment.
Pull request overview
This PR splits the former Swagger plugin into two focused packages—@routup/swagger-generator (OpenAPI generation via TRAPI + routup decorators preset) and @routup/swagger-ui (mounting Swagger UI from a spec/path/URL)—and updates release + docs metadata accordingly.
Changes:
- Replace
@routup/swagger/@routup/swagger-presetwith@routup/swagger-generatorand@routup/swagger-uiin release-please config, manifest, and root README. - Introduce new swagger-ui implementation pieces (handler/module/utils/url/type, assets template) plus Vitest setup and fixtures.
- Move the TRAPI preset into
@routup/decoratorsas a./presetsubpath export and update generator defaults/tests to use it.
Reviewed changes
Copilot reviewed 40 out of 60 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| release-please-config.json | Swap old swagger packages to new generator/ui components + linked-versions group update. |
| README.md | Update plugin list to reference swagger-generator and swagger-ui. |
| packages/swagger-ui/* | New Swagger UI package structure, handler/utilities, assets template, and tests. |
| packages/swagger-generator/* | New generator package structure, generate() API update, and tests. |
| packages/decorators/* | Add ./preset entry build/export and implement preset handlers in decorators package. |
| packages/docs/src/* | Update docs to reflect new package split and navigation entries. |
| package-lock.json | Update lockfile for new workspaces/deps (currently inconsistent). |
| nx.json | Formatting + disable analytics. |
| .release-please-manifest.json | Manifest now tracks swagger-generator and swagger-ui packages. |
| .agents/* | Update repo structure + conventions + TRAPI reference docs for the new split. |
Comments suppressed due to low confidence (1)
packages/swagger-ui/src/handler.ts:20
- createUIHandler() uses isFileURL() to decide whether to read the document from disk, but Node fs.readFileSync() won’t accept a "file://..." string path. Since isFileURL() currently returns true for file URLs (and the unit test asserts this), passing a file URL string as the document will be classified as “file” but then fail at runtime when reading. Fix by converting file URL strings to a URL object / file path (e.g. fileURLToPath(new URL(document)) or pass a URL object into readFileSync).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
packages/swagger-ui/test/vitest.config.ts (1)
11-16: 💤 Low valueCoverage thresholds, especially
branches: 58, are quite low for a new package.58% branch coverage leaves a significant portion of conditional logic untested. Consider raising these thresholds closer to the other metrics (lines/statements at 73%) or adding a tracking issue so they improve over time rather than codifying a low baseline.
🤖 Prompt for 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. In `@packages/swagger-ui/test/vitest.config.ts` around lines 11 - 16, The branch coverage threshold in the Vitest config's thresholds object (key: branches) is set too low (58%); update the thresholds.branches value to be closer to lines/statements (e.g., 73%) or create/link a tracking issue and add a comment in the same vitest.config.ts indicating this is temporary and listing the issue number so the low baseline isn't codified indefinitely; modify the thresholds object in vitest.config.ts accordingly and ensure CI will fail if coverage drops below the new value.packages/swagger-ui/src/index.ts (1)
1-8: 💤 Low valueIntermediate import only needed for the default re-export — can be collapsed.
Line 1's
importexists purely to power Line 8'sexport default. A re-export alias removes the extra step:♻️ Proposed simplification
-import { swaggerUI } from './module'; - export * from './handler'; export * from './module'; export * from './type'; export * from './utils'; - -export default swaggerUI; +export { swaggerUI as default } from './module';🤖 Prompt for 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. In `@packages/swagger-ui/src/index.ts` around lines 1 - 8, Remove the intermediate import of swaggerUI and collapse it into a single re-export: replace the standalone import of swaggerUI (currently used only to power the default export) with a direct re-export from './module' so the module still exports everything from './handler', './module', './type', './utils' and also exposes swaggerUI as the default export; target the symbol swaggerUI from './module' and eliminate the unused import line.
🤖 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/decorators/package.json`:
- Around line 58-65: Update the peer dependency range for "@trapi/metadata" in
package.json so it will accept future pre-releases on the 2.x track; replace the
current "^2.0.0-beta.2" value with a more inclusive range such as
">=2.0.0-beta.2 <3.0.0" (or switch to "^2.0.0" once stable) in the
peerDependencies section so consumers running later 2.x betas (e.g.,
2.0.1-beta.1 or 2.1.0-beta.1) won't get false peer-dep warnings.
In `@packages/docs/src/swagger-generator/index.md`:
- Around line 30-44: The example broadens file discovery by setting
metadata.entryPoint.cwd to process.cwd() with pattern '**/*.ts'; either remove
the explicit entryPoint to use the generator's default scan root or narrow the
entryPoint to target the src tree (e.g., set metadata.entryPoint.cwd to
process.cwd() and metadata.entryPoint.pattern to 'src/**/*.ts'), updating the
example around generate and Version to ensure the snippet only scans source
files not repo-wide tests/configs.
In `@packages/swagger-generator/README.md`:
- Around line 47-58: The V2 README example uses process.cwd() but omits the
required ESM import; add "import process from 'node:process'" at the top of the
V2 snippet so the example that calls process.cwd() compiles in strict
ESM/no-global environments and matches the V3 example; update the code block
that shows generate(...) with Version.V2 (symbols: generate, Version,
process.cwd) to include that import.
In `@packages/swagger-generator/src/generate.ts`:
- Around line 23-27: The generic safety bug stems from GenerateOptions<V>
allowing an optional version, letting callers pick a non-default generic (e.g.,
V2) but pass no version at runtime; fix by making version required for the
generic parameter: change GenerateOptions<V extends `${Version}` = `${Version}`>
to require version: V (i.e., remove the optional from { version?: V } -> {
version: V }), and update generate<V = typeof Version.V3_2>(options:
GenerateOptions<V>) to use options.version directly (remove or avoid the runtime
fallback to Version.V3_2 and the unsafe as V cast); this ensures
OutputForVersion<V> matches the actual runtime options.version and preserves
type soundness for functions like generate and the OutputForVersion return type.
- Around line 28-29: The merge currently created with createMerger and used in
merge({}, DEFAULT_DATA, options.data || {}) prevents callers from replacing
default arrays because arrayDistinct/priority cause DEFAULT_DATA to win; to fix,
either create the merger with explicit options that allow overrides (e.g., set
array: false and priority: 'right' on the createMerger call where createMerger
is invoked) or flip the merge order so caller data is the rightmost source
(e.g., call merge({}, options.data || {}, DEFAULT_DATA)), ensuring DEFAULT_DATA,
merge, createMerger, and options.data/SwaggerGenerateData are the referenced
symbols to update.
In `@packages/swagger-ui/package.json`:
- Around line 57-61: The package.json currently lists "@types/swagger-ui-dist"
under "dependencies", which is wrong for a types-only package; update
package.json in the packages/swagger-ui manifest so that
"@types/swagger-ui-dist" is removed from "dependencies" and placed either into
"devDependencies" if the types are only used internally by this package (no
re-exports), or into "peerDependencies" if the declarations are part of the
public API surface re-exported by this package (so consumers control the
version); verify in src/index.ts whether the types are re-exported to choose the
correct target.
In `@packages/swagger-ui/README.md`:
- Line 29: The README example uses an internal test fixture path in the
router.use('/docs', swaggerUI('test/data/swagger.json')) line; replace that
repository-internal path with a generic or clearly-labelled "local file" example
(e.g., 'path/to/swagger.json' or a note like 'local-file: ./openapi.yaml'), or
add a separate “from a local file” example block, so callers using the
swaggerUI(...) helper see a realistic, non-repo-specific file path; update the
example text near the router.use(...) and swaggerUI(...) invocation to reflect
this change.
- Around line 3-6: The README badges point to the wrong GitHub repository
(Tada5hi/routup); update the badge URLs in the README so they reference the
correct repository (routup/plugins) and package scope for CI, codecov and Snyk.
Concretely, change the GitHub Actions badge link that references
actions/workflows/main.yml, the codecov URL that uses branch/master and token,
and the Snyk test URL so they target routup/plugins instead of Tada5hi/routup;
ensure the npm badge remains scoped to `@routup/swagger-ui` if applicable.
---
Nitpick comments:
In `@packages/swagger-ui/src/index.ts`:
- Around line 1-8: Remove the intermediate import of swaggerUI and collapse it
into a single re-export: replace the standalone import of swaggerUI (currently
used only to power the default export) with a direct re-export from './module'
so the module still exports everything from './handler', './module', './type',
'./utils' and also exposes swaggerUI as the default export; target the symbol
swaggerUI from './module' and eliminate the unused import line.
In `@packages/swagger-ui/test/vitest.config.ts`:
- Around line 11-16: The branch coverage threshold in the Vitest config's
thresholds object (key: branches) is set too low (58%); update the
thresholds.branches value to be closer to lines/statements (e.g., 73%) or
create/link a tracking issue and add a comment in the same vitest.config.ts
indicating this is temporary and listing the issue number so the low baseline
isn't codified indefinitely; modify the thresholds object in vitest.config.ts
accordingly and ensure CI will fail if coverage drops below the new value.
🪄 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: 12c65398-bd94-48a6-b09a-ecee44d02fd8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (59)
.agents/conventions.md.agents/references/trapi.md.agents/structure.md.release-please-manifest.jsonREADME.mdpackages/decorators/package.jsonpackages/decorators/src/preset/class.tspackages/decorators/src/preset/index.tspackages/decorators/src/preset/method.tspackages/decorators/src/preset/module.tspackages/decorators/src/preset/parameter.tspackages/decorators/src/preset/swagger.tspackages/decorators/tsdown.config.tspackages/docs/src/.vitepress/config.mtspackages/docs/src/.vitepress/theme/components/PluginGrid.vuepackages/docs/src/decorators/index.mdpackages/docs/src/swagger-generator/index.mdpackages/docs/src/swagger-ui/index.mdpackages/docs/src/swagger/generator.mdpackages/docs/src/swagger/index.mdpackages/swagger-generator/LICENSEpackages/swagger-generator/README.mdpackages/swagger-generator/package.jsonpackages/swagger-generator/src/generate.tspackages/swagger-generator/src/index.tspackages/swagger-generator/test/unit/v2/module.spec.tspackages/swagger-generator/test/unit/v3/module.spec.tspackages/swagger-generator/test/vitest.config.tspackages/swagger-generator/tsconfig.build.jsonpackages/swagger-generator/tsconfig.jsonpackages/swagger-generator/tsdown.config.tspackages/swagger-ui/LICENSEpackages/swagger-ui/README.mdpackages/swagger-ui/assets/template.tplpackages/swagger-ui/package.jsonpackages/swagger-ui/src/constants.tspackages/swagger-ui/src/handler.tspackages/swagger-ui/src/index.tspackages/swagger-ui/src/module.tspackages/swagger-ui/src/object.tspackages/swagger-ui/src/type.tspackages/swagger-ui/src/url.tspackages/swagger-ui/src/utils.tspackages/swagger-ui/test/data/swagger.jsonpackages/swagger-ui/test/unit/ui/module.spec.tspackages/swagger-ui/test/unit/ui/utils.spec.tspackages/swagger-ui/test/vitest.config.tspackages/swagger-ui/tsconfig.build.jsonpackages/swagger-ui/tsconfig.jsonpackages/swagger-ui/tsdown.config.tspackages/swagger/CHANGELOG.mdpackages/swagger/README.mdpackages/swagger/src/generator/constants.tspackages/swagger/src/generator/index.tspackages/swagger/src/generator/type.tspackages/swagger/src/index.tspackages/swagger/src/utils/index.tspackages/swagger/writable/.gitignorerelease-please-config.json
💤 Files with no reviewable changes (11)
- packages/swagger/src/generator/constants.ts
- packages/swagger/writable/.gitignore
- packages/swagger/src/utils/index.ts
- packages/swagger/src/generator/index.ts
- packages/swagger-ui/tsconfig.json
- packages/docs/src/swagger/index.md
- packages/swagger/CHANGELOG.md
- packages/swagger/src/generator/type.ts
- packages/docs/src/swagger/generator.md
- packages/swagger/README.md
- packages/swagger/src/index.ts
Comment on lines
+3
to
+6
| [](https://badge.fury.io/js/@routup%2Fswagger-ui) | ||
| [](https://github.com/Tada5hi/routup/actions/workflows/main.yml) | ||
| [](https://codecov.io/gh/tada5hi/routup) | ||
| [](https://snyk.io/test/github/Tada5hi/routup) |
There was a problem hiding this comment.
Badge URLs point to the wrong repository.
Lines 4–6 reference Tada5hi/routup (the author's personal repo) instead of routup/plugins where this package actually lives. CI, codecov, and Snyk badges will show stale status from an unrelated repo.
🔧 Proposed fix
-[](https://github.com/Tada5hi/routup/actions/workflows/main.yml)
-[](https://codecov.io/gh/tada5hi/routup)
-[](https://snyk.io/test/github/Tada5hi/routup)
+[](https://github.com/routup/plugins/actions/workflows/main.yml)
+[](https://codecov.io/gh/routup/plugins)
+[](https://snyk.io/test/github/routup/plugins)📝 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
| [](https://badge.fury.io/js/@routup%2Fswagger-ui) | |
| [](https://github.com/Tada5hi/routup/actions/workflows/main.yml) | |
| [](https://codecov.io/gh/tada5hi/routup) | |
| [](https://snyk.io/test/github/Tada5hi/routup) | |
| [](https://badge.fury.io/js/@routup%2Fswagger-ui) | |
| [](https://github.com/routup/plugins/actions/workflows/main.yml) | |
| [](https://codecov.io/gh/routup/plugins) | |
| [](https://snyk.io/test/github/routup/plugins) |
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 963 characters long)
Context: .../badge.fury.io/js/@routup%2Fswagger-ui) 
(EN_EXCESSIVE_EXCLAMATION)
🤖 Prompt for 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.
In `@packages/swagger-ui/README.md` around lines 3 - 6, The README badges point to
the wrong GitHub repository (Tada5hi/routup); update the badge URLs in the
README so they reference the correct repository (routup/plugins) and package
scope for CI, codecov and Snyk. Concretely, change the GitHub Actions badge link
that references actions/workflows/main.yml, the codecov URL that uses
branch/master and token, and the Snyk test URL so they target routup/plugins
instead of Tada5hi/routup; ensure the npm badge remains scoped to
`@routup/swagger-ui` if applicable.
- generate(): switch to overload signatures so version is required when V
is non-default, eliminating the unsound `as V` cast that let
`generate<typeof Version.V2>({})` compile while returning a V3_2 spec
at runtime.
- generate(): swap the smob merger to `{ array: false, priority: 'right' }`
so callers can actually replace `name`, `description`, and the
`consumes`/`produces` arrays in `data` — the previous left-priority
array-merge silently appended to defaults instead of overriding.
- decorators: widen the new `@trapi/metadata` peer range to
`>=2.0.0-beta.2 <3.0.0` — caret + pre-release semver excludes future
pre-releases on a higher patch/minor (e.g., 2.0.1-beta.1) which would
produce false peer warnings.
- swagger-ui: move `@types/swagger-ui-dist` from `dependencies` to
`peerDependencies` (with a devDep mirror for typecheck) — the types
flow through the public `UIOptions` / `Spec` surface, so the consumer
should control the version.
- docs: drop the explicit `metadata.entryPoint = { cwd: process.cwd(),
pattern: '**/*.ts' }` from generator examples; the wrapper default
already scans `<cwd>/src/**/*.ts`, and the explicit form broadened
copy-paste discovery to repo-wide tests/configs.
- swagger-ui README: replace `test/data/swagger.json` (repo-internal
fixture) with `./openapi.json` so the example reads as a generic
caller path.
Repo-wide README badge URLs (`Tada5hi/routup` → `routup/plugins`)
deferred to issue #784 to keep this PR scoped.
…and trim re-exports
Renames the public entry point from `generate` to `generateSwagger` so
the wrapper reads as a routup-flavored variant of `@trapi/swagger`'s
upstream `generateSwagger()` — same name, same shape, plus the
routup-specific defaults. Also rename the source file
`src/generate.ts` → `src/generate-swagger.ts` to match.
Trim the public re-exports to the minimum needed for the
`generateSwagger()` call path:
- types: `OutputForVersion`, `SpecV2`, `SpecV3`,
`SwaggerGenerateData`, `SwaggerGenerateOptions`, `SwaggerSaveOptions`
(plus the metadata types for the `metadata` field)
- values: `Version`, `saveSwagger`
Anything else from `@trapi/swagger` (parameter variants, security
schemes, generator classes, low-level helpers) is now imported
directly from `@trapi/swagger`. Same for `buildPreset` — drop it from
`@routup/swagger-generator`'s surface so consumers import it from
`@routup/decorators/preset` (the canonical location), keeping the
seams clean.
BREAKING CHANGE: `generate()` is now `generateSwagger()`. The
`buildPreset` re-export from `@routup/swagger-generator` is removed —
`import { buildPreset } from '@routup/decorators/preset'` instead.
The wide re-export of `@trapi/swagger` is narrowed; types/values not
in the trimmed list above must now be imported from `@trapi/swagger`
directly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/decorators/package.json (1)
72-72: 💤 Low value
devDependenciesrange for@trapi/metadatais narrower than the peer range.The peer range (
>=2.0.0-beta.2 <3.0.0) now intentionally accepts future2.xpre-releases (e.g.2.0.1-beta.1,2.1.0-beta.1), but the devDependency^2.0.0-beta.2won't resolve those same versions —^on a pre-release tuple only covers pre-releases with the exact samemajor.minor.patchbase. This means local CI/builds won't exercise the package against newer2.xbetas as they ship, creating a subtle gap between what's tested and what's allowed by the peer range.🔧 Proposed fix
- "@trapi/metadata": "^2.0.0-beta.2", + "@trapi/metadata": ">=2.0.0-beta.2 <3.0.0",🤖 Prompt for 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. In `@packages/decorators/package.json` at line 72, The devDependency for `@trapi/metadata` in devDependencies is too narrow compared to the peerDependencies range; update the devDependencies entry for "@trapi/metadata" to match the peer range (for example use ">=2.0.0-beta.2 <3.0.0") so local CI/tests exercise the same 2.x beta releases the peer range allows, then regenerate your lockfile (npm/yarn install) to persist the change.packages/swagger-ui/package.json (1)
54-57: ⚡ Quick winMark
@types/swagger-ui-distas an optional peer dependency.
@types/swagger-ui-distlatest is 3.30.6, so the^3.30.5range is fine. However, listing it as a non-optional peer dependency means every consumer — including plain-JavaScript projects and TypeScript projects that don't need to use the types directly — will receive an unmet peer dependency warning if they don't install it. AddingpeerDependenciesMetawithoptional: trueis the standard pattern for@types/*peer deps.📦 Proposed fix
"peerDependencies": { "@types/swagger-ui-dist": "^3.30.5", "routup": "^5.0.0-beta.6" }, + "peerDependenciesMeta": { + "@types/swagger-ui-dist": { + "optional": true + } + }, "dependencies": {🤖 Prompt for 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. In `@packages/swagger-ui/package.json` around lines 54 - 57, Update package.json to mark the `@types` swagger peer as optional: in the same package.json that declares "peerDependencies" with "@types/swagger-ui-dist": "^3.30.5", add a "peerDependenciesMeta" object and set " `@types/swagger-ui-dist`": { "optional": true } (use the exact package name "@types/swagger-ui-dist" as the key) so consumers that don’t need the types won’t get unmet peer dependency warnings while preserving the existing version range.
🤖 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/swagger-generator/src/generate.ts`:
- Around line 26-31: The overloads for generate are unsound because a
GenerateOptions value with an optional version can match the no-version
overload; make them mutually exclusive by requiring the "no-version" overload to
explicitly forbid a version and the other to require it: change the first
overload signature to accept Omit<GenerateOptions,'version'> & { version?:
undefined } and keep the second as Omit<GenerateOptions,'version'> & { version:
V } (generic V) so generate(...) returns OutputForVersion<typeof Version.V3_2>
when no version is provided and OutputForVersion<V> when a concrete version is
supplied; update references to GenerateOptions, Version, OutputForVersion, and
the generate function accordingly.
---
Nitpick comments:
In `@packages/decorators/package.json`:
- Line 72: The devDependency for `@trapi/metadata` in devDependencies is too
narrow compared to the peerDependencies range; update the devDependencies entry
for "@trapi/metadata" to match the peer range (for example use ">=2.0.0-beta.2
<3.0.0") so local CI/tests exercise the same 2.x beta releases the peer range
allows, then regenerate your lockfile (npm/yarn install) to persist the change.
In `@packages/swagger-ui/package.json`:
- Around line 54-57: Update package.json to mark the `@types` swagger peer as
optional: in the same package.json that declares "peerDependencies" with
"@types/swagger-ui-dist": "^3.30.5", add a "peerDependenciesMeta" object and set
" `@types/swagger-ui-dist`": { "optional": true } (use the exact package name
"@types/swagger-ui-dist" as the key) so consumers that don’t need the types
won’t get unmet peer dependency warnings while preserving the existing version
range.
🪄 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: 1d549be2-92f4-4391-8082-249bf7baf7a2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
packages/decorators/package.jsonpackages/docs/src/swagger-generator/index.mdpackages/docs/src/swagger-ui/index.mdpackages/swagger-generator/README.mdpackages/swagger-generator/src/generate.tspackages/swagger-ui/README.mdpackages/swagger-ui/package.json
✅ Files skipped from review due to trivial changes (4)
- packages/swagger-ui/README.md
- packages/swagger-generator/README.md
- packages/docs/src/swagger-generator/index.md
- packages/docs/src/swagger-ui/index.md
Comment on lines
+26
to
+31
| export function generate( | ||
| options: Omit<GenerateOptions, 'version'>, | ||
| ): Promise<OutputForVersion<typeof Version.V3_2>>; | ||
| export function generate<V extends `${Version}`>( | ||
| options: Omit<GenerateOptions, 'version'> & { version: V }, | ||
| ): Promise<OutputForVersion<V>>; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TypeScript overload resolution: can a value typed as { version?: string }satisfy a parameter type omittingversion, and does that affect which overload is selected?
💡 Result:
Yes. A value of type { version?: string } can satisfy a parameter type that omits version (e.g. {} or an object type without a required version property), and that does not inherently force a different overload to be selected. 1) Why it satisfies the parameter type TypeScript uses structural typing: an object type is compatible with another as long as it has at least the required members of the target type; extra properties are allowed. So if the parameter type doesn’t require version, the optional version?: string property doesn’t prevent assignability. This is consistent with the general assignability/structural compatibility rules in the TS docs. [1] 2) Does that affect which overload is chosen? In overload resolution, TypeScript selects the overload whose parameter type is compatible with the argument type. If both the “version-omitting” parameter type and the argument type are compatible, then the argument can match that overload. There isn’t a special “optional property presence changes overload selection” rule; selection is driven by which overload signatures are applicable under assignability. [2] Important caveat There are known/long-standing edge cases where overload resolution can behave unexpectedly when one of the parameter types has only optional properties (see TypeScript issue #2634). That’s about overload matching quirks in the presence of “all-optional” object shapes, not about normal cases where the parameter simply omits a property. [3] So in the common case you asked about (parameter type omits version, argument type has version?: string), it should still match the “omitting version” overload; the overload selection should be unaffected by merely having an optional extra property. [1][2]
Citations:
- 1: https://typescriptlang.org/docs/handbook/type-compatibility.html
- 2: https://www.typescriptlang.org/docs/handbook/2/functions.html
- 3: Incorrect overloaded function resolution when a parameter's type has all optional properties microsoft/TypeScript#2634
🏁 Script executed:
# First, let's see the file and understand the context around lines 26-31 and 59
head -80 packages/swagger-generator/src/generate.ts | cat -nRepository: routup/plugins
Length of output: 2489
🏁 Script executed:
# Also check if GenerateOptions is defined in this file
grep -n "GenerateOptions" packages/swagger-generator/src/generate.ts | head -20Repository: routup/plugins
Length of output: 310
Overload typing is unsound for GenerateOptions-typed variables.
A variable typed as GenerateOptions structurally satisfies Omit<GenerateOptions, 'version'> (because having an optional version property is compatible with omitting it), so the first overload (lines 26-28) may be selected even when the variable carries a version property at runtime. This causes the return type to resolve to V3_2 while line 59 may actually generate a different version, violating type safety.
💡 Suggested type-safe overload shape
export type GenerateOptions = Partial<SwaggerGenerateOptions>;
+type GenerateOptionsWithoutVersion = Omit<GenerateOptions, 'version'> & { version?: never };
export function generate(): Promise<OutputForVersion<typeof Version.V3_2>>;
export function generate(
- options: Omit<GenerateOptions, 'version'>,
+ options: GenerateOptionsWithoutVersion,
): Promise<OutputForVersion<typeof Version.V3_2>>;
export function generate<V extends `${Version}`>(
options: Omit<GenerateOptions, 'version'> & { version: V },
): Promise<OutputForVersion<V>>;
+export function generate(
+ options: GenerateOptions,
+): Promise<OutputForVersion<`${Version}`>>;📝 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 function generate( | |
| options: Omit<GenerateOptions, 'version'>, | |
| ): Promise<OutputForVersion<typeof Version.V3_2>>; | |
| export function generate<V extends `${Version}`>( | |
| options: Omit<GenerateOptions, 'version'> & { version: V }, | |
| ): Promise<OutputForVersion<V>>; | |
| type GenerateOptionsWithoutVersion = Omit<GenerateOptions, 'version'> & { version?: never }; | |
| export function generate(): Promise<OutputForVersion<typeof Version.V3_2>>; | |
| export function generate( | |
| options: GenerateOptionsWithoutVersion, | |
| ): Promise<OutputForVersion<typeof Version.V3_2>>; | |
| export function generate<V extends `${Version}`>( | |
| options: Omit<GenerateOptions, 'version'> & { version: V }, | |
| ): Promise<OutputForVersion<V>>; | |
| export function generate( | |
| options: GenerateOptions, | |
| ): Promise<OutputForVersion<`${Version}`>>; |
🤖 Prompt for 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.
In `@packages/swagger-generator/src/generate.ts` around lines 26 - 31, The
overloads for generate are unsound because a GenerateOptions value with an
optional version can match the no-version overload; make them mutually exclusive
by requiring the "no-version" overload to explicitly forbid a version and the
other to require it: change the first overload signature to accept
Omit<GenerateOptions,'version'> & { version?: undefined } and keep the second as
Omit<GenerateOptions,'version'> & { version: V } (generic V) so generate(...)
returns OutputForVersion<typeof Version.V3_2> when no version is provided and
OutputForVersion<V> when a concrete version is supplied; update references to
GenerateOptions, Version, OutputForVersion, and the generate function
accordingly.
Merged
This was referenced May 5, 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
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.
Summary by CodeRabbit
New Features
Documentation
Chores