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

Reduce standalone build size #10668

Merged
merged 4 commits into from Nov 21, 2019

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Nov 6, 2019

Q                       A
License MIT

In this PR we build babel-standalone against the src version of @babel/* dependencies instead of lib, which is specified in their package.json. By doing so the transform-runtime can replace babel helper calls by runtime helper imports. It is impossible to achieve this on lib version where the helpers are inlined.

Here is a brief summary on the number of inlined function _interopRequireDefault(obj) declarations in the artifacts. (produced by this script, lower is better)

7.7.1 This branch
@babel/standalone 151 4
@babel/preset-env-standalone 101 0

Note that there are still 4 inlined _interopRequireDefault declaration not removed in @babel/standalone, all of which comes from third party dependencies, i.e. regenerator-runtime.

@JLHwung JLHwung added the PR: Internal 🏠 A type of pull request used for our changelog categories label Nov 6, 2019
@buildsize
Copy link

buildsize bot commented Nov 6, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.78 MB 2.81 MB 29.12 KB (1%)
babel-preset-env.min.js 1.67 MB 1.63 MB -46.82 KB (3%)
babel.js 2.96 MB 2.91 MB -49.49 KB (2%)
babel.min.js 1.63 MB 1.56 MB -74.19 KB (4%)

@JLHwung
Copy link
Contributor Author

JLHwung commented Nov 6, 2019

The size change of babel-preset-env.js comes from forceAllTransforms: true. I have removed forceAllTransform: true because standalone should build with default env targets.

The size change of babel-preset-env.js comes from bloated webpack injected comments like // CONCATENATED MODULE blablabla. The minified size change does support that we are shipping less code than the control branch.

@@ -3,6 +3,7 @@
"version": "7.7.2",
"description": "Babel compiler core.",
"main": "lib/index.js",
"_main:used-by-babel-standalone": "src/index.js",
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this 😆

Is it possible to use webpack's resolve instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By using resolve.alias like the other packages, webpack will ignore the browsers field in package.json.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I follow. If we did something like this, wouldn't it work?

resolve: {
  alias: {
    "@babel/core$": "packages/babel-core/src/index.js"
  }
}

const babelRuntimePackageJSONPath = require.resolve(
"@babel/runtime/package.json"
);
const path = require("path");
Copy link

Choose a reason for hiding this comment

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

It's a simple question. What is the advantage of declaring within this scope rather than at the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a strong reason: require("path") is used only when building babel-*-standalone. However, this config is shared with other building process (build/jest), by doing so we could avoid the overhead of requiring a builtin module.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants