You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, when using stream() or pipeline(), opaque is typed as unknown in the callback, e.g:
constbufs=[];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;returnnewWritable({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:
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:
constbufs=[];undici.stream("https://example.com",{method: 'GET',// forgot to pass opaque},({ statusCode, headers, opaque })=>{// error: opaque is null hereconst{ bufs }=opaque;returnnewWritable({write(chunk,encoding,callback){bufs.push(chunk)callback()}})});
The text was updated successfully, but these errors were encountered:
This adds a `TOpaque` generic type parameter to the type definitions
for request(), connect(), stream(), and pipeline(). The type parameter
defaults to null, which is the default value of the opaque property.
If an opaque value is passed in the options, its type can usually be
inferred automatically, such that no explicit type declaration is
necessary. This commit also adds tsd tests to make sure the type
definitions work as expected.
Previously, the type of `opaque` was `unknown`, which means it needed
to be either type-checked or casted to another type before anything
could be done with it. Such code should not be broken by this commit,
although some type checks or assertions might become redundant. Code
that disabled type checks (e.g. by casting to `any` or using
`@ts-ignore` should be unaffected. Code that does not use typescript
at all is also unaffected.
This closesnodejs#3378
This would solve...
Currently, when using
stream()
orpipeline()
,opaque
is typed asunknown
in the callback, e.g: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 tostream()
and provide the correct type inside the callback.The implementation should look like...
Add a generic type parameter (
<TOpaque = null>
) to thestream()
andpipeline()
type declarations as well as toRequestOptions
and a few other interfaces. For example: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:
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:The text was updated successfully, but these errors were encountered: