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

customizable level of aggressiveness #283

Closed
bisubus opened this issue Feb 13, 2017 · 4 comments
Closed

customizable level of aggressiveness #283

bisubus opened this issue Feb 13, 2017 · 4 comments

Comments

@bisubus
Copy link

bisubus commented Feb 13, 2017

Using Bluebird as global Promise is quite popular, just because it is good (at error handling in particular) and it is fast.

core-js/shim includes core-js Promise polyfill and overrides any other Promise implementation which it considers as bad. From my understanding of situation, Bluebird doesn't pass @@species test and thus is being replaced with ES6 Promise polyfill:

global.Promise = require('bluebird');
// ...
// some nested module that depends on ES6+ features
require('core-js/shim');
global.Promise !== require('bluebird');

I'm not sure if @@species support for promises is really a feature in demand, but I'm positive that among the developers who use Bluebird as global Promise there is no single person who would want to replace it with ES6 Promise polyfill by accident (I've had an unfortunate case before and had to use a descriptor to protect Promise from core-js).

I cannot remember any other core-js polyfill that would be harmful when being accidentally applied, but if the problem may be not specific to Promise, can the level of core-js aggressiveness be made customizable (e.g. with environment variable)?

Usually I would expect from a polyfill that it will be applied if and only if a variable/method doesn't exist. This is the level of polyfilling that I would call generally desirable, and I was surprised by the fact that core-js applies the polyfills by force on some conditions (like a 'bad' promise implementation), and this cannot be adjusted.

I'm aware of core-js/library, so the issue applies to the cases when core-js/shim with no cherry-picking would be good, with the exception of the described behaviour.

@loganfsmyth
Copy link
Contributor

// some nested module that depends on ES6+ features

Generally things in node_modules shouldn't load global polyfills at all, that seems like the real issue here. If a dependency module is mutating global state, filing a bug there is a good first step.

In a normal workflow, I'd expect that the only things that initiate global mutations, like changing Promise, would be the top-level application (never a library), and in those cases the top-level application has control over the ordering of things, so you could simply assign Bluebird as the global after loading core-js.

@bisubus
Copy link
Author

bisubus commented Feb 13, 2017

@loganfsmyth I agree with you on that. But it would be great if core-js had less chances of breaking things, as de facto polyfill library. It is great to have 'strict to the specs' polyfilling mode in case if it is needed, but if it is default and only one, this may cause issues.

Polyfilling usual things like Array.from, Object.values, etc in third-party code looks rather reasonable and painless to me, their use is more consistent than non-native substitutes for these operations, and probably better than instructing the users in readme which ES201* polyfills they should have in order to make the code work (this depends a lot on what is the library and which platforms it aims at).

Even though variable polyfills (Map, Symbol) are better to be imported from core-js/library, it is unlikely that they will cause any problems too. So it is Promise that really makes the difference in my opinion, that's why it was addressed in the issue in the first place.

The example is feathersjs/feathers#510 . The package uses babel-polyfill to provide the support for Node <6 out of the box (probably not the best move but it was made), this cannot be changed any time soon because this would be a breaking change. Obviously, this is not the package that would be usually imported first, but the fact that it imports core-js/shim internally suggests that it would be better if it would. Even if global.Promise = require('bluebird') is done before and after feathers import, the package itself may continue to use polyfilled ES6 Promise and will require to wrap its promises with Promise.resolve().

@zloirock
Copy link
Owner

zloirock commented Apr 5, 2017

Sorry, but @loganfsmyth right, it's how polyfills work.

can the level of core-js aggressiveness be made customizable (e.g. with environment variable)?

I'll think about it.

@bensgroi
Copy link

I'm seeing a similar issue with Object.freeze. core-js overrides IE 11's native functionality here too, which I'm seeing cause stack overflows when used in deep-freeze-strict. (Specifically, it has something to do with onFreeze functionality that it adds.)

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

No branches or pull requests

4 participants