-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Allows custom route.ts exports with the "_" prefix #55157
base: canary
Are you sure you want to change the base?
Conversation
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.
Orca Security Scan Summary
Status | Check | Issues by priority | |
---|---|---|---|
Passed | Secrets | 0 1 0 0 | View in Orca |
@ijjk I can't edit labels myself. This PR is not about documentation (though I added some documentation to reflect the code change). It is about changing Next.js Also the discussion (#55140) hasn't been picked up by anybody, do you know if I could mention someone who could be interested in having a look at my proposal ? |
0273785
to
71227a4
Compare
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.
Orca Security Scan Summary
Status | Check | Issues by priority | |
---|---|---|---|
Passed | Secrets | 0 0 0 0 | View in Orca |
Are there any updates on this? I'm trying to colocate OpenAPI docs with the route implementation and this would be really handy. |
cd39c20
to
fd536d0
Compare
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
@jens-ox Nothing new unfortunately, it's a bit sad to let small PRs like this one fade away. It has been a year. I rebased the PR to have a clean commit that can be merged into the current canary branch, let's see if it can make it to Next.js 15 |
Wouldn't you want these
Is it possible to elaborate on this (versus having like a separate
Is the issue because the thought is you can't export multiple methods in a Route Handler? |
You could build a custom wrapper in That wrapper could expose:
There is currently no way to do that from const apiRoute = customCRUD({ model: UserModel })
// or maybe
const apiRoute = customCRUD({
create: {
input: z.object({
title: z.string(),
}),
handler: ({ title }) => return { title }
},
})
// This works as intended
export { GET, POST } = apiRoute
// This is not allowed, even though `apiRoute` is what contains the actual info, inside `route.ts`
export { _createInputType, _openAPISchema, /* etc */ } = apiRoute
|
e7e6d01
to
0c4853a
Compare
0c4853a
to
f17b419
Compare
What?
App Router only allows HTTP methods and Route Segment Config in
route.ts
files.This PR lets us export our own custom keys, without altering the supported keys behavior
allow all exports_
, like_customConfig
,_test
, ...Why?
Some cases require to export some info: data, configs, types for the client, etc...
Next.js currently forces us to create separate files, that often implement everything in exactly the same way, and then export the HTTP methods in the
route.ts
like so:This can lead to more code duplication and boilerpate
How?
By modifying type definition here (webpack/plugins/next-types-plugin/index.ts#L57), we:
allow all exports"_"
, like_customConfig
,_test
, ...Allowing everything would give us more control, but it will prevent TS from catching typos like
export const Get = ...
(instead ofGET
). I believe that's the actual intent behind the current limitation, and I think_
prefixes are a good compromiseFixes #55140