@yotamN Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

• node — on npm, on unpkg
  • owner-approval: @Renegade334

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

{
  "type": "info",
  "now": "-",
  "pr_number": 72617,
  "author": "yotamN",
  "headCommitOid": "b81fcd8253feee7acdf00bfa37332a03816d244f",
  "mergeBaseOid": "1ba22ccfabc63d96ea75922df24043a9cbccf147",
  "lastPushDate": "2025-04-27T11:20:24.000Z",
  "lastActivityDate": "2025-05-06T05:48:59.000Z",
  "maintainerBlessed": "Waiting for Author to Merge (Blessed)",
  "mergeOfferDate": "2025-05-06T01:06:59.000Z",
  "mergeRequestDate": "2025-05-06T05:48:59.000Z",
  "mergeRequestUser": "yotamN",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/http.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/http.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/http.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/test/http.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Microsoft",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "galkin",
        "parambirs",
        "eps1lon",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina",
        "Semigradsky",
        "Renegade334"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "Renegade334",
      "date": "2025-04-29T17:10:46.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 2833398262,
  "ciResult": "pass"
}

🔔 @microsoft @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @nodejs @LinusU @wafuwafu13 @mcollina @Semigradsky @Renegade334 — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@yotamN One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

Can you please also apply these changes to the v18 and v20 branches?

Otherwise, LGTM.

Sure, do I need to just open a separate PR for them?

Sure, do I need to just open a separate PR for them?

DT's branches are tree-based – you just need to apply the same changes to /types/node/{v18,v20}/....

@Renegade334 Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

DT's branches are tree-based – you just need to apply the same changes to /types/node/{v18,v20}/....

Okay sure, so I added it

Ready to merge

Previously we could access headers by options.headers.Authorization, which is no longer allowed due to this change:

Property 'Authorization' does not exist on type 'OutgoingHttpHeaders | readonly string[]'.
Property 'Authorization' does not exist on type 'readonly string[]'.ts(2339)

IMO this is a bug of the type. Please fix.

Also I don't really see the advantage of this very change.

Previously we could access headers by options.headers.Authorization

In what context?

Something like this (optional auth - but this is only an example):

            const options: RequestOptions = { headers: { 'Content-Length': 0 }, ...}

            if (userName) {
                options.headers.Authorization = ...
            }

I'm not sure what to do with it.
From your code snippet I understand why it breaks backward compatibility but my update just fixed the type to represent the actual implementation.
Yes, you can't assume that the headers is an object and the fact that you could until now was a bug.

@types/node options interfaces aren't necessarily designed for post-declaration mutability; they're just intended to be an accurate representation of what a particular function or method accepts, and this change ultimately reflects the behaviour of the API.

There are still any number of ways of providing the functionality of the example case above:

const options: RequestOptions = {
  headers: {
    'Content-Length': 0,
    ...userName && { Authorization: 'xxx' },
  },
  ...
}

request(options)

etc.

@types/node options interfaces aren't necessarily designed for post-declaration mutability

I see. Sure, there are ways to circumvent the problem. It's a pity, however, if you need to refactor code as a workaround, rendering it less readable.

you can't assume that the headers is an object

this change ultimately reflects the behaviour of the API

According to https://nodejs.org/docs/latest/api/http.html#httprequestoptions-callback, "headers" is explicitely defined as an object, nothing else. So I'm not sure where the string array comes from and whether the type should reflect "undocumented" behaviour.

According to https://nodejs.org/docs/latest/api/http.html#httprequestoptions-callback, "headers" is explicitely defined as an object, nothing else. So I'm not sure where the string array comes from and whether the type should reflect "undocumented" behaviour.

@yotamN @Renegade334 - I'm with @jeffrson here, this is a regression. I noticed it when suddenly hundreds of lines like reqOpts.headers.cookie caused error

Property 'cookie' does not exist on type 'OutgoingHttpHeaders | readonly string[]'.

The docs show that headers should be an object. This change should be reverted.

Changing the type won't change the fact that http.request can receive an array of strings.
The docs are not up to date but you can see that the PR have been merged, meaning you will see it there in the next release.