JSteunou / webstomp-client

Stomp client over websocket for browsers
Apache License 2.0
299 stars 59 forks source link

webstomp.over is not a function anymore? #73

Closed pmeijer closed 6 years ago

pmeijer commented 6 years ago

Bumping from 1.2.0 and 1.2.2 I stared to get an exception saying webstomp.over is not a function.

    const WebSocket = require('ws'); // v5.2.1
    const webstomp = require('webstomp-client');

    const webSocket = new WebSocket('ws://127.0.0.1:15674/ws');
    // https://github.com/JSteunou/webstomp-client#overws-options
    const client = webstomp.over(webSocket, {debug: false}); // This line throws
Way2nnadi commented 6 years ago

Yeah I am getting the same errors. I thought patch bumps weren't supposed to add breaking changes.

JSteunou commented 6 years ago

@Way2nnadi I though too. Ho but maybe I did not do it on purpose...

JSteunou commented 6 years ago

@pmeijer could you inspect your webstomp object after the require('webstomp-client')? over method is still here, nothing change, I suspect the build to have brake something on the exported object.

cexbrayat commented 6 years ago

I'm also running into this issue. If that's any help, webstomp prototype looks like:

prototype: WebSocket
CLOSED:3
CLOSING:2
CONNECTING:0
OPEN:1
binaryType:(...)
bufferedAmount:(...)
close:ƒ close()
extensions:(...)
onclose:(...)
onerror:(...)
onmessage:(...)
onopen:(...)
protocol:(...)
readyState:(...)
send:ƒ send()
url:(...)
constructor:ƒ WebSocket()
Symbol(Symbol.toStringTag):"WebSocket"
JSteunou commented 6 years ago

That's weird, that is the prototype of WebSocket object. If you look at the src or the dist files, the export module is clearly the webstomp object.

What bundler are you using? webpack? Are you using js or ts?

cexbrayat commented 6 years ago

It's an Angular application, using TypeScript and Angular CLI (which uses Webpack underneath). Simply upgrading webstomp-client from 1.2.0 to 1.2.1 or 1.2.2 reproduces the issue.

JSteunou commented 6 years ago

The thing is I'm using it already and I do not have this issue. I cannot reproduce it, but I'm not in TS nor Angular environment. Could your try without the index.d.ts shipped in this project, maybe it is outdated?

cexbrayat commented 6 years ago

I don't think this is a TS issue, because this is a runtime error... Maybe it's an Angular CLI issue, I don't know if the OP is also using Angular CLI? But it's the only library that shows such an issue...

Anyway, I did my best to make a reproduction. See https://github.com/cexbrayat/webstomp-client-issue-73

This is a simple Angular CLI application (generated with ng new webstomp-issue) . I then added webstomp-client: npm i --save-exact webstomp-client@1.2.0. Then I updated the code to add a Webstomp connection:

    const url = 'ws://ponyracer.ninja-squad.com/ws';
    const websocket: WebSocket = new WebSocket(url);
    const stompClient: Client = Webstomp.over(websocket);
    stompClient.connect({ login: null, passcode: null }, () => {
        stompClient.subscribe('player/2179', message => {

      });
    });

This works. To check it out yourself, clone the repo, run npm i then npm start, open http://localhost:4200, there is no error in the console.

Now if I try to update to 1.2.2 (see https://github.com/cexbrayat/webstomp-client-issue-73/pull/1), then I see the error webstomp_client__WEBPACK_IMPORTED_MODULE_1__.over is not a function. You can try to switch to the branch reproduce-issue-73, run npm i and npm start again, and you'll see the error.

I hope this helps.

JSteunou commented 6 years ago

I can see the bug, but I still dont know why it happens :(

In standard build with webpack it does not behave the same. I'll try to fix by moving from webpack to rollup to build the dist files.

JSteunou commented 6 years ago

@cexbrayat I just published 1.2.3 hoping it solves the issue without breaking for others people. I saw different weird things in export / ts definition and build. Anyway, waiting for your feedback before closing this. Appreciate your help ;)

cexbrayat commented 6 years ago

@JSteunou The good news is that it looks like the issue is fixed (no error in the repoduction repo) 🎆

Bad news is that I get:

ERROR in node_modules/webstomp-client/index.d.ts(145,5): error TS7008: Member 'VERSIONS' implicitly has an 'any' type.
node_modules/webstomp-client/index.d.ts(146,5): error TS7008: Member 'client' implicitly has an 'any' type.
node_modules/webstomp-client/index.d.ts(147,5): error TS7008: Member 'over' implicitly has an 'any' type.

in another project. So I think you need to type more strictly what you introduced in https://github.com/JSteunou/webstomp-client/commit/35e50a91bbc8550a6006ffbfed21bbf605cd2b1c to make it perfect 🙂

cexbrayat commented 6 years ago

I think something like :

declare const webstomp: {
    Frame: Frame,
    VERSIONS: typeof VERSIONS,
    client: Client,
    over: Client,
}

would do the job (the type of VERSIONS can maybe be improved, but that should work)

JSteunou commented 6 years ago

Got this ERROR in src/app/app.component.ts(16,33): error TS2349: Cannot invoke an expression whose type lacks a call signature. Type 'Client' has no compatible call signatures.

When trying default import with import Webstomp from 'webstomp-client';

JSteunou commented 6 years ago

I used typeof all the way. I'm not a TS expert so if someone wants to make a better index.d.ts I'm taking it 1.2.4 is published

cexbrayat commented 6 years ago

@JSteunou 👍 I updated my app to 1.2.4 and I have no issues anymore. Thanks for your help!