-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Require Node.js 10 #20
Conversation
@sindresorhus Could you take a look at this one? 🙏 It's currently the 3rd biggest dependent of |
index.js
Outdated
if (!input) { | ||
return Promise.resolve(0); | ||
} | ||
|
||
return pify(zlib.gzip)(input, getOptions(options)).then(data => data.length).catch(_ => 0); | ||
const data = await promisify(zlib.gzip)(input, getOptions(options)); |
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.
It's generally better to do the promisification at the top-level if it doesn't require any local state, so it doesn't have to be done for each function invocation.
|
||
module.exports = (input, options) => { | ||
module.exports = async (input, options) => { | ||
if (!input) { | ||
return Promise.resolve(0); | ||
} | ||
|
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.
The Promise.resolve
is moot now.
package.json
Outdated
"ava": "^2.4.0", | ||
"p-event": "^4.2.0", | ||
"tsd": "^0.13.1", | ||
"typescript": "^4.0.5", |
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.
This is not needed.
This nicely gets rid of the dependency of
pify
.