Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Socket's connect API now takes a secureTransport enum instead of bool. #570

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Socket's connect API now takes a secureTransport enum instead of bool. #570

merged 1 commit into from
Apr 27, 2023

Conversation

dom96
Copy link
Contributor

@dom96 dom96 commented Apr 26, 2023 •
edited
Loading

Changes the connect's options API to take a secureTransport enum instead of a bool:

connect("address:port", { secureTransport: "off" })
connect("address:port", { secureTransport: "allow" })
connect("address:port", { secureTransport: "use" })

Context: #562 (comment)

@dom96 dom96 mentioned this pull request Apr 26, 2023
Closed
@kentonv
Copy link
Member

kentonv commented Apr 26, 2023

Bikeshedding a bit but I don't feel like it's obvious what secureTransport: "allow" means. Whereas allowStarttls: true would have been pretty obvious. But I don't have a strong opinion. Maybe @irvinebroque has thoughts.

@dom96 dom96 force-pushed the dominik/secure-transport-enum branch from fb7f586 to da24a2c Compare April 26, 2023 17:41
@jasnell
Copy link
Member

jasnell commented Apr 26, 2023

Perhaps, secureTransport: "starttls" ?

Warfields
Warfields approved these changes Apr 26, 2023
View reviewed changes
@dom96
Copy link
Contributor Author

dom96 commented Apr 27, 2023

Okay, how about "off", "starttls", "on"?

@dom96 dom96 force-pushed the dominik/secure-transport-enum branch from da24a2c to 7cb74fa Compare April 27, 2023 15:06
@dom96 dom96 force-pushed the dominik/secure-transport-enum branch from 7cb74fa to cb73939 Compare April 27, 2023 15:09
@dom96 dom96 merged commit e15482f into main Apr 27, 2023
@dom96 dom96 deleted the dominik/secure-transport-enum branch April 27, 2023 16:24
@petebacondarwin
Copy link
Contributor

Should there be an update to the TS typings for this too?

@dom96
Copy link
Contributor Author

dom96 commented May 5, 2023

@petebacondarwin you mean the internal ones? They don't really need to be super detailed but updated anyway: #612

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@Warfields Warfields Warfields approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dom96 @kentonv @jasnell @petebacondarwin @Warfields