-
-
Notifications
You must be signed in to change notification settings - Fork 117
Add typescript definition #110
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
base: master
Are you sure you want to change the base?
Conversation
2e63978 to
99dc6b0
Compare
d31072f to
c89a2d4
Compare
c89a2d4 to
070916b
Compare
| type HttpMethods = | ||
| 'get' | | ||
| 'post' | | ||
| 'put' | | ||
| 'head' | | ||
| 'delete' | | ||
| 'options' | | ||
| 'trace' | | ||
| 'copy' | | ||
| 'lock' | | ||
| 'mkcol' | | ||
| 'move' | | ||
| 'purge' | | ||
| 'propfind' | | ||
| 'proppatch' | | ||
| 'unlock' | | ||
| 'report' | | ||
| 'mkactivity' | | ||
| 'checkout' | | ||
| 'merge' | | ||
| 'm-search' | | ||
| 'notify' | | ||
| 'subscribe' | | ||
| 'unsubscribe' | | ||
| 'patch' | | ||
| 'search' | | ||
| 'connect' |
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.
Take from methods lib
| url?: string; | ||
| method?: string; | ||
| originalUrl?: string; | ||
| params?: Record<string, string>; |
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.
Assumption: params are always strings.
| "index.d.ts" | ||
| ], | ||
| "typesVersions": { | ||
| ">=4.0": {"*": ["index.d.ts"]} |
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.
Limited to downstream typescript@>4.0.0
|
Is there any progress here? AFAIK This is the last non-dev dependency to get Express types moving. |
|
cc @ilikejames |
A few years ago I had a quick attempt at adding types for this library. https://github.com/pillarjs/router/pull/76.
The http methods is copied from the
methodslibrary. I had hoped to use its types directly, but that proved tricky with building older versions of node, so instead it's a hard coded list copied from the library.The types level tests are exhaustive, but let me know if there is anything I have missed.
There is a couple of assumptions made:
OutgoingMessagetype.I've limited the published types to [email protected] and greater due to some of the syntax being specific to this more recent version.
I had wanted to include
@types/nodeas apeerDependencybut it appears that was not removable vianpm rm X --save-peerand would result in a build error in early versions ofnpmEdit: Made a further change. It looks like in the tests there is a lot of
To facilitate that, I've updated
RoutedRequest(on the second commit).This might be better done by supplying a generic, but that could be a further later addition.
Edit: Sorry for all the pushes/builds. Should have closed the PR, and worked on the fork only.