-
Notifications
You must be signed in to change notification settings - Fork 595
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
Reassign global Promise to es6-promise if Promise.prototype.finally() is undefined. #331
base: master
Are you sure you want to change the base?
Conversation
If anyone needs this patch before it is reviewed (or wants to review it without a const ep = require('es6-promise');
ep.patchedPolyfill = function(){
let local;
if (typeof global !== 'undefined') {
local = global;
} else if (typeof self !== 'undefined') {
local = self;
} else {
try {
local = Function('return this')();
} catch (e) {
throw new Error('polyfill failed because global object is unavailable in this environment');
}
}
let P = local.Promise;
if (P) {
var promiseToString = null;
try {
promiseToString = Object.prototype.toString.call(P.resolve());
} catch(e) {
// silently ignored
}
if (promiseToString === '[object Promise]' && !P.cast){
if (typeof P.prototype.finally !== "undefined"){
return;
}
}
}
local.Promise = ep;
};
// returns 'undefined', in desktop Safari (but likely not all browsers)
console.log(`typeof Promise.prototype.finally, before polyfill:`, typeof Promise.prototype.finally);
ep.patchedPolyfill();
// returns 'function'.
console.log(`typeof Promise.prototype.finally, after polyfill:`, typeof Promise.prototype.finally); |
@jakearchibald would love you input on this. |
It can polyfill only |
I suppose what would be ideal would be to add some configuration to the polyfill function. Here are two options (presented using TypeScript-style interfaces) for comment: // Option #1: Simply opt-in to ALL features beyond ES6/ES2015
interface Polyfill {
// DEFAULT: polyfill(true, true);
polyfill(beyondES2015?: boolean, asAugmentation?: boolean): void;
}
// Option #2: Opt-in to a specific ECMAScript version.
interface Polyfill {
// DEFAULT: polyfill("ES2018", true);
polyfill(ESVersion?: "ES2015"|"ES2018"|"next", asAugmentation?: boolean): void;
} Options1: Simply opt-in to ALL features beyond ES6/ES2015Nice because it's simple. 2: Opt-in to a specific ECMAScript version.Helpful because it allows users to polyfill only an ECMAScript version of this library that would be in line with the rest of their libraries. Issues with bothAny beyond-ES6/ES2015 feature will have to be written as optional into our distributed TypeScript typings, and all TypeScript users will have to mark usages of
|
Alternatively, instead of adding parameters to Also: I just realised that if we parameterised |
its been two months.. any ideas here? |
@stefanpenner is there anything we can do without @jakearchibald's input in the mean time? |
this feature is currently in TC39 stage4,why this request hasn't been merged yet? |
Hi Guys |
Although this is semantically accurate, Im not sure its super valuable. I suspect if we nail the details here, we can simply have es6-promise's uses benefit... Kinda sucks if you gotta choose different modules entirely to get what you want.
This may be a path forward, some random thoughts: require('es6-promise/es-2018'); // forces finally
require('es6-promise/ensure-finally'); // forces finally |
@stefanpenner The conundrum I was wondering about mainly was: If the browser doesn't have
Currently my PR does 2). Do users have any preference towards 1)? |
What are chances of getting a browser with native Promises, but without finally? |
Chances are good. Chrome v66 and a recent version of FireFox didn’t have it. We had to poly fill with Bluebird, because es6-promise didn’t work. |
I'd go with granular approach then (see what i did? :D) |
Fixes #330 .