feat(core, common): add webdav http methods support #13164
Conversation
Pull Request Test Coverage Report for Build 520beb78-4e7f-4e59-bea1-f1dd49941824Details
💛 - Coveralls |
|
Its sounds good to me 🙂 |
|
There is already a PR here: #12925 |
My bad, i did not see it. But this PR (#12925) is just a copy/paste from the old PR #9465 which contains obvious errors. All new methods in packages/core/adapters/http-adapter.ts return the propfind function. |
| @@ -47,6 +47,20 @@ export interface HttpServer< | |||
| put(path: string, handler: RequestHandler<TRequest, TResponse>): any; | |||
| patch(handler: RequestHandler<TRequest, TResponse>): any; | |||
| patch(path: string, handler: RequestHandler<TRequest, TResponse>): any; | |||
| propfind(handler: RequestHandler<TRequest, TResponse>): any; | |||
There was a problem hiding this comment.
This is a breaking change. We shouldn't add methods to an interface on a patch release.
There was a problem hiding this comment.
I don't really see how this is a breaking change, adding decorators only allows access to methods already implemented in Express and Fastify.
Many of us have been waiting for this feature for a very long time ...
There was a problem hiding this comment.
Adding non-optional methods to a public interface that should be implemented for adapters means that anyone who is maintaining their own adapter, say for Koa, or hyper-express, etc, now must make these changes or possibly have their packages broken by a transient upgrade.
This is a braking change, and while you may have been waiting for it for a while, that doesn't mean it's any more urgent to get out, as we want to avoid making several small breaking changes and would rather lump them together over a major upgrade.
There was a problem hiding this comment.
It is. Updating a public interface results in breaking changes in another library implementing this interface. We can alternatively update this method to be optional; in this case, we won't have to wait for a major release.
see #13000
There was a problem hiding this comment.
@jmcdo29 I agree, I prefer this explanation guys :)
There was a problem hiding this comment.
@dammy001 I can try to adapt starting from what was done by @doronguttman here: 385d21b but I don't know if that will be enough.
There was a problem hiding this comment.
@johaven i think that should be enough
# Conflicts: # packages/core/helpers/router-method-factory.ts
Closes #9459
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #9459
What is the new behavior?
add new decorators for webdav methods, like @propfind
Does this PR introduce a breaking change?
Other information