-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
fix(types): module exports for TypeScript for ES6 compliance. #918
Conversation
|
||
export declare function createBroker (options?: AedesOptions): Aedes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you removed this?
aedes.d.ts
Outdated
declare module 'aedes' { | ||
export = Aedes | ||
} | ||
export { default } from './types/instance'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is correct
It is correct, and I've confirmed its use. To explain it more verbosely:
Agree on |
Could you add a typescript test that covers this to prevent any possible breaking change? |
Add additional validation testing for createBroker.
Added an additional check for createBroker, but there's not much more I can do here. Do let me know if you're going to accept this or not - I'd like to get this PR closed out either way. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix lint issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint issues, fix them: npm run lint:fix
Linting done. |
Pull Request Test Coverage Report for Build 7117819105
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Addresses #878 (comment) and corrects the export declaration in TypeScript.