Bluebird: Should ES6 global variable "Promise" be changed in examples in the docs?

[CWSpear]

With ES6 coming and Promise being a reserved keyword in ES6, should we encourage users to use a different variable (other than Promise) when using Bluebird? Bluebird seems like a good replacement candidate.

I'd be willing to do a PR if others think this is a good idea. I would have thought it'd been brought up before, but couldn't find a ticket. I'm still wondering if there is some other reason it hasn't been changed, so I wanted to ask before taking the time to do a PR (since it's not quite as simple as just finding and replacing).

[bergus]

Promise is not a reserved keyword. It's the name of a built-in object, the identifier for a global variable. Shadowing it is not a problem, it actually is what we aim to do here.

[CWSpear]

Er, sorry, you're right!

Definitely _not_ a reserved keyword. I would still think it's better not to replace it, as it would cause confusion (and potentially cause issues, where someone assumes they have Bluebird in some context, but don't). Most of this is solved with modules, but there will definitely be cases of people not using modules and still using Bluebird is a post-native Promise world.

The cases are indeed edge, but they're there and completely solved by just using Bluebird as a variable name as a "best practice".

[benjamingr]

So basically if you have bluebird which supersedes the ES2015 API (minus sub classing) you probably wouldn't ever want to use native promises anyway.

[CWSpear]

@benjamingr, I agree, which is I think it should should be different, so people are certain to know what they're using.

