-
-
Notifications
You must be signed in to change notification settings - Fork 935
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
Improve types #946
Improve types #946
Conversation
Did you see #876 (comment) ? |
I actually just read that. Should we just augment the global scope and add the two classes there? I don't see another way out ... |
Yeah, we can do that for now. |
Pushed a commit to finish up the work on |
One more that could use a better type: Line 17 in d968e49
And maybe also: got/test/helpers/with-server.ts Line 30 in d968e49
|
Regarding the doc strings. I think that should be a separate PR after this one. I'd like to get this one merged asap so we can do the final Got v10 release. |
Also I've fixed some types here: https://github.com/sindresorhus/got/pull/833/files#diff-05c76183bbee7e002aa6711e6baca2ef so you may want to take a look and fix it here also. |
LGTM but it'd be nice if you merged my fixes into this PR, I don't expect my PR to get merged before yours. |
The first one - I think the interface is actually 100% equal to the ServerResponse class from the 'http' library? I checked The second one - I haven't found a way to work around that. It seems to be a TypeScript problem as stated here? I can try removing the generics for |
@szmarczak Do you want me to pull in all changes related to types from there? I am particularly not very confident on pulling in changes in this file, since there is some change in normalization logic too? |
I'll send a PR to your PR ( |
@pmmmwh Sent you a PR ;) |
If you've got any questions, feel free to ask. This is great work! :D |
Fix types & increase coverage
…ow-up # Conflicts: # source/utils/types.ts
Checked and merged 😺 |
Awesome! Thanks for all your work on this, @pmmmwh! We could not have done this without you. 🙌 |
This is a follow-up to #928 . Hopefully this is the last batch of (type breaking?) changes 🤞 .
Changes
ts-ignore
comments should have been removed by Make TypeScript types conforms to strict mode #928 or this PRts-ignore
comments should have a description stating what happened thereChecklist