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

[CS2] Polyfills? #4674

Closed
GeoffreyBooth opened this issue Aug 31, 2017 · 3 comments
Closed

[CS2] Polyfills? #4674

GeoffreyBooth opened this issue Aug 31, 2017 · 3 comments
Labels
Milestone

Comments

@GeoffreyBooth
Copy link
Collaborator

(From @helixbass #4652 (comment)):

Babel outputs polyfills for code it generates, but not for code the CoffeeScript compiler outputs. In other words, sending Babel this:

var obj2 = {...obj1};

will result in this:

"use strict";

var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

var obj2 = _extends({}, obj1);

But sending it the CoffeeScript compiler output of obj2 = {obj1...}:

var obj2;

obj2 = Object.assign({}, obj1);

Babel leaves it unmodified. Babel’s babel-preset-env converts syntax for compatibility with browser presets, but it doesn’t add polyfills that your target browsers may need (such as for Object.assign in this case).

I for one was under the mistaken impression that Babel covered us in such cases. I’ve done some research and there doesn’t appear to be any tool out there to scan your code to identify which polyfills you’ll need. There’s some discussion that Babel should do this, but it seems a ways off. Here’s a great article that mentions the same idea. There’s also http://jscc.info/, but it appears to be several years out of date (it doesn’t know about Object.assign, for example).

In #4526 we removed polyfills for Array.indexOf and Function.prototype.bind, which are supported in Internet Explorer 9+. In #4493 we added Object.assign (not supported in any version of Internet Explorer, even 11) without a polyfill.

As far as I can tell, reading through nodes.coffee, those are the only browser native functions that might possibly need polyfilling. The others we use—{}.hasOwnProperty, [].slice, [].splice—are so old that MDN just lists “(yes)” instead of a browser version when they were first supported.

We should do one of the following:

  1. Update the docs to mention that polyfills are required if Internet Explorer support is desired (spelling out which polyfills for which versions based on the above).
  2. Polyfill Object.assign similarly to how we used to polyfill Array.indexOf, probably using the same polyfill that Babel outputs (see second code block above). Mention in the docs that if support for IE8 is desired, the user should polyfill Array.indexOf and Function.prototype.bind.
  3. Polyfill all three, and leave the docs alone. We would write the polyfills like Babel does, so that the native method is used if available: Object.assign || ....
@helixbass
Copy link
Collaborator

@GeoffreyBooth thanks for the useful summary. I think # 2 is the most sensible option. Haven't read the original discussion of removing indexOf and bind polyfills but targeting IE9+ (with, as you suggest, a mention in the docs) seems like the right choice. For Object.assign, you'd be forcing most people to complicate their build chain/code to include the polyfill so I don't think it's a good idea not to "polyfill" it ourselves

Opened #4675 which replaces the direct call to Object.assign() when compiling object spreads into a call to a new _extends utility (copied directly from Babel's)

@GeoffreyBooth
Copy link
Collaborator Author

Agreed. I added the docs note into your PR.

@GeoffreyBooth
Copy link
Collaborator Author

Fixed via #4675.

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

No branches or pull requests

2 participants