nfl / react-helmet

A document head manager for React
MIT License
17.3k stars 659 forks source link

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

Open petermikitsh opened 5 years ago

petermikitsh commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@petermikitsh nice, why not pr this anyway?

petermikitsh commented 5 years ago

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

AubreyHewes commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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.