-
Notifications
You must be signed in to change notification settings - Fork 659
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
Comments
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. |
Thanks for following up @jamsea. If @cwelch5 is onboard, I will open a PR. Also, I want to add 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'
}
}
} |
@petermikitsh nice, why not pr this anyway? |
I'm happy to make a PR if maintainers approve of this change. So far, I haven't heard from an active maintainer. |
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? |
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. |
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. |
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. |
@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 |
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. |
It looks like this was addressed in #395 and released in 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. |
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):
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: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.
The text was updated successfully, but these errors were encountered: