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

@types/socketcluster update to v15 #41893

Closed
4 tasks done
ChisholmKyle opened this issue Jan 27, 2020 · 9 comments
Closed
4 tasks done

@types/socketcluster update to v15 #41893

ChisholmKyle opened this issue Jan 27, 2020 · 9 comments

Comments

@ChisholmKyle
Copy link
Contributor

ChisholmKyle commented Jan 27, 2020

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

  • I tried using the @types/socketcluster 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

Socketcluster recently updated to v15. Code has been restructured based on asyngular-client and asyngular-server and prefix SC is now AG

@DanielRose thank you for adding the SCxxx types but it would be great if you can update to v15 with AGxxxx.

@DanielRose
Copy link
Contributor

Ok, I'll take a look.

@DanielRose
Copy link
Contributor

socketcluster itself changed into a cli-tool with version 15, so there are no type definitions which make sense. However, I updated the other related type-definitions, ex. socketcluster-server and socketcluster-client: #42068

@ChisholmKyle
Copy link
Contributor Author

Thank you!!!

@DanielRose
Copy link
Contributor

PR was merged, so you can close this :)

@ChisholmKyle
Copy link
Contributor Author

ChisholmKyle commented Feb 6, 2020

Ok I started using it and looking good, except for the 'subscribeStateChange' listener data (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;
}

I suppose I should create a new issue? Is there an easy way to amend your commit. Is another PR required?

@DanielRose
Copy link
Contributor

Yup, my mistake. The PR is merged, so a new PR is needed. Do you want to do that?

@ChisholmKyle
Copy link
Contributor Author

Would you mind doing the PR? I think the merge gets fast tracked when an author of the package makes the PR and I've never done one for DefinitelyTyped before.

Also, one other tiny thing came up: for socketcluster-server, I think the option 'protocolVersion' is not required, see socketcluster-server/server.d.ts:199-201

@ChisholmKyle
Copy link
Contributor Author

I made a new issue #42192

@jessetrinity
Copy link
Contributor

The PR moves into the same fast lane if the author approves it too, and he seems pretty responsive :)

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

No branches or pull requests

3 participants