Telegram-Mini-Apps / TelegramUI

React components library for Telegram Mini Apps inspired by Telegram interface
https://ton.org/mini-apps
MIT License
59 stars 5 forks source link

Unnecessary code gets bundled when adding a component #3

Closed kubk closed 2 months ago

kubk commented 2 months ago

Hi. The library has a lot of working components, which is great because users can already start integrating them into their projects. The issue I noticed is that even importing one component results in a lot of unnecessary bundled code, causing the bundle to bloat.

Here is my code example:

<AppRoot>
  <SegmentedControl>
    {labels.map(({ value, label }) => (
      <SegmentedControl.Item
        key={value}
        selected={selected === value}
        onClick={() => setSelected(value)}
      >
        {label}
      </SegmentedControl.Item>
    ))}
  </SegmentedControl>
</AppRoot>;

It produces the following report generated by the vite-bundle-visualizer:

Screenshot 2024-03-11 at 09 47 14

Here we see a lot of tgui components (all the components I guess), Radix UI, a library to remove scroll. Reproduction repo: https://github.com/kubk/tgui-bundle-size-issue

Why tree-shaking is important Users may be reluctant to use the library because importing even one component results in loading a lot of unnecessary code.

mainsmirnov commented 2 months ago

It seems like your Vite configuration might not be set up correctly. Our library supports tree shaking, and its bundle size is quite minimal. You can verify its size on BundlePhobia.

Additionally, you can test tree shaking with an appropriate build tool, such as react-scripts, which you can find here: react-scripts on npm.

Alternatively, consider using our template available at XeleneStudio TGUI-Example on GitHub

kubk commented 2 months ago

@mainsmirnov It's default Vite configuration. You can recreate it via npm create vite@latest my-react-app -- --template react-ts and then add tgui.

vite has 11.7 millions downloads weekly: https://www.npmjs.com/package/vite react-scripts has 3.2 millions: https://www.npmjs.com/package/react-scripts It's even doubtful whether react-scripts is still maintained: https://github.com/facebook/create-react-app/issues/13503

Vite works fine with code splitting out of the box, the code splitting works for my telegram mini app and other dependencies, except tgui. I'd consider this worth looking from the tgui side, if the library doesn't want to miss plethora of vite users.

kubk commented 2 months ago

Not sure if tgui is going to be included in the Telegram Mini App template that is being built by @heyqbnk, but that template is also based on Vite: https://github.com/Telegram-Mini-Apps/reactjs-template.

heyqbnk commented 2 months ago

Hey! Yeah, I have plans on adding this package to the templates based on React. I tried to use it several days ago, but met some problems, and decided to postpone the idea of integrating it. Moreover, it seems like the package is hardly bound to the Telegram SDK and I am not sure if it can be used along with @tma.js.

Thanks for your mention, because I forgot to check the bundle size with this package included. React JS template already uses @tonconnect/ui-react (~300kb) and react-router-dom (~200kb). Adding 1 more huge library will just blow my mind. So, we are not going not add this package until its size is optimal and problems are solved.

Waiting for grant to be completed and coming back to see the results. I am currently developing the separate package working with Telegram SDK and @tma.js using Solid.

heyqbnk commented 2 months ago

Due to the reason, that bundlephobia is currrently down, I have tried bundlejs.com to check the bundle size.

image

As you can see here, using the only Button component costs 226 kb.

Not sure that config could be the problem, because we use default config that already includes tree shaking and works fine with other package

mainsmirnov commented 2 months ago

We adopted the build configurations from the library. If the issue also exists there, it might be necessary to adjust your setup.

My attempt to replicate the issue with webpack was unsuccessful. Checking the library's bundle reveals that files are individually compiled, ensuring full tree-shaking support.

We will take a closer look look at it, thank you for your notes and adjustments

heyqbnk commented 2 months ago

I have tried this code locally:

import { Typography } from '@xelene/tgui';

console.log(Typography);

After building the code using Vite (which uses Rollup, minifying the code via terser), I receive 204kb bundle size. In case, I import only React, the bundle size becomes 142kb.

I have tried this code to see if we pull whole package importing the only Typography component:

import * as tgui from '@xelene/tgui';

console.log(tgui);

Bundle size is 295kb. So, we don't pull the whole package importing the only one component, but half of it.

Then, I tried to import this component using this way:

import { Typography } from '@xelene/tgui/dist/components/Typography';

console.log(Typography);

Bundle size is now 10kb.

So, I assume, that package contains side-effects which can't be tree-shaken. Importing any component from the root will lead to importing all side-effects. That's why importing Typography not from the root costs way less bundle size.

Let me know if I am missing something.

mainsmirnov commented 2 months ago

Thank you for providing the detailed repository with a demonstration. We have resolved the issue in version 2.0.4

Screenshot 2024-03-13 at 12 52 30 PM
kubk commented 2 months ago

@mainsmirnov Thank you! Can confirm it fixed the issue 👍