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

Add ability to deduplicate babel helpers. #251

Merged
merged 1 commit into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = {
'blueprints/*/index.js',
'config/**/*.js',
'tests/dummy/config/**/*.js',
'lib/**/*.js'
],
excludedFiles: [
'addon/**',
Expand Down
31 changes: 31 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ interface EmberCLIBabelConfig {
*/
'ember-cli-babel'?: {
includePolyfill?: boolean;
includeExternalHelpers?: boolean;
compileModules?: boolean;
disableDebugTooling?: boolean;
disablePresetEnv?: boolean;
Expand Down Expand Up @@ -159,6 +160,36 @@ let app = new EmberApp(defaults, {
});
```

#### External Helpers

Babel often includes helper functions to handle some of the more complex logic
in codemods. These functions are inlined by default, so they are duplicated in
pzuraq marked this conversation as resolved.
Show resolved Hide resolved
every file that they are used in, which adds some extra weight to final builds.
pzuraq marked this conversation as resolved.
Show resolved Hide resolved

Enabling `includeExternalHelpers` will cause Babel to import these helpers from
pzuraq marked this conversation as resolved.
Show resolved Hide resolved
a shared module, reducing app size overall. This option is available _only_ to
pzuraq marked this conversation as resolved.
Show resolved Hide resolved
the root application, because it is a global configuration value due to the fact
that there can only be one version of helpers included.

Note that there is currently no way to whitelist or blacklist helpers, so all
helpers will be included, even ones which are not used. If your app is small,
this could add to overall build size, so be sure to check.

`ember-cli-babel` will attempt to include helpers if it believes that it will
lower your build size, using a number of heuristics. You can override this to
force inclusion or exclusion of helpers in your app by passing `true` or `false`
to `includeExternalHelpers` in your `ember-cli-babel` options.

```js
// ember-cli-build.js

let app = new EmberApp(defaults, {
'ember-cli-babel': {
includeExternalHelpers: true
}
});
```

#### Enabling Source Maps

Babel generated source maps will enable you to debug your original ES6 source
Expand Down
3 changes: 2 additions & 1 deletion ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ module.exports = function(defaults) {
let app = new EmberAddon(defaults, {
'ember-cli-babel': {
includePolyfill: true,
includeExternalHelpers: true,
// ember-cli-babels defaults, should be parallelizable. If they are not,
// it should fail to build. This flag being enabled ensure that to be the
// case.
throwUnlessParallelizable: true
throwUnlessParallelizable: true,
}
});

Expand Down
98 changes: 96 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ const clone = require('clone');
const path = require('path');
const semver = require('semver');

const defaultShouldIncludeHelpers = require('./lib/default-should-include-helpers');
const findApp = require('./lib/find-app');

const APP_BABEL_RUNTIME_VERSION = new WeakMap();

let count = 0;

module.exports = {
Expand Down Expand Up @@ -84,18 +89,99 @@ module.exports = {
}
},

_shouldIncludeHelpers() {
let customAddonOptions = this.parent && this.parent.options && this.parent.options['ember-cli-babel'];

if (customAddonOptions && 'includeExternalHelpers' in customAddonOptions) {
throw new Error('includeExternalHelpers is not supported in addon configurations, it is an app-wide configuration option');
}

let appOptions = this._getAppOptions();
let customOptions = appOptions['ember-cli-babel'];

let shouldIncludeHelpers = false;

if (customOptions && 'includeExternalHelpers' in customOptions) {
shouldIncludeHelpers = customOptions.includeExternalHelpers === true;
} else {
// Check the project to see if we should include helpers based on heuristics.
shouldIncludeHelpers = defaultShouldIncludeHelpers(this.project);
}

let appEmberCliBabelPackage = this.project.addons.find(a => a.name === 'ember-cli-babel').pkg;
let appEmberCliBabelVersion = appEmberCliBabelPackage && appEmberCliBabelPackage.version;

if (appEmberCliBabelVersion && semver.gte(appEmberCliBabelVersion, '7.3.0-beta.1')) {
return shouldIncludeHelpers;
} else if (shouldIncludeHelpers) {
this.project.ui.writeWarnLine(
`${this._parentName()} attempted to include external babel helpers to make your build size smaller, but your root app's ember-cli-babel version is not high enough. Please update ember-cli-babel to v7.3.0-beta.1 or later.`
);
}

return false;
},

_getHelperVersion() {
if (!APP_BABEL_RUNTIME_VERSION.has(this.project)) {
let checker = new VersionChecker(this.project);
APP_BABEL_RUNTIME_VERSION.set(this.project, checker.for('@babel/runtime', 'npm').version);
}

return APP_BABEL_RUNTIME_VERSION.get(this.project);
},

