Skip to content

fix: export UndiciHeaders type and set dispatch headers to UndiciHeaders#3849

Merged
mcollina merged 1 commit intonodejs:mainfrom
dancastillo:fix-and-export-dispatcher-headers
Mar 27, 2025
Merged

fix: export UndiciHeaders type and set dispatch headers to UndiciHeaders#3849
mcollina merged 1 commit intonodejs:mainfrom
dancastillo:fix-and-export-dispatcher-headers

Conversation

@dancastillo
Copy link
Contributor

This relates to...

#3840

Changes

  • Export UndiciHeaders
  • Set dispatcher headers type to UndiciHeaders
  • Add tests to test/types/dispatcher.test.d.ts

Status

* **reset** `boolean` (optional) - Default: `false` - If `false`, the request will attempt to create a long-living connection by sending the `connection: keep-alive` header,otherwise will attempt to close it immediately after response by sending `connection: close` within the request and closing the socket afterwards.
* **body** `string | Buffer | Uint8Array | stream.Readable | Iterable | AsyncIterable | null` (optional) - Default: `null`
* **headers** `UndiciHeaders | string[]` (optional) - Default: `null`.
* **headers** `UndiciHeaders` (optional) - Default: `null`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should have undici types here? @mcollina

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can rename to OutgoingHttpHeaders also.
I saw this in the docs which matched the type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any thoughts on this @mcollina ?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should define UndiciHeaders somewhere if we do.

* **reset** `boolean` (optional) - Default: `false` - If `false`, the request will attempt to create a long-living connection by sending the `connection: keep-alive` header,otherwise will attempt to close it immediately after response by sending `connection: close` within the request and closing the socket afterwards.
* **body** `string | Buffer | Uint8Array | stream.Readable | Iterable | AsyncIterable | null` (optional) - Default: `null`
* **headers** `UndiciHeaders | string[]` (optional) - Default: `null`.
* **headers** `UndiciHeaders` (optional) - Default: `null`.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should define UndiciHeaders somewhere if we do.


export default Dispatcher

export type UndiciHeaders = IncomingHttpHeaders | string[] | Iterable<[string, string | string[] | undefined]> | null
Copy link
Member

Choose a reason for hiding this comment

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

This definition should be in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dancastillo dancastillo force-pushed the fix-and-export-dispatcher-headers branch from 87c3c6d to d41d11d Compare December 3, 2024 14:13
@dancastillo dancastillo requested a review from ronag December 21, 2024 02:51
@dancastillo
Copy link
Contributor Author

@ronag @mcollina could you ptal

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 242ec9b into nodejs:main Mar 27, 2025
@github-actions github-actions bot mentioned this pull request Apr 2, 2025
@github-actions github-actions bot mentioned this pull request May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants