Promise constructor being polyfilled incorrectly in nodejs environment with jsdom

[mwijngaard]

Commit c08f5e6 inverted an if statement in the promise constructor detection, causing it to now return true for nodejs environments with jsdom loaded, e.g. with Jest (due to: V8_VERSION >=51, native promise, window object present and thus IS_BROWSER=true). I'm not exactly sure why the if statement was inverted, but it doesn't seem like it was the intention to leave the last line out. Now it actually means something different.

[zloirock]

This if statement is incomplete, thanks, I missed some changes. However, in V8, including NodeJS, all works as expected.

V8 Promise had an early exit for preventing deoptimization. Because of Deno < 1.24 Promise, some checks enabled for V8 Promise again.

jsdom polyfill a part of the browser environment and after that core-js think that the engine is a browser. However, it seems it does not polyfill PromiseRejectionEvent, so core-js polyfill Promise as it's expected for a browser environment without PromiseRejectionEvent / unhandledrejection event support.

So core-js works as expected.

[zloirock]

I don't think that it's a core-js issue, so I think that it should be closed.

[zloirock]

It seems it should be fixed on jsdom side jsdom/jsdom#2401

[mwijngaard]

Ah, interesting! Thanks for pointing that out. The issue I'm experiencing is actually that the polyfilled Promise is missing a finally method. I thought it was due to it being polyfilled by mistake, but it seems I have to look further. I'll close it for now.

[zloirock]

Most likely it's related to the way how you load polyfills. You can manually load core-js/actual/promise/finally.

[gbettencourt]

@zloirock I'm running into the same issue, did an npm install which pulled in an updated core-js version. Now when I run jest (node v14), I get the following error:

TypeError: promise.then(...).catch(...).finally is not a function

[zloirock]

@gbettencourt the situation explained above, I don't think that makes sense to explain something more. In NodeJS 14 with a non-broken environment (when NodeJS does not try looks like a browser or Promise already was not overwritten by any incorrect polyfill) core-js does not polyfill Promise. .finally is not a function because loaded only es.promise module, but es.promise.finally is missed, so the problem in your way of polyfills loading.

[gbettencourt]

@gbettencourt the situation explained above, I don't think that makes sense to explain something more. In NodeJS 14 with a non-broken environment (when NodeJS does not try looks like a browser or Promise already was not overwritten by any incorrect polyfill) core-js does not polyfill Promise. .finally is not a function because loaded only es.promise module, but es.promise.finally is missed, so the problem in your way of polyfills loading.

I'm not actually polyfilling anything. I'm using storybook 6.5, and importing the story to run the test. Perhaps they're doing something incorrectly and I can open an issue with them. But all I know from looking at my package-lock is that core-js is the only dependency that updated and it broke, pinning an older version fixed my problem. So surely this will impact others.

[mwijngaard]

@gbettencourt In my case I'm running my code through babel but not using a finally method in the code that gets transformed, causing babel-polyfills to load the promise constructor polyfill, but not the finally polyfill. Although I'm not too sure about the fact that the finally method is not included in es.promise in core-js, I would agree with the author that core-js is behaving as it should and the problem should be resolved in babel-polyfills, so I've openend babel/babel-polyfills#136 there.

[gbettencourt]

@mwijngaard also using babel, and looking at the stack the code using finally is in React testing library which we don't transpile.

[mwijngaard]

@gbettencourt The solution would be to not use babel-preset-env with useBuiltIns: usage, but use useBuiltIns: entry

[gbettencourt]

@mwijngaard not setting that option for preset-env, so it should default to entry