Skip to content

Use secureOptions in request tests, not securityOptions - #34680

Merged
uniqueiniquity merged 1 commit into
masterfrom
fix-request-tests
Apr 12, 2019
Merged

Use secureOptions in request tests, not securityOptions#34680
uniqueiniquity merged 1 commit into
masterfrom
fix-request-tests

Conversation

@weswigham

@weswigham weswigham commented Apr 12, 2019

Copy link
Copy Markdown
Contributor

Found while working on microsoft/TypeScript#30853. "securityOptions" is fictional - there's no documentation reference of it anywhere.

@weswigham
weswigham requested a review from Bartvds as a code owner April 12, 2019 00:57
@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels Apr 12, 2019
@typescript-bot

typescript-bot commented Apr 12, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@weswigham Thank you for submitting this PR!

🔔 @soywiz @bonnici @Bartvds @joeskeen @ccurrens @lookfirst @mastermatt @josecolella @murbanowicz @cglantschnig @AyaMorisawa @gustavohenke - 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.

@mastermatt

mastermatt commented Apr 12, 2019

Copy link
Copy Markdown
Contributor

This set correctly in @types/node. Do we know why this didn't error before?

https.AgentOptions -> tls.ConnectionOptions -> tls.SecureContextOptions

Node docs

@weswigham

weswigham commented Apr 12, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Because TS didn't issue an excess property error when the target type was a union or intersection before (so it was just an 'unused' property that didn't affect assignment); see linked TS issue.

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Apr 12, 2019
@typescript-bot

typescript-bot commented Apr 12, 2019

Copy link
Copy Markdown
Contributor

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@weswigham weswigham mentioned this pull request Apr 12, 2019
Merged
@uniqueiniquity
uniqueiniquity merged commit 269cdff into master Apr 12, 2019
@weswigham
weswigham deleted the fix-request-tests branch April 12, 2019 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bartvds Bartvds Awaiting requested review from Bartvds Bartvds is a code owner
1 more reviewer
@mastermatt mastermatt mastermatt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Owner Approved A listed owner of this package signed off on the pull request. Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@weswigham @typescript-bot @mastermatt @uniqueiniquity