-
Notifications
You must be signed in to change notification settings - Fork 71
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
clean up assert exports #1729
clean up assert exports #1729
Conversation
packages/assert/index.js
Outdated
string: assertString, | ||
typeof: assertTypeof, | ||
} = globalAssert; | ||
const { bare, details, error, Fail, note, quote, makeAssert } = globalAssert; |
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 thought part of the consensus was to have the assert
exported by this package contain only the assertions, i.e., the methods not separately exported by name.
const { bare, details, error, Fail, note, quote, makeAssert } = globalAssert; | |
const { bare, details, error, Fail, note, quote, makeAssert, ...assertions } = globalAssert; | |
Object.freeze(assertions); | |
export { assertions as assert }; |
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.
Thanks, I forgot to include that change.
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.
And the exported assert
needs to be callable as well.
const { bare, details: redacted, error, Fail: throwRedacted, note, quote, makeAssert, ...assertions } = globalAssert;
const assert = (...args) => globalAssert(...args);
Object.assign(assert, assertions);
export { assert, redacted, throwRedacted, quote, bare, note, makeAssert };
3c02d8b
to
dab66f9
Compare
dab66f9
to
d8681b7
Compare
d8681b7
to
a4182db
Compare
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, thanks!
refs: #1717
Description
Fewer exports per #1717 (comment)
Clearer names per #1717 (comment)
Security Considerations
Scaling Considerations
Documentation Considerations
These are breaking changes to master, but not to what's been published.
Testing Considerations
Upgrade Considerations