Node-fs-extra: Alternate Promise libraries

[RyanZim]

As per https://github.com/jprichardson/node-fs-extra/pull/403 & https://github.com/overlookmotel/fs-extra-promise/pull/27, people are asking to use custom (non-native) promise implementations.

I don't know if this is a use-case we should support or not.

[rijnhard]

I went into the universalify code and using the method option that was discussed, the code changes involve adding a module scoped variable and a setter function which even with nodes small module philosophy is still highly negligible.

However when I looked at how you used it in fs-extra I was expecting a final transform step (from callbacks to universal) at the top level export, not at each function definition. Which means that it wont so easy to carry the custom promise implementation into fs-extra. Which means a fair bit of code changes.

Honestly I'd say that it's still worth it to provide it, to make the unknown portion of the 1.1 million odd daily downloaders of Q or bluebird promises that could use fs-extra lives a lot easier.

[nrser]

I'd like to voice support for this feature. I'm delighted to see universal promise / callback functionality added out of the box (thanks!), but I've got bluebird integrated across my libraries and it would be great to have that functionality available in filesystem calls without an additional layer of wrapping.

[bsberry]

+1 to this, particularly because having bluebird .finally at the end of a promise chain when dealing with filesystem operations is very helpful. Especially on windows where filesystem operations can fail somewhat mysteriously, having the confidence of a finally that will always run is very useful.

[RyanZim]

@jprichardson @manidlou Thoughts?

[jprichardson]

If we can do a simple setPromise() without too much trouble, then it may be worth it. But what I don't understand is why people don't just do global.Promise = require('bluebird') or whatever their favorite Promise implementation is. Wouldn't that solve the problem?

[RyanZim]

But what I don't understand is why people don't just do global.Promise = require('bluebird') or whatever their favorite Promise implementation is. Wouldn't that solve the problem?

Yeah, I don't get it either. Someone care to explain?

[manidlou]

But what I don't understand is why people don't just do global.Promise = require('bluebird') or whatever their favorite Promise implementation is. Wouldn't that solve the problem?

I am wondering too. Is there any specific reasons that users don't want to do this?!

[rijnhard]

@jprichardson @RyanZim @manidlou
because you can use the setPromise method once at the entry point to your application, but with global.Promise = require('bluebird') you have to do it every time before you include fs-extra.

At least i had issues with it, open for correction.

[RyanZim]

@rijnhard That isn't necessary; just set global.Promise = require('bluebird') at the top of the entry point to your app. (Before requiring fs-extra or anything else).

[rijnhard]

@RyanZim Welllllllllll then theres no reason. Assuming using es6 imports it's still possible then its just ignorance on my part.

[RyanZim]

@rijnhard With ES6 imports, you'd do:

import bluebird from 'blubird'
global.Promise = bluebird

[rijnhard]

I find it ironic that i didn't know this, even though it's pretty obvious.

I see no reason to provide a setPromise anymore, maybe some documentation demonstrating how to set the promise implementation, even though its not something that should be documented here it will surely help others.

I have some refactoring to do.

[thecjharries]

I recently stopped overriding the global Promise for a few reasons. It made my immediate work easier, but I slowly came to the realization that it was going to make maintenance a nightmare. Different coders like different Promise libraries, none of which agree on an API beyond the spec. If I pass my code off to someone that uses Q, for example, they can certainly handle it, but they'll either have to constantly remember the Promise API could change per project or refactor any failed tests to use their API. When I explicitly state I'm using Bluebird or Q, anyone looking at any of my code knows exactly which library I'm using.

More importantly, messing with global can have unintended consequences. I slapped together a quick illustration. If this build is failing, it means multiple developers overriding global.Promise (i.e. you did it, a package it, your coworker did, etc.) is a bad idea:

(Edit: I should have gone to sleep last night and built that this morning. It's now failing for the right reasons.)

[rmvermeulen]

Maybe any-promise is a solution here? It defaults to the native Promise, so nothing changes for users who don do anything. Imo it's great to have a single place to set the promise implementation for a bunch of libraries, without touching the global state.

[RyanZim]

In regards to https://github.com/RyanZim/universalify/pull/5:

Frankly, I don't get the point. How is overriding the promise implementation used by universalify and anything else that depends on it more "evil" than overriding global.Promise?

If you're a library, yes, overriding global.Promise is evil; but the same applies to overriding universalify's Promise. I'd also contend that a library returning non-standard promises is evil as well.

If you're an app, what's the harm of overriding global.Promise?

[thecjharries]

You're not overriding universalify.Promise everywhere, you're overriding it in the specific context where it should be overridden. It has to be defined per file, which means you know what you're doing. If you're overriding it in a library, you're either limiting its scope and not exposing it, you're warning dependents you've overridden it (i.e. you're letting them know your library returns Bluebird not Promise), or no one uses your library when it breaks. No matter what, you're changing the behavior properly within its own scope.

[RyanZim]

You're not overriding universalify.Promise everywhere

@thecjharries Erm, yes, you are. CommonJS modules are singletons. Anything that imports universalify afterwards will get your custom promise implementation.

[thecjharries]

@RyanZim Huh, I apologize. I'll modify that pull request when I get off this evening to use a factory. With a factory, my argument still applies. Again, sorry. I should have remembered that.

[thecjharries]

I've been thinking about this all afternoon, and I don't see any good way to do it without a major version bump, which is a slightly ridiculous request.

Bluebird offers promisifyAll, which is what I've been using. I can't find anything similar in Q's API, and I'm too lazy to hunt down other libraries. It would be really nice to be able to call fse.readFile instead of fse.readFileBluebird, but, again, it doesn't seem possible without a major API change. Thanks for all the feedback.

[develar]

Bluebird offers you long stack traces. In my lib I use special instance of bluebird (it is not singleton, you can have any version of bluebird with any config). That's why I use my own wrapper fs-extra-p and Promise support in the fs-extra is not useful for me (and even more — should be not used to ensure that stack traces will be clear).

[RyanZim]

Going to close this out as I've yet to hear a good argument for not just overriding global.Promise in your app.

[johannes-scharlach]

Let me give it a try:

If you're a library, yes, overriding global.Promise is evil;

That is a believe I share. The case I have is a private npm module, which I'd consider as a library in this sense, so I don't want to overwrite global.Promise. But within that module I'd like to use bluebird features (.tap and .finally are particularly useful in places where I'm using fs-extra). Probably around two thirds of our code are in such private modules, so it's the regular case where we'd like to use bluebird.

In order to use bluebird with fs-extra, we currently have to either wrap the fs-extra call in a promise chain like Promise.resolve().then(() => fse.readFile(filename)).finally or I have to trust that _all applications_ in which this package is used overwrite global.Promise. In addition, I have to overwrite it in all my test files.

For a while we did the global.Promise overwrites, but now we're starting to remove them again, since that can easily be forgotten. This lead to code that worked in some scripts and didn't work in others. It's particularly annoying if your tests are passing but your regular code is failing, because you didn't set global.Promise.

[johannes-scharlach]

I've understood my requirements a little better now: I (as a library) definitely don't want to change the behaviour of fs-extra for everyone, so I need to make sure that if I want to use bluebird, such a promise setting affects only me.

The way I'm now handling the bluebirdification of fs-extra is during the require phase:

const fse = require('ensure-bluebird')(require('fs-extra'))

The ensure-bluebird package is something I just wrote to fix this very problem – I hope others in this thread may also find it useful.