Add typescript definition #110
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ilikejames
force-pushed
the
add-typescript-definition
branch
2 times, most recently
from
2e63978
to
99dc6b0
Compare
ilikejames
force-pushed
the
add-typescript-definition
branch
17 times, most recently
from
d31072f
to
c89a2d4
Compare
ilikejames
force-pushed
the
add-typescript-definition
branch
from
c89a2d4
to
070916b
Compare
ilikejames
commented
Mar 31, 2023
Comment on lines
+6
to
+32
| 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' |
ilikejames
commented
Mar 31, 2023
| url?: string; | ||
| method?: string; | ||
| originalUrl?: string; | ||
| params?: Record<string, string>; |
There was a problem hiding this comment.
Assumption: params are always strings.
ilikejames
commented
Mar 31, 2023
| ], | ||
| "typesVersions": { | ||
| ">=4.0": {"*": ["index.d.ts"]} |
There was a problem hiding this comment.
Limited to downstream typescript@>4.0.0
suddenlyGiovanni
added a commit
to suddenlyGiovanni/router
that referenced
this pull request
Jul 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.