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

When Object.prototype is polluted, polyfilling can fail #973

Closed
kellym opened this issue Aug 17, 2021 · 5 comments
Closed

When Object.prototype is polluted, polyfilling can fail #973

kellym opened this issue Aug 17, 2021 · 5 comments

Comments

@kellym
Copy link

kellym commented Aug 17, 2021

When working on a library that is run on sites with other scripts, we ran into an issue where, while trying to avoid global pollution, we found ourselves polluted by another script.

In essence, another script had modified Object.prototype and had defined a property includes that was not configurable on all new objects. This threw an error for us while trying to polyfill [].includes.

A quick and dirty solution was to replace some references of {} in core-js/packages/core-js-pure/override/internals/export.js to Object.create(null).

I'm trying to submit a proper PR to fix this, but was wondering, is there a proper way to use a polyfill inside this export file?

@zloirock
Copy link
Owner

zloirock commented Aug 17, 2021

Could you create a reproducible example for understanding with which subset of the problem we are dealing? core-js have some such assumptions on initialization to minimize minimalistic bundles size and coupling.

@kellym
Copy link
Author

kellym commented Aug 18, 2021

Sure, for a quick example of something we've actually run into:

// third party script does a polyfill like so
Object.defineProperty(Object.prototype, 'includes', {
  configurable: false,
  writable: false,
  value: function(i) { return this.indexOf(i) != -1; }
});
// core-js now does this: 
'use strict';

const path = {};
path['String'] = {}
path['String'].includes = () => 'new function';
// ^ Throws: Uncaught TypeError: Cannot assign to read only property 'includes' of object '<Object>'

Here's an example of a commit that fixes the error: activeprospect@ef571d6. Note, though, it checks for Object.create, which is only present in IE9+. Fortunately, Object.defineProperty is only fully implemented in IE9+ as well, so it's fairly likely that commit will effectively resolve the issue.

@zloirock
Copy link
Owner

zloirock commented Aug 18, 2021

I'm not sure should such workaround be added on the core-js side or not since it's the case of a broken environment and it could cause some other problems. I'll think about it.

@zloirock
Copy link
Owner

I'll add a workaround, but in a little different way.

@kellym
Copy link
Author

kellym commented Aug 18, 2021

Awesome, thanks! I appreciate the help!

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

2 participants