force ws:// protocol when instantiating a new WebSocket
#172
Conversation
yes, I have updated the code |
|
| @@ -1,7 +1,7 @@ | |||
| 'use strict' | |||
| const From = require('fastify-reply-from') | |||
| const WebSocket = require('ws') | |||
|
|
|||
| const { convertUrlToWebSocket } = require('./utils') | |||
There was a problem hiding this comment.
nit: keep a blank line between requires/imports and other statements
| @@ -0,0 +1,8 @@ | |||
| 'use strict' | |||
test/ws-prefix-rewrite-core.js
Outdated
| async function proxyServer (t, backendURL, backendPath, proxyOptions, wrapperOptions) { | ||
| async function proxyServer(t, backendURL, backendPath, proxyOptions, wrapperOptions) { |
There was a problem hiding this comment.
can we avoid these formatting changes? they're inconsistent with the rest of the codebase, e.g. here. Either we change it everywhere or we don't change it. I would go with the latter.
There was a problem hiding this comment.
My linter was messed up, now it should be fixed to the previous version (with space)
| const t = require('tap') | ||
| const { convertUrlToWebSocket } = require('../utils') | ||
|
|
||
| t.test('convertUrlToWebSocket', function (t) { |
There was a problem hiding this comment.
tap has a built-in way to create array of tests, check it out. also, before and after are not two very meaningful names for inputs and outputs of the tests. more idiomatic names are e.g. input/expected
|
If we're happy with this let's merge it into the base branch which is the dependabot bump PR. |
* Bump ws from 7.5.3 to 8.0.0 Bumps [ws](https://github.com/websockets/ws) from 7.5.3 to 8.0.0. - [Release notes](https://github.com/websockets/ws/releases) - [Commits](websockets/ws@7.5.3...8.0.0) --- updated-dependencies: - dependency-name: ws dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * force `ws://` protocol when instantiating a new `WebSocket` (#172) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leonardo Rossi <leonardo.rossi@gmail.com>
ws@8.0.0introduces a breaking change that throws aSyntaxErrorwhen callingnew WebSocket()with an URL withhttp://protocol.This PR fixes such issue, forcing when needed the
ws://protocol instead.Checklist
npm run testandnpm run benchmarkand the Code of conduct