-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove app.use() #3502
Comments
Please do this. I wouldn't be upset if we also deprecate |
I think they are useful plugins, we should keep having them but reduce their visibility. |
Can I take this? How should I go about this like removing |
First thing we should probably do is issue a new v3 minor that adds a depreciation warning. |
For this should the docs be updated here? |
That would be part of it. The main thing would be adding a new deprecation error and triggering it when the method is invoked. |
it's already throwing an error, we can just remove it. |
Should I make this change on a new branch while keeping |
Yes |
Hey, so to fix this I made a new branch while keeping |
You need to remove the corresponding test for fastify/test/middleware.test.js Lines 10 to 20 in 0ae362b
fastify/test/middleware.test.js Lines 50 to 58 in 0ae362b
|
Removed these tests but I still have errors from multiple test files |
Looking at your errors, I would recommend to As for the rest, you'd need to look why they are failining. |
What about this one? |
Update the |
Fixed all of the above > fastify@4.0.0-dev test:typescript
> tsc test/types/import.ts && tsd
types/logger.d.ts:18:33 - error TS2694: Namespace 'pino' has no exported member 'SerializerFn'.
18 export type SerializerFn = pino.SerializerFn
~~~~~~~~~~~~
types/logger.d.ts:23:38 - error TS2694: Namespace 'pino' has no exported member 'BaseLogger'.
23 export type FastifyBaseLogger = pino.BaseLogger & {
~~~~~~~~~~
types/logger.d.ts:27:34 - error TS2694: Namespace 'pino' has no exported member 'PrettyOptions'.
27 export type PrettyOptions = pino.PrettyOptions & { suppressFlushSyncWarning?: boolean }
~~~~~~~~~~~~~
Found 3 errors.
|
It is expected to be fail as pinojs/pino#1195 introduce the type breaking in certain area. |
Got it! |
Issues like fastify/fastify-express#61 are why I think we should deprecate the In short: I think we have given enough lifecycles to facilitate migrating. It's time to move forward. |
Done in #3506 |
Prerequisites
馃殌 Feature Proposal
I think we should be removing
app.use()
in thenext
release, i.e.fastify/fastify.js
Lines 327 to 330 in a174829
Motivation
We should not be encouraging people to use middlewares. Both
middie
andfastify-express
will still be there and maintained.Example
No response
The text was updated successfully, but these errors were encountered: