-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: allow string value for port
#3354
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3354 +/- ##
==========================================
+ Coverage 95.65% 95.67% +0.02%
==========================================
Files 33 33
Lines 1288 1295 +7
Branches 373 377 +4
==========================================
+ Hits 1232 1239 +7
Misses 52 52
Partials 4 4
Continue to review full report at Codecov.
|
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.
Can we add a simple e2e so it doesn't fail during compilation?
Added 👍🏻 |
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.
We also can have port: string
and webSocketServer: { type: 'ws', options: { port: string } }
, let's add these tests too, maybe will be great to move it in normalize and convert to Number
to avoid working with different types:
if (typeof this.options.port === 'string' && this.options.port !== 'auto') {
this.options.port = Number(this.options.port)
}
// Same for `websocketUrl.port` and `webSocketServer.options.port`
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.
Feel free to merge when CI will be green
For Bugs and Features; did you add new tests?
Motivation / Use-Case
Allow string for
client.webSocketURL.port
Breaking Changes
None
Additional Info
No