cacheable-request: Possibly incorrect typing

[TBoshoven]

trafficstars

Describe the bug The CacheResponse type is defined as ServerResponse | typeof ResponseLike. I believe the intention is IncomingMessage | typeof ResponseLike.

The ServerResponse type is used for outgoing responses, whereas the IncomingMessage type is used for incoming responses. The response callback for the Node request function emits an IncomingRequest, the ResponseLike class mimics IncomingRequest, and the mimicResponse function's response type is bounded by IncomingMessage.

How To Reproduce (best to provide workable code or tests!) A code attempt to reproduce the issue using instanceof failed because of the response cloning, so this ended up a bit convoluted:

import CacheableRequest from 'cacheable-request';
import * as https from 'node:https';

const cacheableRequest = new CacheableRequest(https.request).request();

const cacheReq = cacheableRequest('https://example.com/', resp => {
  // headersSent is defined on OutgoingMessage, the parent of ServerResponse
  console.log('Is it a ServerResponse?', 'headersSent' in resp);
  // body is defined on ResponseLike
  console.log('Is it a ResponseLike?', 'body' in resp);
  // httpVersion is defined on IncomingMessage (and no other Node http types)
  console.log('Is it maybe an IncomingMessage?', 'httpVersion' in resp);
});
cacheReq.on('request', req => req.end());

This results in:

Is it a ServerResponse? false
Is it a ResponseLike? false
Is it maybe an IncomingMessage? true

If the type was correct, I believe one of the first two lines should have said true.

[jaredwray]

@TBoshoven thanks for fixing this. Do you want to do a pull request to update docs and apply a fix?

[TBoshoven]

Per #756 I don't have a lot of context around how typing was done in this project in the first place, so I'm not 100% confident that what I wrote in the description is correct.

Happy to make a PR to update this, but it will likely have an impact on library users. Even fixing the type can cause things to fail building unexpectedly so the change would probably have to be folded into a major release.

[jaredwray]

@TBoshoven - going to hold off on this as we are working on a new module to replace this called @cacheable/http that will be released later this year.