-
Notifications
You must be signed in to change notification settings - Fork 22
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
Expose JavaScript API for generating OpenAPI schema #160
Conversation
This change introduces a method of generating an OpenAPI schema from the JS API. Additionally, we introduce the ability to supply overrides to the esbuild options.
This change updates the example project with a custom OpenAPI generator script, which fixes the example using JSDOM.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -0,0 +1,25 @@ | |||
import { TypedNextResponse, route, routeOperation } from 'next-rest-framework'; | |||
import { z } from 'zod'; | |||
import { JSDOM } from 'jsdom'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example using jsdom
that is now marked as external
in our custom openapi-generate.ts
file.
@@ -0,0 +1 @@ | |||
export * from './src/generate'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This root generate
file allows us to import with next-rest-framework/generate
instead of exporting directly from the index file, which prevents esbuild
from being included in the main bundle. Open to suggestions, but I don't think we want to include esbuild
in the main bundle.
@@ -227,6 +236,17 @@ export const findConfig = async ({ configPath }: { configPath?: string }) => { | |||
return config; | |||
}; | |||
|
|||
async function dynamicallyImportRoute(filePathToRoute: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was needed to work with ts-node
. Importing using the file://
path wasn't resolving the module correctly. Open to suggestion here as well.
// by esbuild to avoid bundling it in the output. | ||
// | ||
// ref: https://github.com/evanw/esbuild//issues/2698#issuecomment-1325160925 | ||
return ['esbuild', ...(buildOptions.external ?? [])]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is now part of a JS API, we don't want esbuild
to consider esbuild
in its output. Otherwise, we receive a similar warning as the jsdom
warning. See comment above.
Compiling endpoints...
▲ [WARNING] "esbuild" should be marked as external for use with "require.resolve" [require-resolve-not-external]
../../node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1825:36:
1825 │ const libMainJS = require.resolve("esbuild");
try { | ||
await compileEndpoints({ | ||
buildOptions: { | ||
external: ['jsdom'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this + example route below for demonstrating the issue + testing.
I'm realizing now that this doesn't quite work the way that I'd like when the module is actually published. The |
Thanks for looking into this, I posted a more detailed answer here, but I have a slightly different approach to this fix in #161. I have also included your changes on the example application on that PR and set you as the co-author so you can expect those changes along with the fix for the ESBuild issues to be published soon. |
Fixes #159
I created individual commits that express the changes more cleanly.