-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(types): router option types #6282
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
Conversation
gurgunday
left a comment
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.
lgtm
|
Because typing is used in two files, maybe we can export a type from fastify.d.ts: export interface RouterOptions {
allowUnsafeRegex?: boolean,
buildPrettyMeta?: (route: { [k: string]: unknown, store: { [k: string]: unknown } }) => object,
caseSensitive?: boolean,
constraints?: {
[name: string]: ConstraintStrategy<FindMyWayVersion<RawServer>, unknown>,
},
defaultRoute?: (req: FastifyRequest, res: FastifyReply) => void,
ignoreDuplicateSlashes?: boolean,
ignoreTrailingSlash?: boolean,
maxParamLength?: number,
onBadUrl?: (path: string, req: FastifyRequest, res: FastifyReply) => void,
querystringParser?: (str: string) => { [key: string]: unknown },
useSemicolonDelimiter?: boolean,
} |
mcollina
left a comment
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.
lgtm
|
The typescript tests are failing |
|
Looks like typescript tests are failing because Should we update fastify to use |
|
@dancastillo can you fix the problem in a separate PR if you're free? 🙏 |
|
@gurgunday of course here is PR #6286 |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@mcollina @gurgunday the typescripts tests are now passing |
Uzlopak
left a comment
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.
LGTM
|
Is it possible to release a new version including this change? This should have been included in v5.5.0, as for now we get this deprecation message:
And we can't do nothing about it, as TypeScript will get mad. Thanks! |
Since Fastify v5.5.0, there is the following deprecation warning: > [FSTDPE022] FastifyWarning: The router options for maxParamLength property access is deprecated. Please use "options.routerOptions" instead for acce ssing router options. The router options will be removed in `fastify@6`. Related fastify/fastify#6282, fastify/fastify#5985 ## 🎯 Changes Update the Fastify adapter documentation to fix the deprecation warning. Should we put the 2 possible ways for both v5.4 and prior Fastify versions, and for v5.5.0 and above, or it's fine to only put the "latest" way (like how it is done currently in this PR)? <!-- Note: once you create a Pull request, we will automatically fix auto-fixable lint issues in your branch --> ## ✅ Checklist - [x] I have followed the steps listed in the [Contributing guide](https://github.com/trpc/trpc/blob/main/CONTRIBUTING.md). - [x] If necessary, I have added documentation related to the changes made. - [x] I have added or updated the tests related to the changes made. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Documentation - Updated Fastify adapter docs to show maxParamLength is now configured under routerOptions (default 5000). - Clarified that server initialization and plugin setup are unchanged. - Confirmed no changes to public APIs and guided users to configure maximum URL parameter lengths in the new location. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Checklist
types for #5985
npm run test && npm run benchmark --if-presentand the Code of conduct