redefinition of Promise (W079)

[helmus]

I just switched from 2.4.4 to 2.5.1
But now all my requires are messed up because:

var Promise = require('bluebird')
Will warn about:
Redefinition of Promise (W079)

Is there anyway I can fix this ? I'm using bluebird all over the place.

[valueof]

Hmm, that's because Promise is a built-in global now. This is a good point that we need to fix somehow. Marking as P1.

[simonexmachina]

Also, I can't see how I can switch off Promise as a built-in global. I've tried setting browser: false but it's still considered a global.

[gabegorelick]

+1 I'm using browser: false and node: true. You'd think that would work, unless Node has plans to add Promise when they upgrade V8.

[helmus]

@gabegorelick Promises are already in node unstable so I'm pretty sure they will be in node V0.12, if they ever actually release it...
I think we probably want a more explicit flag then node or browser to allow this redefinition to allow an adaptation period to native Promises.

[carldanley]

bump on this. receiving the same errors.

[rwaldron]

Would you write this?

var Array = require("my-array-implementation");

...or pave over any other built-in object?

[carldanley]

Excellent point. Would you suggest rolling with some alternative then, like BluebirdPromise? I'm open to it, just liked the readability and I def. don't want to pave over anything.

[rwaldron]

Sure, or BPromise or whatever, something that won't conflict with other code that might expect Promise to be what it is. Fact: the community wanted Promise baked into the language and that's what happened. A side effect is that the word "Promise" isn't really free for all anymore.

[helmus]

@rwaldron I actually agree with you. I will just rename my references.

[simonexmachina]

Good call @rwaldron, I agree.

[rwaldron]

Great! This is a very progressive resolution :D

[helmus]

Instead of renaming my references I've found a different solution :
I have just added the following to my .jshintrc file

"globals"      : {
    "Promise"   : true
}

Works fine !
EDIT: to be clear, I don't think this is very good solution, it's just the easiest for me now. Using an alternative reference like BPromise seems the best solution to me.
I wouldn't really use Bluebird as reference like they did here: krakenjs/kraken-js@701f4c3 It just reads awkward to me, It should be clear that it's a Promise, regardless of the implementation.

[xaka]

@rwaldron I'd disagree with the community wantedPromisebaked into the language being a reason to make Promise a global name because right now language != Node.js environment != browser environment. jshint shouldn't be so restrictive in this case, I'd also say it's backward-incompatible change that landed to minor version update which started affecting all of our (and probably other people) builds. I don't think you'd argue that a lot of devs are using bluebird and assign it to Promise and I don't think we're going to see native Promise in Node.js soon enough to claim Promise as a global everywhere. Even when we have Promise everywhere, it'll be difficult to make a switch from bluebird or whatever library people are using because they're also using additional APIs provided by those libraries. The workaround with globals: { ... } is also a bad idea because you're introducing a hot-spot and if develop forgets to do var Promise = ..., jshint won't complain, but your code will crash during runtime. My suggestion would be either make sure Promise isn't global by default when node: true OR revert the change entirely and let people decide until we see Promise enabled by default (w/o additional flags or magic, etc.) in Node.js and browser.

[caitp]

Promise is already a global in V8/node so long as you pass in --harmony-promises or --harmony.

In V8 3.28.71.2, Promise, Symbol, and harmony collections (Map/Set/WeakMap/WeakSet) are all enabled by default (with harmony-arrays and a few other features coming soon), so really it's just a matter of time before these are enabled by default unless joyent/strongloop explicitly delete those globals while bootstrapping, which seems unlikely.

Promise is part of the ecosystem, but it makes sense to be able to disable es6 globals in configuration, IMO. (if you want to see how other features are coming, http://www.chromestatus.com/features#es6 is a good indicator of when it's coming to V8, and when it's pref'd on by default).

For what it's worth, Mozilla isn't even hiding harmony features behind a preference, and a number of these features are enabled by default in Chrome --- so this really is a browser environment. As for node, as said above it's really just a matter of time

[xaka]

@caitp Not in node v0.10.x and v0.12.x isn't released yet (and probably won't be for next N months) and even when it'll come to this world, I doubt people are going to make a switch right away and I doubt even more that people are going to run in production with --harmony-promises. I agree though that there should be some kind of es6 option in configuration. It'd be victory for everyone.

[caitp]

there is an es6 option, but the Promise, Symbol, Reflect, Map, Set, WeakMap and WeakSet globals are not predefined based on the presence of that flag.

Maybe they should be, it's probably a pretty simple CL to put together

[rwaldron]

@xaka I added all of the new global api names to the Ecma identifiers list and I'm not taking them out. As of 2.5.4 you may do this:

/* global -Promise */
var Promise = require("promise");

Or in predef in a .jshintrc:

{
  "undef": true,
  "unused": true,
  "predef": [ "-Promise" ]
}

[WebReflection]

Is there any reason the following would not work on module level ?

var Promise = global.Promise  || require("promise");

or this on global one

var Promise = Promise  || require("promise");

That would free the module from concerns it should not have but in any case if a native implementation is actually broken, I don't see why we should not be able to overwrite it locally or even globally, as done before to patch engines here and there.

[simonexmachina]

I don't know how that's more understandable, but anyway... ;)​

[sam-github]

Even with Promise in v8/io.js, its deathly slow, and _unlike_ Array, Promise was explicitly designed to allow alternative and interoperable implementations.

Promise = require('bluebird');

should be a warning

var Promise = require('bluebird');

should not be, IMO. I've gone the globals route, ATM.

[aldendaniels]

@domenic - would love your input here. Is overriding window.Promise on the web or global.Promise on node/iojs within a local scope future-proof?