In the browser, code-wise (as so far, I've just been discussing the docs and usage), it would be much more important to not replace the global Promise and affect other code that is expecting Promise. Typically, since Bluebird encompasses everything Promise does, it should not cause any issues, but again, it seems like we shouldn't take the risk.

[hoonto]

@CWSpear I agree, I've been using Promise like the docs, and, as with Q, I will switch to using Bluebird based on those points but also it quickly/clearly calls out what kind of promise API is available for readers.

[benjamingr]

Typically, since Bluebird encompasses everything Promise does, it should not cause any issues, but again, it seems like we shouldn't take the risk.

There is no risk, it is perfectly safe and guaranteed to work.

If you import bluebird or use a module loader it's not an issue anyway. I don't want to break naming for users for something like this.

[CWSpear]

If a user does not use a module loader, there is absolutely an issue (specifically in the browser). What if some other promise library comes by and thinks like you: they will add a global for the variable Promise since they are hoping to replace native promises and add methods. And let's say you have different methods or worse: the same method, but a different signature?

And then let's say you're working on some WordPress site that already included that library (abd not uses a module loader) and you try and use Bluebird, but you didn't realize that other library was added later and all your promises aren't quite working and you can't figure out why...

And then someone is looking at some code and trying to make an edit and they don't know if that Promise is native or Bluebird or this other new library (or maybe even something else!).

[benjamingr]

I don't play the 100% theoretical use case game. Show me a single WordPress site that had this problem - that would go a long way in convincing me of sacrificing backwards compatibility for users.

[CWSpear]

(Note that this scenario happens all the time and frustrates users when using libs that set global variables for $ and _. But this library is purposefully using a global variable that it is trying to emulate, one that other similar libs would similarly try to emulate.)

[benjamingr]

I think I understand where my confusion about this is form - Bluebird
features a .noConflict() mode which you can and are welcome to use if you
need access to the previous promise object _and_ can't use a module loader.

There is no harm done in exporting it _by default_ since it's designed to
act as a drop in replacement and supersedes the ES2015 API.

On Thu, Jul 30, 2015 at 6:31 PM, Cameron Spear [email protected]
wrote:

(Note that this scenario happens all the time and frustrates users when
using libs that set global variables for $ and _. But this library is
purposefully using a global variable that it is trying to emulate, one that
other similar libs would similarly try to emulate.)

—
Reply to this email directly or view it on GitHub
https://github.com/petkaantonov/bluebird/issues/715#issuecomment-126371303
.

[CWSpear]

I'm in my phone so it is a little harder to search for stuff like this, but there are tons of issues around $ if you search $.noConflict: https://github.com/search?q=%24.noConflict&type=issues

I am not saying remove it, since it does depend on backward compatibility. But perhaps you could deprecate it (see if you can detect if someone uses it and print a message) and update the docs to use Bluebird as a variable.

I know Bluebird has noConflict, but by looking at those search results above, you can see it is still a great source of confusion and frustration that could be better avoided.

[spion]

I understand your concern, but I think that not using a module loader in 2015 on anything thats complex enough to load two different promise libraries is the problem.

Also, there is more than one downside to doing this.

The first and obvious one is breaking the code of everyone currently using bluebird without a module loader. On the plus side, it will be just this once, and there is an easy workaround (window.Promise = Bluebird)

The second and slightly less obvious one is that bluebird's API naming has relied very much on the export being named Promise. For example, unlike with Q (Q.Promise) the default export is the constructor - making a new promise after the change would be var p = new Bluebird((resolve, reject) => ...). Another example would be some static methods whose names will also be affected as they rely on the word Promise being on the left for clarity - most notably, Promise.fromNode(callback => fs.readFile(path, callback)) reads as "promise from node callback"

Perhaps BBPromise might work as a compromise (no pun intended :smile:)?

[bergus]

I think using the name Promise is a sensible default, and there's absolutely no need to change the docs (which this issue is all about). It should be obvious to everyone that you can choose a different name in your code if you are facing name conflicts with anything.

[rsp]

@bergus It's worth noting that JSHint wouldn't agree with you, it considers all of those examples in the Bluebird docs incorrect - see issue jshint/jshint#1747

[rsp]

@benjamingr Since you closed this issue I'd like to know your opinion on var Promise = require("bluebird"); Do you recommend redefining Promise as in the examples in the Bluebird docs? JSHint considers it an error: jshint/jshint#1747 and @domenic calls it "future hostile". I'd like to know what's the current best practice. Thanks.

[spion]

There is a difference between window.Promise = require('bluebird') and var Promise = require('bluebird')

For the second version, you can use whatever name you like, really.

[CWSpear]

Man, if I had a dime for everytime I got a message like this (this one is copy and pasted from earlier today):

@cwspear I just spent like 30 mins debugging cus I didn't import bluebird, and it defaulted to the native Promise... noob mistake.

These kinds of issues will always exist, but can be minimized with the official docs championed const Bluebird = require('bluebird');, etc.

[CWSpear]

Here we are in mid-2019, and people are still having issues because they used Promise instead of Bluebird. This issue is arguably more relevant today than in the past.

And we're not asking for a _breaking_ change, we're just saying it'd be better if the docs recommended that you didn't use Promise.

[spion]

I don't think that Bluebird is the issue in that article, but, I've come to think that you are indeed right. Difficult to trace errors due to a forgotten bluebird import can be a problem. Common scenario:

1. Why is this fn().map erroring out on the .map method ?
2. Oh because fn() returns a non-bluebird promise. How is that possible?
3. Oops, because the module where fn() is defined forgot to import bluebird when doing new Promise. Why did that happen?
4. Most modules that use promises don't really need to import bluebird, they can simply call the promise methods - until there is a need to construct a new promise or call Promise.all. At that point in time its easy to forget to import it.

With node 12 supporting async stack traces, I think the best thing we can do is to somehow provide a migration path towards native promises. A static implementation of most Bluebird methods would probably be the best idea. In that light I'm not entirely sure if the best thing to do about this particular issue is to change the imports, or to keep them, recommending to slowly move to static versions of methods and then at the end switch the static method library and remove the import.

Of course that doesn't apply to users that use cancellation, synchronous inspection or monitoring - so I think changing the default name in the docs is indeed still reasonable.

Sorry I didn't quite see the issue originally.

[CWSpear]

Why do you not think Bluebird is the issue in that article? The author of the post had an update (emphasis mine):

UPDATE: like many people, we've been using Bluebird as a promise library. Turns out this is a Bluebird issue, not a Node vs Chrome issue. Be advised!

It seems like you understand what I'm trying to convey, tho... that it gets confusing knowing if you have a Promise Promise or a Bluebird Promise, and I feel that if this repo recommended using Bluebird instead of Promise, the vast majority of common and recurring issues would be eliminated/much more quickly found out.

Bluebird still can have a place, as mentioned, but it's not a Promise polyfil. A Bluebird instance is more powerful than a Promise instance, and we should be more clear about which we're having in our code, and the argument is that this repo can help promote that practice.

[spion]

@CWSpear because you should not use instanceof to check anything in JavaScript due to cross-realm issues and multiple-library-copies issues. Not arrays, not promises, not anything. Using the method I posted there in the comments of that article avoids the problem and makes it irrelevant.

But thats a distraction, and the reason I missed the point of the original issue were such pedantic distractions :) Even with everything done right the error proneness remains.