add generate openapi script to support all APIs generating OpenAPI specs#1738
add generate openapi script to support all APIs generating OpenAPI specs#1738
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe OpenAPI generation workflow was refactored to use a CLI-driven script with dynamic output path configuration. The package.json script now invokes a modified generate-openapi.ts that accepts a required --out argument instead of using hardcoded paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
Updates the monorepo’s OpenAPI generation workflow by making the generator script app-agnostic, so any app can produce an OpenAPI 3.1 spec as long as it exposes an @/openapi-document module (supporting issue #1601’s direction for shared OpenAPI tooling).
Changes:
- Generalizes the OpenAPI generator script to require an explicit
--outpath and resolve it from the executing app’s working directory. - Updates the Mintlify docs
generate:openapiscript to call the new generic generator with the appropriate output path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/generate-openapi.ts | Adds --out CLI handling and removes ENSApi-specific hardcoded output path to make the script reusable across apps. |
| docs/docs.ensnode.io/package.json | Updates the docs OpenAPI generation command to use the new script and pass --out explicitly. |
Comments suppressed due to low confidence (2)
scripts/generate-openapi.ts:19
- The header comment says this script has “no runtime dependencies”, but it still requires external CLIs (
pnpmandbiome) at runtime for the formatting step. Consider rewording to clarify it has no app/runtime API deps, but does require these tools to be available on PATH.
scripts/generate-openapi.ts:6 - PR description says the script still uses Prettier formatting, but this script formats the output with Biome (
biome format). Please update the PR description (or the script docs) to avoid confusion for future contributors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/generate-openapi.ts (1)
54-73: 🧹 Nitpick | 🔵 TrivialConsider using the typed error from
child_process.The type assertion on line 61 works but is slightly awkward. Node.js
execFileSyncthrows an error with additional properties (status,signal,stderr, etc.) when the subprocess fails. You could simplify this by using the actual type.♻️ Optional: Use a more explicit type guard
} catch (error) { console.error("Error: Failed to format with Biome."); if (error instanceof Error) { - const err = error as NodeJS.ErrnoException & { status?: number }; + const err = error as NodeJS.ErrnoException & { status?: number; signal?: string }; if (err.code === "ENOENT") { console.error("'pnpm' is not available on your PATH."); } else if (err.status !== undefined) { console.error(`Biome exited with code ${err.status}.`); console.error( `Try running 'pnpm -w exec biome format --write ${outputPath}' manually to debug.`, ); + } else if (err.signal) { + console.error(`Biome was killed by signal ${err.signal}.`); } else if (err.message) { console.error(err.message); } } process.exit(1); }Alternatively, you could keep it as-is since the current handling covers the most common cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-openapi.ts` around lines 54 - 73, Replace the ad-hoc type assertion for the caught error with the standard child_process ExecException type: in the catch block for execFileSync (around the error handling for execFileSync/biome), cast the caught error to import('child_process').ExecException (or import ExecException from 'child_process') instead of NodeJS.ErrnoException & { status?: number }, then use its properties (code, status, message, stderr) in the existing checks (the caught error variable and the checks for ENOENT, status, and message should reference ExecException) so you use the proper typed error for subprocess failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/generate-openapi.ts`:
- Around line 54-73: Replace the ad-hoc type assertion for the caught error with
the standard child_process ExecException type: in the catch block for
execFileSync (around the error handling for execFileSync/biome), cast the caught
error to import('child_process').ExecException (or import ExecException from
'child_process') instead of NodeJS.ErrnoException & { status?: number }, then
use its properties (code, status, message, stderr) in the existing checks (the
caught error variable and the checks for ENOENT, status, and message should
reference ExecException) so you use the proper typed error for subprocess
failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bd97db2d-5e12-40d9-ae30-5b40d9033bef
📒 Files selected for processing (2)
docs/docs.ensnode.io/package.jsonscripts/generate-openapi.ts
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
scriptsandopenapifolders to the monorepo #1601Why
generate-ensapi-openapi.tsscript was hardcoded to ENSApi with a fixed output path.@/openapi-documentmodule.--outCLI flag instead of being baked in.Testing
generate:openapinpm script indocs/docs.ensnode.io/package.jsonstill produces the sameensapi-openapi.jsonoutput with the updated command.Notes for Reviewer (Optional)
generate-ensapi-openapi.tstogenerate-openapi.tsto reflect its generic.--outflag is resolved relative to the app's root directory (i.e. the cwd when executed viapnpm --filter <app> exec).generateOpenApi31Document()import and prettier formatting.Pre-Review Checklist (Blocking)