fix(nodejs): remove undefined from OutgoingHttpHeaders

[boopathi]

In Node, if a request header is undefined, then making a http.request throws (synchronously) with a TypeError - ERR_HTTP_INVALID_HEADER_VALUE. This is something that should be caught at compile time.

For example,

const http = require("http");

try {
  http.request({
    host: "localhost",
    headers: {
      "Something": undefined
    }
  }, (res) => {
    console.log("Success");
  });
} catch (e) {
  console.log("Sending request with undefined header throws this error - ");
  throw e;
}

I've attempted a fix where the OutgoingHttpHeaders used as the input path of a request does not allow undefined as one of the possible values and the OutgoingHttpHeaders at the output path - such as getHeaders() - undefined is one of the possible values.

I will change the types for older node versions after receiving an initial feedback

---

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: <>
• Increase the version number in the header if appropriate.
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

[typescript-bot]

@boopathi Thank you for submitting this PR!

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @matthieusieben @mohsen1 @n-e @octo-sniffle @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @jeremiergz @samuela @kuehlein @j-oliveras @bhongy - 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.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

[typescript-bot]

@boopathi The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[typescript-bot]

👋 Hi there! I’ve run some quick performance metrics against master and your PR. This is still an experiment, so don’t panic if I say something crazy! I’m still learning how to interpret these metrics.

Let’s review the numbers, shall we?

Comparison details 📊

	master	#36265	diff
Batch compilation
Type count	20162	20208	+0.2%
Assignability cache size	38134	38136	0.0%
Subtype cache size	2460	2598	+5.6%
Identity cache size	19	19	0.0%
Language service
Samples taken	4394	2595	-40.9%
Identifiers in tests	10617	10679	+0.6%
getCompletionsAtPosition
Mean duration (ms)	830.4	833.0	+0.3%
Median duration (ms)	827.6	821.7	-0.7%
Mean CV		4.8%
Worst duration (ms)	1259.2	1234.3	-2.0%
Worst identifier	encoding	listS
getQuickInfoAtPosition
Mean duration (ms)	796.0	810.6	+1.8%
Median duration (ms)	776.5	795.9	+2.5%
Mean CV		5.0%
Worst duration (ms)	1154.5	1338.1	+15.9%
Worst identifier	inspector	inspector
System information
Node version	v10.15.3	v10.15.3
CPU count	2	2
CPU speed	2.397 GHz	2.294 GHz
CPU model	Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz	Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz
CPU Architecture	x64	x64
Memory	6.8 GiB	6.8 GiB
Platform	linux	linux
Release	4.15.0-1041-azure	4.15.0-1045-azure

First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt.

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

---

If you have any questions or comments about me, you can ping @andrewbranch. Have a nice day!

[Flarna]

see #20695 (comment) why | undefined was added.

There are discussions in typescript issue tracker (sorry, have no issue number) regarding should | undefined part of an index signature or not as it's inconsistent currently. Manually adding | undefined is a workaround with some drawbacks.

Most likely this change will break some users applications as they augment OutgoingHttpHeaders as described in above comment.

[typescript-bot]

@boopathi I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

[typescript-bot]

@boopathi To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!