Repository navigation
Add typescript definition - #110
Closed
ilikejames wants to merge 2 commits into
Closed
ilikejames wants to merge 2 commits into
ilikejames wants to merge 2 commits into
Conversation
ilikejames
force-pushed
the
add-typescript-definition
branch
2 times, most recently
from
March 28, 2023 16:02
2e63978 to
99dc6b0
Compare
ilikejames
marked this pull request as draft
March 28, 2023 16:10
ilikejames
force-pushed
the
add-typescript-definition
branch
17 times, most recently
from
March 29, 2023 21:30
d31072f to
c89a2d4
Compare
ilikejames
marked this pull request as ready for review
March 29, 2023 21:40
ilikejames
force-pushed
the
add-typescript-definition
branch
from
March 29, 2023 21:43
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>; |
Author
There was a problem hiding this comment.
Assumption: params are always strings.
ilikejames
commented
Mar 31, 2023
| "index.d.ts" | ||
| ], | ||
| "typesVersions": { | ||
| ">=4.0": {"*": ["index.d.ts"]} |
Author
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
|
Is there any progress here? AFAIK This is the last non-dev dependency to get Express types moving. |
|
cc @ilikejames |
Member
|
close in favor of #197 |
This file contains hidden or 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
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 typescript@4.0.0 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.