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

[Docs] Tree-shaking benchmark of RN app #581

Open
jbroma opened this issue Apr 24, 2024 · 19 comments
Open

[Docs] Tree-shaking benchmark of RN app #581

jbroma opened this issue Apr 24, 2024 · 19 comments
Labels
help wanted especially welcoming for external contributions pinned Keeps this issue or PR active and prevents it from going stale. type:documentation Improvements or additions to Re.Pack documentation
Milestone

Comments

@jbroma
Copy link
Member

jbroma commented Apr 24, 2024

Summary

Add page to docs that will showcase potential gains obtained through the use of tree-shaking.

We could also add few tips how to optimize the codebase to squeeze even more out of tree-shaking, also backed with data.

@jbroma jbroma added type:documentation Improvements or additions to Re.Pack documentation help wanted especially welcoming for external contributions labels Apr 24, 2024
@thymikee
Copy link
Member

cc @matthargett

Copy link

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

@github-actions github-actions bot added the Stale label May 25, 2024
@jbroma jbroma removed the Stale label May 26, 2024
Copy link

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 26, 2024
@matthargett
Copy link

I'd still really like to see this, especially to track new rspack versus old webpack backend as time goes on.

@github-actions github-actions bot removed the Stale label Jul 26, 2024
Copy link

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 25, 2024
@jbroma jbroma removed the Stale label Aug 25, 2024
@matthargett
Copy link

@matthargett
Copy link

another optimization scenario that affects React hot path: https://gitlab.com/seaofvoices/darklua/-/issues/44

Copy link

github-actions bot commented Oct 6, 2024

This issue has been marked as stale because it has been inactive for 30 days. Please update this issue or it will be automatically closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 6, 2024
Copy link

This issue has been automatically closed because it has been inactive for more than 14 days. Please reopen if you want to add more context.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2024
@jbroma jbroma removed the Stale label Nov 1, 2024
@jbroma jbroma reopened this Nov 1, 2024
@jbroma jbroma added the pinned Keeps this issue or PR active and prevents it from going stale. label Nov 1, 2024
@hosseinmd
Copy link
Contributor

Tree shaking doesn't work in module federation, is there any solution?

@jbroma
Copy link
Member Author

jbroma commented Jan 28, 2025

Tree shaking doesn't work in module federation, is there any solution?

if you declare module as shared then treeshaking of that module is disabled - its not possible to determine what to treeshake then since a remote might be using parts of a shared dependency that the host is not using

@hosseinmd
Copy link
Contributor

hosseinmd commented Jan 29, 2025

You are right, but modules are not shared doesn't tree shaking too, i found the reason of it.
we are using babel plugin of react-native which transforming es module import to commonjs require
this will help

                     [
                        "module:@react-native/babel-preset",
                        {
                            disableImportExportTransform: true,
                        },
                    ],

@hosseinmd
Copy link
Contributor

tree shaking just working with import/export to enable it we should not transform them

@jbroma
Copy link
Member Author

jbroma commented Jan 29, 2025

tree shaking just working with import/export to enable it we should not transform them

you're absolutely correct here - RN babel contains a plugin that converts every module transformed to CJS and CJS modules are very hard to treeshake 👍

@matthargett
Copy link

Dead code elimination is not just about eliminating modules that aren't imported/required. In the darklua examples above, major hot paths in the Rect reconciler can be optimized with a combination of inlining, const propagation, and compile-time eval. Most of the branches checking values in *FeatureeFlags.js files can be eliminated.

Webpack has plugins for const propagation and compile-time boolean evaluation, and if Rspack doesn't also have this capability, then I'd expect Rspack/Repack to perform significantly worse versus a well-tuned Webpack config.

@jbroma
Copy link
Member Author

jbroma commented Jan 30, 2025

@matthargett @hosseinmd did some direct comparison between Metro and Re.Pack here, your feedback would be greatly appreciated:

https://github.com/jbroma/rn-treeshake-benchmark

there is no in depth analysis or a case study based on that but it's a start :)

@matthargett
Copy link

Thank you! Looks like there is work to do since RePack is currently 3MB larger on the production bundle than Metro. I was more interested in the difference with the Webpack-based RePack before transitioning to Rspack, as we know Haul roundly beat Metro in terms of bundle size in 2019.

So, now I'm curious what the roadmap is to claw back that 3MB that Rspack has regressed. Will the final non-RC release be held until this gap is closed? Or will the 5.x approach with Rspack be reverted? Or was the plan to support the 4.x line until 5.x doesn't have regressions for end-users of Repack applications?

@jbroma
Copy link
Member Author

jbroma commented Feb 3, 2025

Thank you! Looks like there is work to do since RePack is currently 3MB larger on the production bundle than Metro. I was more interested in the difference with the Webpack-based RePack before transitioning to Rspack, as we know Haul roundly beat Metro in terms of bundle size in 2019.

It's not a regression, its expected with Rspack:

Image

Reference: https://rspack.dev/guide/optimization/tree-shaking#tree-shaking

@jbroma jbroma added this to the Re.Pack 5 milestone Feb 4, 2025
@matthargett
Copy link

I meant it's a regression versus Repack backed by webpack, which has a great reputation for optimizing bundle size better than Metro ever has. I might suggest working upstream in Rspack to try and get parity with the webpack-backed Repack before releasing this major version. If the intention is to maintain the webpack-based branch for a while, until the Rspack-based new version is competitive, I guess that's okay.

It just seems like a complication of how to message it to users and the broader community, again given that Haul/Repack's consistent differentiator for 7 years was around bundle optimizations that makes users happy due to faster app launch times and OTA downloads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted especially welcoming for external contributions pinned Keeps this issue or PR active and prevents it from going stale. type:documentation Improvements or additions to Re.Pack documentation
Projects
None yet
Development

No branches or pull requests

4 participants