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

Align OnConnectCallback with specs expecting Connack packet #1333

Merged
merged 2 commits into from Oct 4, 2021

Conversation

ccarcaci
Copy link
Contributor

$ npm run typescript-compile-test

> mqtt@4.2.8 typescript-compile-test
> tsc -p test/typescript/tsconfig.json

types/lib/client-options.d.ts:3:23 - error TS7016: Could not find a declaration file for module 'ws'. '/home/ccarcaci/git/MQTT.js/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm install @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

3 import WebSocket from 'ws'
                        ~~~~


Found 1 error.
  • OnConnectCallback receives back a Connack packet as stated in the MQTT specs.
  • Not sure which kind of release version to associate with this PR. Major is suggested because of interface change that is not backward compatible for the ones are using typescript.

…compilation error resolving ws types

OnConnectCallback receives back a Connack packet as stated in the MQTT specs
@ccarcaci ccarcaci changed the title Aligne OnConnectCallback with specs expecting Connack packet Align OnConnectCallback with specs expecting Connack packet Sep 30, 2021
@YoDaMa
Copy link
Contributor

YoDaMa commented Oct 4, 2021

@ccarcaci could you add @types/ws to the dev dependencies of the package and then remove the changes to tsconfig

@YoDaMa
Copy link
Contributor

YoDaMa commented Oct 4, 2021

looks good pending reverting tsconfig adjustments

Restore noImplicitAny typescript rule
@ccarcaci
Copy link
Contributor Author

ccarcaci commented Oct 4, 2021

looks good pending reverting tsconfig adjustments

Done.

@YoDaMa
Copy link
Contributor

YoDaMa commented Oct 4, 2021

lgtm

@YoDaMa
Copy link
Contributor

YoDaMa commented Oct 4, 2021

thank you @ccarcaci for your contribution :)

@YoDaMa YoDaMa merged commit e3e15c3 into mqttjs:master Oct 4, 2021
@ccarcaci
Copy link
Contributor Author

ccarcaci commented Oct 5, 2021

thank you @ccarcaci for your contribution :)

My pleasure :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants