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

Upgrade to Babel 7 and use @babel/plugin-transform-runtime to reduce bundle size #403

Open
petermikitsh opened this issue Sep 29, 2018 · 11 comments

Comments

@petermikitsh
Copy link
Contributor

If you look at the compiled source for react-helmet (e.g., https://unpkg.com/react-helmet@5.2.0/es/Helmet.js), you'll see stuff like this (line 1):

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; };

This is a babel helper function. It's inlined, but it doesn't have to be. If you add @babel/runtime as a dependency to this project, and run babel with the @babel/plugin-transform-runtime plugin, you'd get output like this:

var _extends11 = _interopRequireDefault(require("@babel/runtime/helpers/extends"));

By referencing the helper from @babel/runtime, every time it gets imported in a bundle, you only end up with a single copy (instead of a potentially infinite number of inlined helper functions). It's an optimization that helps keep bundle sizes in check.

@jamsea I saw you were the last to merge to master. Mentioning for visibility. If you would accept a PR for this issue, please let me know, and I would contribute it.

To be clear -- there are no breaking changes suggested here, and this could go out as a patch release.

@jamsea
Copy link
Contributor

jamsea commented Sep 29, 2018

Nice, thanks @petermikitsh but I’m not part of the org anymore. @cwelch5 this is a good one to merge! Maybe bug Robbie or Michael to take a look at it on Monday.

@petermikitsh
Copy link
Contributor Author

Thanks for following up @jamsea. If @cwelch5 is onboard, I will open a PR. Also, I want to add "module": "es/Helmet.js" to package.json so bundlers (webpack, rollup, etc) automatically use the ES Module build without any configuration.

Currently, in my webpack config, I have to alias to the module build so it is used, e.g.:

{
  resolve: {
    alias: {
      'react-helmet': 'react-helmet/es/Helmet.js'
    }
  }
}

@AubreyHewes
Copy link
Contributor

@petermikitsh nice, why not pr this anyway?

@petermikitsh
Copy link
Contributor Author

I'm happy to make a PR if maintainers approve of this change. So far, I haven't heard from an active maintainer.

@AubreyHewes
Copy link
Contributor

Yeah I understand, but creating the PR and mentioning the maintainers is more help to keep the project moving? The PR is what maintainers could eventually approve. I.e. a working example?

@petermikitsh
Copy link
Contributor Author

If you'd like to implement these changes (without knowing they'll be merged), I'd welcome you to do so.

It's not written in stone, but most maintainers prefer discussion of a change prior to starting the work. If the maintainer doesn't want it, then it's a waste of everyone's time.

@AubreyHewes
Copy link
Contributor

AubreyHewes commented Oct 3, 2018

You have already spent the time.. the PR is the discussion.. it is how it works. If they dont't want it PR rejected.

If you do not want to share your work so be it.

@petermikitsh
Copy link
Contributor Author

There must be a misunderstanding here. I haven't made any changes. I don't have a branch that addresses what is described in this issue.

@AubreyHewes
Copy link
Contributor

@petermikitsh I apologize I misunderstood your meaning. When you said "I will open a PR". Still a PR of your proposed change is always appreciated and the more the merrier #391

@tmbtech
Copy link
Contributor

tmbtech commented Oct 31, 2018

Hi All, i'm also interested in added in the same feature. If you post a PR, i'll work with @cwelch5 to get it merged in.

@petermikitsh
Copy link
Contributor Author

It looks like this was addressed in #395 and released in 6.0.0-beta.

The new ES Module build of Helmet.js (https://unpkg.com/react-helmet@6.0.0-beta/es/Helmet.js) has no inlined babel helpers. It's 35185 bytes.

According to Bundlephobia, the minified, gzipped size went from 6.4 kB to 5.9 kB.

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

4 participants