Example:

var Promise = require('bluebird');

[rwaldron]

Is overriding window.Promise on the web or global.Promise on node/iojs within a local scope future-proof?

This is self contradictory.

[domenic]

No, it is future hostile.

[aldendaniels]

@domenic - Thanks!

@rwaldron -

This is self contradictory.

Sorry for the confusion. I've updated my question for clarity. I meant, "is redefining the Promise builtin within a local scope future-proof".

The examples provided by @petkaantonov in bluebird redefine the Promise builtin, which led me to wonder if (in the case of Promises particularly - per @sam-github's comment) this was actually accepted practice.

Sounds like the answer is "no".

[sam-github]

#1747 (comment)

@domenic why future hostile? Promises were explicitly designed by spec to support alternate co-existing implementations... why not call the promise implementation being used in a particular node module Promise, rather than, say, Bluebird, or MyPromise?

Is this going to somehow confuse the v8 optimizer? I wouldn't think so.

Or is it because you hope that all other promise implementations are going to die and only the one shipping in the js runtime will remain?

[domenic]

For the same reason it is future hostile to overwrite Object, Map, or window.

[sam-github]

I dunno. var Promise = Promise || require('bluebird') seems future proof, not future hostile. Object has always existed, Promise barely exists.

[caitp]

treating them as interchangeable is a good way to break the web, please don't do that u_u use one or the other, explicitly

[rwaldron]

For the same reason it is future hostile to overwrite Object, Map, or window.

Yep, exactly as I've said before.

[sam-github]

Ah, well, this is just going in circles, but any serious attempt to convince the world that all existing code that has var Promise in it suddenly became wrong sometime recently (even in environments that have no Promise implementation, like pretty much all node code in production), is going to take a bit more work.

Drawing analogies between Promise and js builtin types that have existed for decades needs a bit more backup than simply asserting they are analogous.

[rwaldron]

(even in environments that have no Promise implementation, like pretty much all node code in production)

There is no way for JSHint to know what runtime the code it's analyzing will run in. JSHint isn't going to break anyone's production code. If you want to pave over the Identifer bindings of built-in objects, just do it, and tell JSHint to shut up

[caitp]

just do it, and tell JSHint to shut up

This is perfectly fine --- but try not to treat native implementations and third party implementations as interchangeable, unless you have good reason to be sure that the third party implementation is as spec-compliant as possible, or is at least trying to be, so that there's a bit more room for the spec to grow if it needs to without breaking applications

[helmus]

var Promise = Promise || require('bluebird')

That is horrible, don't do that ever

[PavelPolyakov]

How this issue should be fixed at the end?

I've tried the

futurehostile : true,

But still have this warning:

From the other point:

predef   : ['angular','-Promise']

works, but as I understand I should stick to the first solution.

Any thoughts?

[andrew-luhring]

I've got a similar issue:
I have a file that I use both in the browser and in node;
When in the browser, I rely on the function being a global function (it's an object that contains a bunch of custom matchers for jasmine), but when it's in node, I export it via module.exports.

"use strict";
var module = module || {
    exports : {}
};

function Matchers(){ 
    // code...
}
// code...

module.exports = Matchers;

Jshint throws the error:

  line 3  col 5  Redefinition of 'module'.

two questions:

1. If what I'm doing is ok, is this a bug with jshint?
2. If I shouldn't do this, what should I do?

[WebReflection]

@andrew-luhring beside the empty block within the constructor, this would pass:

"use strict";
var module;
if (!module) {
    module = {
        exports: {}
    };
}

function Matchers() {
    // put something here
}

module.exports = Matchers;

what's the difference? actually there's no concrete difference with what you wrote, it's just a more prolix and maybe clear way to do the same ( I would have used your version as well, give JSHint a spin ;-) )

Cheers

[joegoldbeck]

I'm probably missing something, but is there a way to suppress this error? I am using esnext: true, which is the option which generates the error, and have tried such varied options to override the specific Promise behavior as

  "globals": {
    "Promise": false // or true
  },
  "-Promise": false,
  "predef": [ "-Promise" ]

but none of them seem to override the error.

The error only occurs with let Promise = and const Promise=. There is no error with var Promise=

Update: After some futzing, I got it to work with
"globals": { "Promise": null }

[jugglinmike]

Hi @joegoldbeck. While I'm glad you found a workaround that solved your
problem, that is not a formally supported usage of the globals configuration.
Since we don't document it or test it, it may not work in a future release of
JSHint. I just verified that the following configuration suppresses the warning
you reported for both let and const bindings:

{
  "esversion": 6,
  "predef": ["-Promise"]
}

This is a safer approach, so please consider using it instead.

[joegoldbeck]

Hi @jugglinmike thanks for the response! I'd much prefer to use a supported configuration. The one you suggest isn't working for me for some reason. I've stripped my .jshintrc so that it only contains those lines and I still get the W079 warning. I am using JSHint v2.9.4 on WebStorm.

[rsp]

I'd like to point out that to this day, as of April 2017, the Bluebird docs still recommend this usage in Node:

var Promise = require("bluebird");

and every single piece of documentation uses names like Promise.promisifyAll (not Bluebird.promisifyAll) to describe methods that are available in Bluebird but not in native Promise.

That is why I would expect everyone to follow the usage recommended in the Bluebird documentation unless it it changed to reflect the recommendation expressed here.

See: http://bluebirdjs.com/docs/getting-started.html

Also, it seems that Bluebird doesn't agree that redefining the Promise is a bad idea - see issue petkaantonov/bluebird#715 - so is seems that following Bluebird examples will make a code break with ESLint. There seems to be no consensus here.