Skip to content
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

Socketcluster small fixes for channel and options in v15 #42192

Closed
ChisholmKyle opened this issue Feb 7, 2020 · 4 comments
Closed

Socketcluster small fixes for channel and options in v15 #42192

ChisholmKyle opened this issue Feb 7, 2020 · 4 comments

Comments

@ChisholmKyle
Copy link

@ChisholmKyle ChisholmKyle commented Feb 7, 2020

If you know how to fix the issue, make a pull request instead.

  • I tried using the @types/socketcluster-client package and had problems.
  • I tried using the @types/socketcluster-server package and had problems.
  • I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
  • I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
  • Mention the authors (see Definitions by: in index.d.ts) so they can respond.

If you do not mention the authors the issue will be ignored.

Description

In socketcluster-client, the 'subscribeStateChange' listener data has a problem (see docs: https://socketcluster.io/docs/api-ag-client-socket/#events). In socketcluster-client/lib/clientsocket.d.ts:414 the SubscribeStateChangeData type should be

interface SubscribeStateChangeData extends SubscribeData {
    oldChannelState: AGChannel.ChannelState;
    newChannelState: AGChannel.ChannelState;
}

In socketcluster-server, the option 'protocolVersion' should be optional, see socketcluster-server/server.d.ts:199-201. It should have the ? as follows:

// Can be 1 or 2. Version 1 is for maximum backwards
// compatibility with SocketCluster clients.
protocolVersion?: 1 | 2;
@ChisholmKyle ChisholmKyle mentioned this issue Feb 7, 2020
4 of 4 tasks complete
@ChisholmKyle

This comment has been minimized.

Copy link
Author

@ChisholmKyle ChisholmKyle commented Feb 8, 2020

Looks like they just bumped the version number again to 16! I don't think many API changes happened for types though

@DanielRose

This comment has been minimized.

Copy link
Contributor

@DanielRose DanielRose commented Feb 10, 2020

I'll take a look!

@DanielRose

This comment has been minimized.

Copy link
Contributor

@DanielRose DanielRose commented Feb 14, 2020

PR is #42366

@ChisholmKyle

This comment has been minimized.

Copy link
Author

@ChisholmKyle ChisholmKyle commented Feb 24, 2020

Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.