react-native-svg icon indicating copy to clipboard operation
react-native-svg copied to clipboard

Bundle size css-tree

Open wilau2 opened this issue 5 years ago • 22 comments

The bundle size passed from 55kb to 173kb updating the library from 9.11.1 to 11.0.1.

Should css-tree be an optional dependency?

image

wilau2 avatar Jan 24 '20 21:01 wilau2

We were doing good without css-tree. #1166 https://github.com/react-native-community/react-native-svg/commit/957914d59b27e22121d13f13cb54a051b893b446

wilau2 avatar Jan 24 '20 21:01 wilau2

Feel free to make a pr to fix this

msand avatar Feb 29 '20 20:02 msand

No response, closing.

msand avatar Mar 08 '20 10:03 msand

@wilau2 How did You manage to get rid of it?

@msand Just checked, css-tree adds almost 100Kb of result React Native bundle which may result in startup time degradation/bigger bundles for OTA updates. I'm ready to work on PR, just need to make sure what will be the way to rectify it...

todorone avatar Apr 03 '20 07:04 todorone

I suspect the easiest way is to use patch-package to remove/comment out these lines of code such that metro doesn't attempt to bundle them:

https://github.com/react-native-community/react-native-svg/blob/e66e87a5b5c090509d5e2127237963f83e60f1e9/src/ReactNativeSVG.ts#L27-L34

https://github.com/react-native-community/react-native-svg/blob/e66e87a5b5c090509d5e2127237963f83e60f1e9/src/ReactNativeSVG.ts#L90-L97

Alternatively, to set up some replacement for css-select and css-tree, assuming you're not using the css functionality, you should be able to replace them with no-op functions. Alternatively, it might be possible to use random access modules, but that might require additional configuration, and would thus limit usability, haven't looked into it. At least I haven't found any way to get metro to work without attempting to load those two, even if the modules that use them aren't used anywhere. Maybe it's possible to configure somehow, or maybe haul/webpack supports it.

msand avatar Apr 03 '20 10:04 msand

@msand Thanks for help and quick response. Yeah, just commenting those lines with patch-package saved almost 250Kb of bundle, according to react-native-bundle-visualizer.

Bundle is 2.1 MB in size (--- has decreased with 246003 bytes since last run)

It's ridiculous, how much trash sometimes we bundle into our apps, and then claiming that react native is slow...😸

todorone avatar Apr 03 '20 11:04 todorone

Yeah, would be great to get proper tree-shaking at both development and in production builds. Would you mind looking into how to achieve this?

msand avatar Apr 03 '20 13:04 msand

@msand I'll try to investigate

todorone avatar Apr 03 '20 13:04 todorone

This might be relevant https://github.com/facebook/metro/pull/511

msand avatar Apr 03 '20 16:04 msand

https://github.com/msand/react-native-svg-e2e/pull/31

msand avatar Apr 03 '20 16:04 msand

Hey @msand Thanks for the links provided. Now metro-babel-preset allows conditional importing, but tree shaking is not implemented and not planned, unfortunately - https://github.com/facebook/metro/issues/227

~~The only solution for now is to migrate to monorepo and multiple packages like react-navigation did. We can have something like @react-native-svg/core and @react-native-svg/css. I know it will introduce a bit of overhead in maintaining, but it's the only viable solution for now.~~ Otherwise currently those couple components that were introduced recently take 246003 bytes, which is 2x more than the all the rest, which is ridiculous...😆

Wdyt?

UPDATE: Oh, totally forgot about bundling separate chunks in separate files. What do we need:

  • Move all exports of css.tsx out of index.ts
  • Bundle css.tsx in the root of module along with index.ts
  • Import modules related to css like import { SvgCss } from 'react-native-svg/css'
  • Voila 🎉🎉🎉

todorone avatar Apr 09 '20 18:04 todorone

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You may also mark this issue as a "discussion" and I will leave this open.

stale[bot] avatar Jun 09 '20 04:06 stale[bot]

@msand So the solution will be simple(and can be reused with any of big planned features like filters):

  • Remove all exports of css.tsx in index.ts
  • Bundle css.tsx in the root of module along with index.ts
  • Import modules related to css like import { SvgCss } from 'react-native-svg/css'
  • Voila, metro won't include any css code to the bundle 🎉🎉🎉

This is common practice used by huge libraries like Material UI, the only downside is this is a breaking change, but it's not a big deal if we will document it in release note like: import { SvgCss } from 'react-native-svg' => import { SvgCss } from 'react-native-svg/css'

If You give me a green flag, I'll prepare PR...

todorone avatar Jun 09 '20 05:06 todorone

@todorone Sounds great. I haven't had time to work on open-source lately, will probably be able to again, starting july or something. Might be good to have a release in between, with a deprecation warning. At least nicer for users if the latest major version, before the breaking change, warns about upcoming api changes and ideally has a codemod available.

msand avatar Jun 09 '20 07:06 msand

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You may also mark this issue as a "discussion" and I will leave this open.

stale[bot] avatar Aug 08 '20 08:08 stale[bot]

Thanks for ping, stale bot, i'll try to prepare PR tomorrow...

todorone avatar Aug 08 '20 08:08 todorone

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You may also mark this issue as a "discussion" and I will leave this open.

stale[bot] avatar Oct 10 '20 03:10 stale[bot]

A PR is open mister stale bot !

wilau2 avatar Oct 10 '20 12:10 wilau2

tree shaking support https://github.com/facebook/metro/issues/632

chang-ke avatar Feb 05 '21 09:02 chang-ke

@wilau2 Can we close that issue?

bohdanprog avatar Aug 05 '24 11:08 bohdanprog

Feel free to close in not working with react native anymore

wilau2 avatar Aug 05 '24 11:08 wilau2

@wilau2, has that issue been resolved?

bohdanprog avatar Aug 05 '24 11:08 bohdanprog