-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(hosts): accept url as string #1189
Conversation
In v3 the API was `hosts: string[]`. It's easy to miss out on this in the migration guide, and it's also simpler to write just a string in the general case fixes #1188
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the protocol
, what do you think?
@@ -1,6 +1,14 @@ | |||
import { CallEnum, HostOptions, StatelessHost } from '.'; | |||
|
|||
export function createStatelessHost(options: HostOptions): StatelessHost { | |||
if (typeof options === 'string') { | |||
return { | |||
protocol: 'https', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to force the protocol? I think I would rather keep not set the protocol
key and let the regular business logic deal with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the only place where protocol is set. If you set { url: xxx }
the protocol will also be https. If you want a custom protocol you can use the object form as normal. Don't you think that most cases would be using https?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think https
is a reasonable default here.
Also FMPOV the DX is better if createStatelessHost({url: xxx})
and createStatelessHost(xxx)
have the same behavior
f23fbc9
to
a341934
Compare
In v3 the API was
hosts: string[]
. It's easy to miss out on this in the migration guide, and it's also simpler to write just a string in the general casefixes #1188