_getHelpersPlugin() {
return [
[
require.resolve('@babel/plugin-transform-runtime'),
{
version: this._getHelperVersion(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how they're using version in the implementation, but I can't find any documentation for it on the plugin page. I don't imagine that's intended to be a "secret" option, but have you seen anything indicating it's public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not, but it's definitely needed to actually include decorator helpers or anything more recent than 7.0.0-0, so I can't imagine it's not public? Does seem very strange to me though

regenerator: false,
useESModules: true
}
]
]
},

treeForAddon() {
// Helpers are a global config, so only the root application should bother
// generating and including the file.
if (!(this.parent === this.project && this._shouldIncludeHelpers())) return;

const path = require('path');
const Funnel = require('broccoli-funnel');
const UnwatchedDir = require('broccoli-source').UnwatchedDir;

const babelHelpersPath = path.dirname(require.resolve('@babel/runtime/package.json'));

let babelHelpersTree = new Funnel(new UnwatchedDir(babelHelpersPath), {
srcDir: 'helpers/esm',
destDir: '@babel/runtime/helpers/esm'
});

return this.transpileTree(babelHelpersTree, {
'ember-cli-babel': {
// prevents the helpers from being double transpiled, and including themselves
disablePresetEnv: true
}
});
},

treeForVendor() {
if (!this._shouldIncludePolyfill()) { return; }
if (!this._shouldIncludePolyfill()) return;

const Funnel = require('broccoli-funnel');
const UnwatchedDir = require('broccoli-source').UnwatchedDir;

// Find babel-core's browser polyfill and use its directory as our vendor tree
let polyfillDir = path.dirname(require.resolve('@babel/polyfill/dist/polyfill'));

return new Funnel(new UnwatchedDir(polyfillDir), {
let polyfillTree = new Funnel(new UnwatchedDir(polyfillDir), {
destDir: 'babel-polyfill'
});

return polyfillTree;
},

included: function(app) {
Expand Down Expand Up @@ -123,6 +209,12 @@ module.exports = {
return (this.parent && this.parent.options) || (this.app && this.app.options) || {};
},

_getAppOptions() {
let app = findApp(this);

return (app && app.options) || {};
},

_parentName() {
let parentName;

Expand Down Expand Up @@ -158,6 +250,7 @@ module.exports = {
_getBabelOptions(config) {
let addonProvidedConfig = this._getAddonProvidedConfig(config);
let shouldCompileModules = this._shouldCompileModules(config);
let shouldIncludeHelpers = this._shouldIncludeHelpers();

let emberCLIBabelConfig = config['ember-cli-babel'];
let shouldRunPresetEnv = true;
Expand Down Expand Up @@ -188,6 +281,7 @@ module.exports = {
let userPostTransformPlugins = addonProvidedConfig.postTransformPlugins;

options.plugins = [].concat(
shouldIncludeHelpers && this._getHelpersPlugin(),
userPlugins,
this._getDebugMacroPlugins(config),
this._getEmberModulesAPIPolyfill(config),
Expand Down
34 changes: 34 additions & 0 deletions lib/default-should-include-helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const SHOULD_INCLUDE_HELPERS = new WeakMap();

function shouldIncludeHelpers(addonOrProject) {
// Currently we check for @ember-decorators transforms specifically, but we
// could check for any number of heuristics in this function. What we want is
// to default to on if we reasonably believe that users will incur massive
// cost for inlining helpers. Stage 2+ decorators are a very clear indicator
// that helpers should be included, at 12kb for the helpers, it pays for
// itself after usage in 5 files. With stage 1 decorators, it pays for itself
// after 25 files.
if (addonOrProject.pkg && addonOrProject.pkg.name === '@ember-decorators/babel-transforms') {
return true;
}

if (addonOrProject.addons) {
return addonOrProject.addons.some(shouldIncludeHelpers);
}

return false;
}


module.exports = function defaultShouldIncludeHelpers(project) {
if (SHOULD_INCLUDE_HELPERS.has(project)) {
return SHOULD_INCLUDE_HELPERS.get(project);
}

let shouldInclude = shouldIncludeHelpers(project);

SHOULD_INCLUDE_HELPERS.set(project, shouldInclude);

return shouldInclude;
}

14 changes: 14 additions & 0 deletions lib/find-app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

module.exports = function findApp(addon) {
let current = addon;
let app;

// Keep iterating upward until we don't have a grandparent.
// Has to do this grandparent check because at some point we hit the project.
do {
app = current.app || app;
} while (current.parent.parent && (current = current.parent));

return app;
}
1 change: 0 additions & 1 deletion lib/relative-module-paths.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-env node */
'use strict';

const path = require('path');
Expand Down
93 changes: 91 additions & 2 deletions node-tests/addon-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,19 @@ describe('ember-cli-babel', function() {

beforeEach(function() {
this.ui = new MockUI();
let project = { root: __dirname, emberCLIVersion: () => '2.16.2' };
let project = {
root: __dirname,
emberCLIVersion: () => '2.16.2',
addons: []
};

this.addon = new Addon({
project,
parent: project,
ui: this.ui,
});

project.addons.push(this.addon);
});

afterEach(function() {
Expand Down Expand Up @@ -283,12 +290,19 @@ describe('ember-cli-babel', function() {

describe('@ember/string detection', function() {
beforeEach(function() {
let project = { root: input.path(), emberCLIVersion: () => '2.16.2' };
let project = {
root: input.path(),
emberCLIVersion: () => '2.16.2',
addons: []
};

this.addon = new Addon({
project,
parent: project,
ui: this.ui,
});

project.addons.push(this.addon);
});

it('does not transpile the @ember/string imports when addon is present', co.wrap(function* () {
Expand Down Expand Up @@ -456,6 +470,81 @@ describe('ember-cli-babel', function() {
});
});

describe('_shouldIncludeHelpers()', function() {
beforeEach(function() {
this.addon.app = {
options: {}
};
});

it('should return false without any includeExternalHelpers option set', function() {
expect(this.addon._shouldIncludeHelpers()).to.be.false;
});

it('should throw an error with ember-cli-babel.includeExternalHelpers = true in parent', function() {
this.addon.parent.options = { 'ember-cli-babel': { includeExternalHelpers: true } };

expect(this.addon._shouldIncludeHelpers).to.throw;
});

it('should return true with ember-cli-babel.includeExternalHelpers = true in app and ember-cli-version is high enough', function() {
this.addon.pkg = { version: '7.3.0-beta.1' };

this.addon.app.options = { 'ember-cli-babel': { includeExternalHelpers: true } };

expect(this.addon._shouldIncludeHelpers()).to.be.true;
});

it('should return false with ember-cli-babel.includeExternalHelpers = true in app and write warn line if ember-cli-version is not high enough', function() {
this.addon.project.name = 'dummy';
this.addon.project.ui = {
writeWarnLine(message) {
expect(message).to.match(/dummy attempted to include external babel helpers/);
}
};

this.addon.app.options = { 'ember-cli-babel': { includeExternalHelpers: true } };

expect(this.addon._shouldIncludeHelpers()).to.be.false;
});

it('should return false with ember-cli-babel.includeExternalHelpers = false in host', function() {
this.addon.app.options = { 'ember-cli-babel': { includeExternalHelpers: false } };

expect(this.addon._shouldIncludeHelpers()).to.be.false;
});

describe('autodetection', function() {
it('should return true if @ember-decorators/babel-transforms exists and ember-cli-babel version is high enough', function() {
this.addon.pkg = { version: '7.3.0-beta.1' };
this.addon.project.addons.push({
pkg: {
name: '@ember-decorators/babel-transforms'
}
});

expect(this.addon._shouldIncludeHelpers()).to.be.true;
});

it('should return false if @ember-decorators/babel-transforms exists and write warn line if ember-cli-version is not high enough', function() {
this.addon.project.name = 'dummy';
this.addon.project.ui = {
writeWarnLine(message) {
expect(message).to.match(/dummy attempted to include external babel helpers/);
}
};

this.addon.project.addons.push({
pkg: {
name: '@ember-decorators/babel-transforms'
}
});

expect(this.addon._shouldIncludeHelpers()).to.be.false;
});
})
});

describe('_shouldCompileModules()', function() {
beforeEach(function() {
this.addon.parent = {
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
"dependencies": {
"@babel/core": "^7.0.0",
"@babel/plugin-transform-modules-amd": "^7.0.0",
"@babel/plugin-transform-runtime": "^7.2.0",
"@babel/polyfill": "^7.0.0",
"@babel/preset-env": "^7.0.0",
"@babel/runtime": "^7.2.0",
"amd-name-resolver": "^1.2.1",
"babel-plugin-debug-macros": "^0.2.0-beta.6",
"babel-plugin-ember-modules-api-polyfill": "^2.6.0",
Expand Down
Loading