Skip to content
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

feat: experimental defineRouteMeta #2102

Merged
merged 35 commits into from
Apr 10, 2024
Merged

feat: experimental defineRouteMeta #2102

merged 35 commits into from
Apr 10, 2024

Conversation

eriksLapins
Copy link
Contributor

@eriksLapins eriksLapins commented Jan 20, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

I added the option to change the default provided schema directly in the server route file with defineRouteMeta, which can take in any key: value pair, with one defined in this PR - openAPI operations object, which will amend the default schema with types inferred from OperationsObject in openapi-typescript

This solves the issue that we can get better looking openapi documentation with better support

Possibly resolves the problem shown in discussion #2056

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@eriksLapins
Copy link
Contributor Author

Also, I wanted to add a test for the defineOpenAPISchema function, but it errors out with
Failed to load url #internal/nitro/virtual/server-handlers if I load in the function as below

or Failed to load url #internal/nitro if I load the added function from #internal/nitro/routes/openapi

My idea was to add an additional preset with something like:

import { describe, expect, it } from "vitest";
import { defineOpenAPISchema } from "../../src/runtime/routes/openapi";
import { useStorage } from "#internal/nitro";


describe("nitro:openapi:schema-definition", () => {
    it("stores the result in useStorage", async () => {
        defineOpenAPISchema({
            routeBase: '/api/test',
            method: 'get',
            schema: {
                tags: ["test"]
            }
        })

        const storage = await useStorage("/api/test").getItem('openapi-get');

        expect(storage).toBe({
            routeBase: '/api/test',
            method: 'get',
            schema: {
                tags: ["test"]
            }
        })
    })
})

examples/api-routes/api/test.get.ts Outdated Show resolved Hide resolved
examples/api-routes/api/test.post.ts Outdated Show resolved Hide resolved
src/runtime/routes/openapi.ts Outdated Show resolved Hide resolved
Copy link
Contributor

nuxt-studio bot commented Jan 22, 2024

βœ… Live Preview ready!

Name Edit Preview Latest Commit
nitro Edit on Studio β†—οΈŽ View Live Preview 21264e2

@eriksLapins eriksLapins requested a review from pi0 January 25, 2024 06:08
@pi0 pi0 added this to the 2.10 milestone Feb 27, 2024
@pi0
Copy link
Member

pi0 commented Feb 27, 2024

This looks really promising and sorry being delayed. I might need time until 2.10 release to properly test it.

(in the meantime you don't need to rebase I can happily handle rebase once ready to check on it πŸ‘πŸΌ )

Copy link
Contributor

@septatrix septatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely looks like a nice improvement. I am still mostly looking forward to automatically inferring the type (see #938) but this is a good compromise for the time being.

What I do not like is that is seems to only support object literals (based on reading the code, please correct me if I am wrong). No real evaluation for the route metadata is possible. This can and will confuse a lot of people. My first though was that this could be used with zod to generate zome of the types automatically but this does not seem feasible.

src/scan.ts Outdated Show resolved Hide resolved
@pi0 pi0 marked this pull request as draft April 4, 2024 19:14
@pi0 pi0 marked this pull request as ready for review April 10, 2024 13:38
@pi0 pi0 changed the title feat: experimental openapi define schema in server route files feat: experimental defineRouteMeta Apr 10, 2024
@pi0
Copy link
Member

pi0 commented Apr 10, 2024

Thanks for your initiatives on this dear @eriksLapins. I have made some refactors in the way it works to experiment alternative approach for Nitro.

The meta is scanned using a rollup plugin now to allow HMR and uses a native parser of rollup + AST to Object transform and is only enabled with an experimental open API flag.

@pi0 pi0 merged commit da05b8d into nitrojs:main Apr 10, 2024
4 checks passed
@atinux
Copy link
Collaborator

atinux commented Apr 11, 2024

How can we share the validation (let's say I am using zod) between the route meta and the readValidatedBody for example?

@pi0
Copy link
Member

pi0 commented Apr 11, 2024

It is experimental and not for validation right now.

@rmarquet21
Copy link

@pi0 Any idea when this will be in production and not just on the "nightly" version?

@pi0
Copy link
Member

pi0 commented Apr 17, 2024

2.10 release no ETA. Also mind you, this API is likely to completely change into event handler object format nothing to rely on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants