Skip to content
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

Include es.promise.finally in PromiseDependencies #136

Conversation

mwijngaard
Copy link

Include es.promise.finally in PromiseDependencies, so it is correctly loaded when the native Promise constructor is overwritten.

context:
Version 3.24.0 of CoreJS changed how the loading of the Promise constructor shim behaves in NodeJS environments, causing it to overwrite the native Promise constructor when JSDom is loaded (e.g. with Jest), because JSDom tries to emulate a browser but misses the global PromiseRejectionEvent. I would agree with the author that this is correct behavior from a core-js point of view (zloirock/core-js#1110 (comment)). However, this means that when using babel-polyfills, the new Promise is missing a finally method, because it is not listed as one of the PromiseDependencies, causing 3rd party code that is not run through Babel to break.

Although this is how the problem occurs to me, I don't think this problem is JSDom-specific. It could occur in any browser that has a non-compliant native Promise implementation.

This PR adds es.promise.finally to the PromiseDependencies, so it always gets loaded when Promises are used, and not just when the finally method is used in transformed code

… loaded when a native Promise constructor is overwritten
@zloirock
Copy link
Member

I don't think that it's a good idea. Now, es.promise.finally injected only when .finally method is used and it should not be in common Promise dependencies. It can be missed if code that uses .finally is not transpiled - however, I don't think that it's a core-js or babel issue.

@zloirock
Copy link
Member

zloirock commented Jul 27, 2022

It can be extrapolated to all other constructors and methods. For example, Promise.allSettled also will be missed in your case if your code is not transpiled.

@mwijngaard
Copy link
Author

I understand, but I do believe that in practice that means that it is no longer possible to exclude third party code in node_modules from transformation when using babel-preset-env with useBuiltins: usage, since whether or not the finally method would exist depends on if you use it in your transformed code.

@zloirock
Copy link
Member

Similarly to .allSettled and all other features that should be polyfilled. It's the expected limitation of the usage polyfills injection mode.

@mwijngaard
Copy link
Author

Very well.

@mwijngaard mwijngaard closed this Jul 27, 2022
@zloirock
Copy link
Member

For proper solving of such issues / this limitation required a new polyfills injection mode that can't be properly solved only on the transpiler side. I'm experimenting with it, but I have no idea when it will be available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants