Promise support

[RyanZim]

This uses https://github.com/RyanZim/universalify, a library I wrote for this purpose.

This is a semver-major change.

Needs docs (Done!), and we should add more tests in the future.

🎉 Promises are here! 🎉

[RyanZim]

Need to fix my hacky tests since they fail on Node 4 & 5.

[RyanZim]

Added basic README docs; should eventually migrate docs/ in a seperate PR.

[manidlou]

Probably not that important, but for the purpose of consistency in using ', wouldn't it be better console.log("success!") replaced with console.log('success!') since we use ' pretty much everywhere else? It actually has been there since the beginning (I guess). I just saw it now and thought that's be nice that we fix it here 😄

[RyanZim]

Yeah, we should fix that; thanks for the catch. I also removed an old semicolon in these docs. 😉

Someone ought to make a tool to run eslint on code blocks in markdown.

[manidlou]

Same here: console.log("success!") -> console.log('success!')?

[manidlou]

Also here, console.log("success!") -> console.log('success!')?

[manidlou]

Other than that minor thing, LGTM. Nice work @RyanZim 👏

[RyanZim]

@manidlou Fixed.

---

Should also add: I'd appreciate review/improvements to https://github.com/RyanZim/universalify as well.

[coveralls]

Coverage increased (+0.5%) to 86.729% when pulling daaa4f0 on promises into 2a4ec71 on develop.

[jprichardson]

Nice work @RyanZim!

1. Will you verify that the test cases for universalify include any additional test cases from https://github.com/sindresorhus/pify/blob/master/test.js and https://github.com/paulmillr/micro-promisify/blob/master/test.js - i.e. I'm just trying to ensure unverisalify doesn't have any edge case bugs that these battle-hardened libraries have already solved.

2. Maybe I missed it, but will you add some tests demonstrating that a method imported from fs returns a promise?

Thanks!

[RyanZim]

@jprichardson

1. All the cases covered by pify, are already tested; except pify works on entire modules, which is something universalify will never do.

   micro-promisify has a few cases I should add. Thanks for the suggestions!

2. Already done in lib/fs/__tests__/mz.test.js

[manidlou]

Someone ought to make a tool to run eslint on code blocks in markdown.

I believe that's a great idea!

Actually, that is specifically mentioned in standardjs docs:

Can I check code inside of Markdown or HTML files?

As it is mentioned there, they introduce two modules for that:

• standard-markdown
• eslint-plugin-markdown

I tried standard-markdown (on our old README.md), and this was the result:

Linting file 1 of 1

   README.md
         74:15:    Strings must use singlequote.
         75:3:     Extra semicolon.
         79:15:    Strings must use singlequote.

Then, we can add a new scripts something like lintmd to package.json to automate this process.

So, @jprichardson, @RyanZim what do you think?

Edit

Maybe that'd be better I opened a new issue for that, IDK! 😕

[RyanZim]

@manidlou Yeah, new issue or PR for that.

[RyanZim]

@jprichardson micro-promisify sets the name and length properties on the returned function. I'm fine with setting name, but setting length isn't too straightforward, since we are potentially adding or removing the final callback parameter. I'd just skip setting length IMO. Are you OK with that?

[RyanZim]

Updated docs/ to reflect this change.

[louisgv]

LGTM 👍

[coveralls]

Coverage increased (+0.5%) to 86.729% when pulling e09efe1 on promises into 2a4ec71 on develop.

[RyanZim]

@jprichardson Updated PR to use universalify v0.1.0, which sets name for the returned function.

Anything else before the merge?

[jdalton]

Hurray! I was using pify to wrap fs-extra methods 😸

[rijnhard]

Hi

This is pretty cool because now i dont need fs-extra-promise but its also kind of annoying that its opinionated. For instance I use Q for pretty much everything and now I have to wrap any fs returns like this Q(fs.close()).
Ideally using a library like any-promise makes it easy for the user to pick his preferred implementation.

[RyanZim]

@rijnhard I'm of the opinion that fs-extra should be as light as possible. Adding any-promise increases weight. I'm firmly opposed to this ATM.

[rijnhard]

@RyanZim theres another alternative I've seen, where theres some variable or method that accepts an A+ promise implementation, then they use that for all the promise creation.
like

import Q from 'q';
import fs from 'fs-extra';
fs.setPromise(Q.Promise);

and in the code you use the standard es6 style promise as you would normally.

[RyanZim]

Issue filed: #425