Skip to content
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

Add generic type for opaque object #3385

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Conversation

jfhr
Copy link
Contributor

@jfhr jfhr commented Jul 2, 2024

This relates to...

This closes #3378

Changes

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.

Features

N/A

Bug Fixes

N/A

Breaking Changes and Deprecations

No breaking change, unless I've missed something in the explanation below:

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.

Status

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 closes nodejs#3378
Copy link
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 10caf6d into nodejs:main Jul 9, 2024
30 checks passed
@github-actions github-actions bot mentioned this pull request Dec 3, 2024
This was referenced Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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