Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

[TS] Change websocket library in Node #2989

Merged
merged 4 commits into from
Sep 21, 2018
Merged

[TS] Change websocket library in Node #2989

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
551b80f
[TS] Change websocket library in Node
BrennanConroy Sep 20, 2018
25abbdc
isArrayBuffer?
BrennanConroy Sep 20, 2018
7292294
format
BrennanConroy Sep 20, 2018
0e1f4ac
comment
BrennanConroy Sep 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 51 additions & 67 deletions clients/ts/FunctionalTests/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion clients/ts/FunctionalTests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"karma-summary-reporter": "^1.5.0",
"ts-node": "^4.1.0",
"typescript": "^3.0.1",
"websocket": " ^1.0.26"
"ws": " ^6.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I need to get approval for this one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are swapping in a pure-JavaScript websocket library instead of the previous WebSocket library we were using which had a native component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

},
"scripts": {
"clean": "node ../common/node_modules/rimraf/bin.js ./wwwroot/dist ./obj/js",
Expand Down
7 changes: 3 additions & 4 deletions clients/ts/FunctionalTests/ts/WebSocketTests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ describe("WebSockets", () => {
return;
}
} else {
const websocketModule = require("websocket");
const hasWebsocket = websocketModule && websocketModule.w3cwebsocket;
if (hasWebsocket) {
webSocket = new websocketModule.w3cwebsocket(ECHOENDPOINT_URL.replace(/^http/, "ws"));
const websocketModule = require("ws");
if (websocketModule) {
webSocket = new websocketModule(ECHOENDPOINT_URL.replace(/^http/, "ws"));
} else {
// No WebSockets implementations in current environment, skip test
done();
Expand Down
2 changes: 1 addition & 1 deletion clients/ts/signalr-protocol-msgpack/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion clients/ts/signalr-protocol-msgpack/src/MessagePackHubProtocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as msgpack5 from "msgpack5";
import { CompletionMessage, HubMessage, IHubProtocol, ILogger, InvocationMessage, LogLevel, MessageHeaders, MessageType, NullLogger, StreamInvocationMessage, StreamItemMessage, TransferFormat } from "@aspnet/signalr";

import { BinaryMessageFormat } from "./BinaryMessageFormat";
import { isArrayBuffer } from "./Utils";

// TypeDoc's @inheritDoc and @link don't work across modules :(

Expand All @@ -31,7 +32,7 @@ export class MessagePackHubProtocol implements IHubProtocol {
*/
public parseMessages(input: ArrayBuffer | Buffer, logger: ILogger): HubMessage[] {
// The interface does allow "string" to be passed in, but this implementation does not. So let's throw a useful error.
if (!(input instanceof ArrayBuffer) && !(input instanceof Buffer)) {
if (!(input instanceof Buffer) && !(isArrayBuffer(input))) {
throw new Error("Invalid input for MessagePack hub protocol. Expected an ArrayBuffer or Buffer.");
}

Expand Down
11 changes: 11 additions & 0 deletions clients/ts/signalr-protocol-msgpack/src/Utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

// Copied from signalr/Utils.ts
/** @private */
export function isArrayBuffer(val: any): val is ArrayBuffer {
return val && typeof ArrayBuffer !== "undefined" &&
(val instanceof ArrayBuffer ||
// Sometimes we get an ArrayBuffer that comes from a polyfill
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wrote this comment when I was randomly speculating :). Just say something like "Sometimes we get an ArrayBuffer that doesn't satisfy instanceof 🤷‍♂️ "

(val.constructor && val.constructor.name === "ArrayBuffer"));
}
Loading