babel / babel/babel

[7.0.0-beta.3] babel-preset-env useBuiltIns "usage" (possibly) adding unnecessary polyfills

Open
#6,491 10 comments 0 reactions 0 assignees View on GitHub
pkg: polyfill pkg: preset-env
Dominant language
TypeScript
Stars
44k
Forks
6k
Avg merge
5d 15h
Merged PRs (30d)
23

Description

**Bug report**

I am working with an angular component library developed for an internal application for my organization. It is built via webpack, and therefore uses babel-loader to run transpilation. The current version (not the experimental branch _this_ issue uses) works off v6.26.0 for all babel libs, and v1.6.0 of babel-preset-env. The transpilation works as necessary, but includes _all_ polyfills based on the target environments. In an effort to reduce the size of the final bundle, this experiment was undertaken. Instead of posting a ton of very similar issues for each polyfill problem I encountered (though I will, if desired), I am posting as much as I can in this particular issue. I will try to keep each instance as concise as possible, displaying each issue with only applicable code (as I see it).

### Input Code
#### `es6.function.name` (common, angular-specific)
This issue has to do with the common practice of exporting the name of an angular module while simultaneously defining it. It does not export any function name, but rather the name of the defined module.
```js
// Debug Output
// Added following polyfill:
// es6.function.name { "android":"4.4.3", "edge":"14", "ie":"11" }

// Input Code (brief)
import angular from 'angular';

export default angular.module('nameThatGetsExported', [])
.name;
```

#### `es6.array.find` (common, angular-specific)
This issue has to do with the jqLite function `find()` for an angular element. It is library-specific, so no need for a polyfill.
```js
// Debug Output
// Added following polyfill:
// es6.array.find { "android":"4.4.3", "ie":"11" }

// Input Code (brief)
export default class SomethingElementService {
/* @ngInject */
constructor(/* $compile, other angular injectables */) {
// e.g. many of these...
// this.$compile = $compile;
}

makeSomething(options = {}) {
// this._options.parent is expected to be a jqLite element
// this is the _only_ reference to a `find()` method in this file
const $something = this.$element || this._options.parent.find('something-element');
// ...
}
}
```

#### `es6.object.set-prototype-of`, `es6.symbol`
This polyfill was only included in files that define angular controllers that extend a base angular controller class. This may not be an issue, but I thought there was already a plugin that handled class transpilations (e.g. `transform-es2015-classes`).
```js
// Debug Output
// Added following polyfill:
// es6.object.set-prototype-of { "android":"4.4.3" }
// es6.symbol { "android":"4.4.3", "edge":"14", "ie":"11" }

// Input Code (only names of controllers changed, everything else as-is)
import BaseCtrl from './base-ctrl.es6';

export default class ExtendedCtrl extends BaseCtrl {
/* @ngInject */
constructor($element, $transclude) {
super($element, $transclude);
}
}
```

#### `es6.regexp.match`, `es6.regexp.match`
This issue is similar to the issue with `es6.array.find` in that it seems to just see the method names and then add the polyfills. There may be confusion with the string methods that have the same name and are using RegExps for the pattern. It should be noted that some of the code in the example is working with jqLite elements, and lodash is being used. Also, for brevity, a similar polyfill, `es6.regexp.split`, is added in another file with a similar implementation as below, where `.split()` is being called on a string variable using a RegExp as the pattern.
```js
// Debug Output
// Added following polyfill:
// es6.regexp.match { "android":"4.4.3", "edge":"14", "ie":"11" }
// es6.regexp.search { "android":"4.4.3", "edge":"14", "ie":"11" }

// Input Code (brief; only relevant code with match/search methods shown)
import _ from 'lodash';

export default class SomeCtrl {
/* @ngInject */
constructor(/*angular injectables*/) {
// many of these...
// this.$injectable = $injectable;
}

_handleKeyup(e) {
const rawInput = this.inputElement.val();
const before = rawInput.substring(0, this.inputElement[0].selectionStart).match(/[@\w]*$/)[0];
const after = rawInput.substring(this.inputElement[0].selectionStart, rawInput.length).match(/^[@\w]*/)[0];
const currentWord = before + after;
if (_.includes(currentWord, '@')) {
this._search(currentWord);
} else {
this._closeMenu();
}
}

_handleSelected(user) {
const rawInput = this.inputElement.val();
const selectionStart = this.inputElement[0].selectionStart;
const left = rawInput.slice(0, selectionStart);
const leftPos = left.search(/\S+$/);
// ...
}

_search(query) {
return this.someService.search(_.trimStart(query, '@'), this.contextType, this.contextId)
.then(/*...*/)
// ...
}
}
```

### Babel Configuration (.babelrc, package.json, cli command)
```js
{
"presets": [
["env", {
"modules": false,
"targets": {
// browserlist combined by OR clause
// https://github.com/ai/browserslist
"browsers": ["last 2 versions", "not ie <= 10"],
},
"useBuiltIns": "usage",
"debug": true,
}],
],
"plugins": [
[
"transform-es2015-modules-commonjs-simple", {
"noMangle": true, // needed or DI explodes
},
],
],
}
```

### Expected Behavior
No polyfills are needed/added.

### Current Behavior
Polyfills added.

### Possible Solution
It seems like babel has some trouble determining when some functions are defined by a third party library or by an implementor, and instead just looks for some matches based on name alone. I realize that this is incredibly difficult to trace, especially if any directories are excluded from transpilation. Perhaps there should be some sort of comment exclusion (like eslint uses with `/* eslint-disable */` comments) to explicitly tell babel not to include some polyfills in situations where it may not always be realistic for babel-preset-env/babel to determine polyfill necessity itself.

### Context
Adding unnecessary polyfills increases the overall bundle size. While it appears that there are less overall polyfills being added than the previous version of babel-preset-env, I imagine that striving for accuracy with the new functionality is desirable.

### Your Environment

| software | version(s)
| ---------------- | -------
| Babel(core, preset-env, polyfill) | 7.0.0-beta.3
| webpack | 3.5.5
| babel-loader | 7.1.2
| node | 6.10.3
| npm | 3.10.10
| Operating System | Ubuntu 16.04 (Vagrant)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.