undici icon indicating copy to clipboard operation
undici copied to clipboard

Add generic type for opaque object for stream() and pipeline()

Open jfhr opened this issue 7 months ago • 2 comments

This would solve...

Currently, when using stream() or pipeline(), opaque is typed as unknown in the callback, e.g:

const bufs = [];

undici.stream("https://example.com", {
    method: 'GET',
    opaque: { bufs }
}, ({ statusCode, headers, opaque }) => {
    // TypeError here: Property 'bufs' does not exist on type 'unknown'.ts(2339)
    const { bufs } = opaque;
    return new Writable({
        write(chunk, encoding, callback) {
            bufs.push(chunk)
            callback()
        }
    })
});

Checking this code with TypeScript will generate an error: Property 'bufs' does not exist on type 'unknown'.ts(2339)

If opaque was defined as generic, TypeScript could infer the type from the second argument to stream() and provide the correct type inside the callback.

The implementation should look like...

Add a generic type parameter (<TOpaque = null>) to the stream() and pipeline() type declarations as well as to RequestOptions and a few other interfaces. For example:

declare function stream<TOpaque = null>(
  url: string | URL | UrlObject,
  options: { dispatcher?: Dispatcher } & Omit<Dispatcher.RequestOptions<TOpaque>, 'origin' | 'path'>,
  factory: Dispatcher.StreamFactory<TOpaque>
): Promise<Dispatcher.StreamData>;

I have also considered...

The alternative is to keep the current type declarations and require users to always cast opaque to a certain type before using it.

Additional context

@Ethan-Arrowood , who originally added the type declarations, said in his pull request:

For simplicity, I'm keeping this as unknown. Using a generic to pass through a type is no safer than type casting an unknown value.

Maybe I'm missing something here, but it seems to me that the generic approach is safer because it enforces that the opaque object that was passed in the request options has the same type as the one received in the callback. For example, the following would currently only cause a runtime error, but with the generic approach would give a type error:

const bufs  = [];

undici.stream("https://example.com", {
    method: 'GET',
    // forgot to pass opaque
}, ({ statusCode, headers, opaque }) => {
    // error: opaque is null here
    const { bufs } = opaque;
    return new Writable({
        write(chunk, encoding, callback) {
            bufs.push(chunk)
            callback()
        }
    })
});

jfhr avatar Jun 28 '24 11:06 jfhr