Improve typings relating to requestAsEventEmitter #760
Conversation
You should not need to do changes like these. We have the |
That makes sense; I'll revert them. Though it's odd that Added: Ah, I guess this is the expected behavior when specifying input files to |
This allows easily declaring a parameter that will be passed into the constructor. Ref sindresorhus/got#760 (comment)
This allows easily declaring a parameter that will be passed into the constructor. Ref sindresorhus/got#760 (comment)
# Conflicts: # source/request-as-event-emitter.ts
* [cacheable-request] Include type alias for cache adapter This allows easily declaring a parameter that will be passed into the constructor. Ref sindresorhus/got#760 (comment) * Fix missing semicolon * Move StorageAdapter into namespace
source/request-as-event-emitter.ts
Outdated
| if (options.useElectronNet && (process.versions as any).electron) { | ||
| // @ts-ignore | ||
| const r = ({x: require})['yx'.slice(1)]; // Trick webpack | ||
| const electron = r('electron'); | ||
| fn = electron.net || electron.remote.net; | ||
| requestFn = electron.net || electron.remote.net; |
There was a problem hiding this comment.
Are there tests for this code? I realize this was not accessing electron.net.request as it should.
There was a problem hiding this comment.
No, too much overhead to include Electron testing to be worth it. We do manual test after doing changes to that code.
…yped#34155) * [cacheable-request] Include type alias for cache adapter This allows easily declaring a parameter that will be passed into the constructor. Ref sindresorhus/got#760 (comment) * Fix missing semicolon * Move StorageAdapter into namespace
|
@paulmelnikow Is this good to merge or do you intend to do more changes? |
|
Good to merge! |
|
Thank you! 🙏 |
|
Thanks for all the comments! |
This improves several of the types in
requestAsEventEmitter, including its own return type and its dependencies. It also clears several TypeScript warnings.There are several changes which convert
to
This is to clear warnings which look like this:
My workflow has primarily to run
./node_modules/.bin/tsc --noEmit source/request-as-event-emitter.tsand then the tests. I'm not sure if there is a better way to evaluate the changes to the types.This is my first PR on got. I'm not an expert in TypeScript and there are definitely some good challenges here! Looking forward to feedback.
Checklist
Ref: #758