DEV-269 Use isomorphic-ws instead of ws
#443
Conversation
dmytroyarmak
requested review from
OgulcanCelik,
EslaM-M,
Campalo,
sinahashemy and
caglaralkiss
dmytroyarmak
force-pushed
the
DEV-269-migrate-to-isomorphic-ws
branch
from
671b65f to
4c856ca
Compare
|
|
||
| describe("when executing in a browser environment", () => { | ||
| before(() => { | ||
| (global as any).window = { | ||
| WebSocket: {...ws, fake: true}, | ||
| }; | ||
| }); | ||
|
|
||
| after(() => { | ||
| (global as any).window = undefined; | ||
| }); | ||
|
|
||
| it("should use the native browser WebSocket implementation", () => { | ||
| createClientStub.resetHistory(); | ||
|
|
||
| client.graphQL.createSubscriptionClient(); | ||
|
|
||
| expect(createClientStub.callCount).to.equal(1); | ||
| expect(createClientStub.getCall(0).args[0].webSocketImpl.fake).to.equal(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🤔 why we are removing test? we can still verify if isomorphic-ws using native WebSocket with this
There was a problem hiding this comment.
This test case was specifically for typeof window === "undefined" condition that was in our code base, but now it's removed and it's now internal logic of isomorphic-ws. They actually handle it in more graceful way by having browser entry point in package.json (webpack uses it during bundling) that loads very tiny JS wrapper to normalize native websocket and for node.js it loads completely different file with ws implementation.
Anyway, I've extendede previous test case to verify that we pass WebSocket from isomorphic-ws to createClient: d5600cc
| "ws": "^8.4.2" | ||
| "ws": "^8.16.0" |
There was a problem hiding this comment.
❔ Why did you downgrade ws library?
Also could it be totally replaced by isomorphic-ws library?
There was a problem hiding this comment.
ws is actually upgraded from 4 to 16 😉 It's just unusual to read when version number has 2 digits
isomorphic-ws internally uses ws for NodeJS and they require to have ws package installed as "peer dependency".
There was a problem hiding this comment.
ws is actually upgraded from 4 to 16 😉 It's just unusual to read when version number has 2 digits
🙈
Use
isomorphic-wsinstead ofwsto fix problem withwebpackv5 complaining about native module usage byws(it's intender to be run only on Node.js)