-
Notifications
You must be signed in to change notification settings - Fork 637
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
Tree shaking while bundling #227
Comments
Metro currently does not support tree shaking based on module import/export. @mjesun is currently adding first-class support for Metro to understand them and this will eventually allow us to do tree shaking. Stayed tuned! 😃 |
any outcome on tree-shaking ? Looking forward to that. |
@rafeca Now that we have experimental support for import/export inside metro, is tree shaking now available out of the box or is there work going on to support it? Anything that we can help with to make this possible? |
@karanjthakkar : we're planning to implement some sort of tree shaking (or better said, minification of code based on the es6 module information) during H1 2019. Based on our internal tests, the most impactful minification in terms of bundle size that can be done is to minify the import/export names, so for example: // bar.js
export const ANY_VARIABLE_NAME = 'value'; // foo.js
import {ANY_VARIABLE_NAME} from './bar';
console.log(ANY_VARIABLE_NAME); would get minified to: // bar.js
exports.a = 'value'; // foo.js
console.log(r('./bar').a); Afterwards, we're going to be able to do dead code elimination based on the exported values that are not being used anywhere |
Any movement or resolution on this? |
Is this still slated for H1 2019? |
Is it supported yet? |
Hi ! Any news ? |
Seems that's rather late |
Its february 2020, brexit happened but not this. |
@rafeca and @mjesun have since left Facebook. We are not actively working on tree shaking at Facebook and are exploring other more promising ways to keep our app size small. If you are using Metro for React Native, it is unlikely that tree shaking will have a major impact on your applications size or behavior. If you are looking for size improvements, consider compressing the JS bundle that you ship with your app. If you are looking for performance improvements, there are likely many other things (like shipping Hermes and bytecode) that will have a large impact. I'll close this as wontfix now but we will accept contributions towards tree shaking – and I'm accepting your job applications if you wanna work on this kinda stuff at Facebook London :) |
I think what is repak:
|
It's disappointing that Facebook's position is essentially, "bundling the entire import probably won't that much bloat, or add that much performance degradation to your app". True (maybe), but why not just... not add things that aren't needed? Tree-shaking is an expected feature of any JS bundler, and not having tree shaking violates the Law of Least Surprise. If we use 1 function in, say, a giant hooks library with 1,000 hooks, we expect only that 1 function to show up in our app. If Metro can't provide this, then IMO Metro should not be in the business of bundling and should pair with Vite, Webpack, or other such bundlers to find a solution for React Native devs. |
Totally agree. The smaller bundle can still be valid even though we performed compression and backed it with Hermes. Currently, we are sending the bundle over HTTP with brotli compression and the underlying engine is Hermes for better performance but that doesn't rule out the use case for tree shaking. Also, I believe folks here undermine the improvement one can get with tree shaking. When I raised this issue, Haul (predecessor of repack) solved this issue by using webpack. At that time we saw an improvement of around ~49.64% of bundle size.
We couldn't go to production with Haul because of a lack of community support and also the bundle it spits breaks our internal system. Our internal diffing logic would have needed overall changes. Also, using scripts to change haste-dependent libraries dynamically seems kind of a hack and is a bit dangerous to send in production. But now I think we have multiple solutions out there:
Since these tools are now available, it is bound that users with advanced bundling needs will move away from Metro. |
btw for those of you using Metro and wanting tree shaking, over at MSFT we have developed a plugin for Metro to basically rely on esbuild for treeshaking: https://github.com/microsoft/rnx-kit/tree/main/packages/metro-serializer-esbuild (guide here: https://microsoft.github.io/rnx-kit/docs/guides/bundling#tree-shaking) We already use it in prod on a couple projects and saw good bundle size improvements |
Too bad... Ignoring real cases where Tree Shaking is necessary |
Facebook went ahead and wrote their own JavaScript engine (hermes) to try to optimize startup behavior and in my own testing simply having less JavaScript goes a long way in improving startup performance... and dare I say is a lot simpler than writing your own JS engine from scratch 🤯 |
Went ahead and wrote a version of tree shaking in Expo CLI to get the ball rolling. It can be added to any React Native project by adopting The implementation is landed on main and it'll ship experimentally in SDK 52 (October 2024). Here are the docs: Tree Shaking in Expo CLI |
Any way to remove .git folder from bundle? Is there any reason why metro do that? |
Is it possible anyhow to implement tree shaking during bundling.
Suppose I have a file say
greetings.js
:Now if in my project I am only using
sayHi
then I wantsayBye
to be removed from the final bundle which is currently not the case.Is it possible anyhow to implement this (even if I had to do changes in the config file for the metro bundler).
The text was updated successfully, but these errors were encountered: