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

Reduce Build Size #1528

Open
james-rae opened this issue Dec 6, 2022 · 7 comments
Open

Reduce Build Size #1528

james-rae opened this issue Dec 6, 2022 · 7 comments
Labels
flavour: build Changes to the build process priority: nice Would be excellent if this got done type: preventative Reduce complexity, improve quality. Preventative Management Bucket

Comments

@james-rae
Copy link
Member

Investigate ways to reduce the size of the build. It is currently around 10MB. Part of this size difference (in comparison to RAMP2) is that we are bundling the ESRI API instead of having the app dynamically download it at startup.

  • Minification. Are we doing it now? Some things look minified, others are not.
  • Tree shaking. Are we doing it now? Lets ensure we are not bundling modules/libs that are not actually being used.
  • Other grand ideas. Comment them, try them.
@james-rae james-rae added flavour: build Changes to the build process priority: nice Would be excellent if this got done type: preventative Reduce complexity, improve quality. Preventative Management Bucket labels Dec 6, 2022
@mohsin-r
Copy link
Member

mohsin-r commented Dec 7, 2022

I have some findings.

Minification. Are we doing it now? Some things look minified, others are not.

Minification is occurring. Vite uses esbuild as a default for minification. The reason that some things do not look minified is because esbuild has some limitations for JS code. See below:

The minification algorithm in esbuild does not yet do advanced code optimizations. In particular, the following code optimizations are possible for JavaScript code but are not done by esbuild (not an exhaustive list):

  • Dead-code elimination within function bodies
  • Function inlining
  • Cross-statement constant propagation
  • Object shape modeling
  • Allocation sinking
  • Method devirtualization
  • Symbolic execution
  • JSX expression hoisting
  • TypeScript enum detection and inlining

Now vite also supports using terser for minification (see here for options). I tried this out with the best config options possible. It resulted in ramp.esm.js going from 14.4MB to 15.7MB and ramp.js going from 10.3MB to 9.9MB. Additionally, the build took forever. So this option sucks.

Tree shaking. Are we doing it now? Lets ensure we are not bundling modules/libs that are not actually being used.

This is also being done via the underlying Rollup bundle. I attempted to change the treeshake option in rollupOptions to 'smallest' so that we can get as much treeshake as possible. This resulted ramp.esm.js decreasing in size by 7KB and ramp.js decreasing in size by 4KB. So not a very significant difference.

Other grand ideas

I tried finding vite plugins that people have made that would help with minification. I found this one and this one but they are for HTML and multi-page apps i.e. not our use case.

TLDR: We have minification and tree shaking in place already. The ways to enhance things I have found so far do not make a huge difference.

Will do some more digging and post here if I find anything meaningful.

@james-rae
Copy link
Member Author

Another thing we used to have (pre-vite, I believe), was the build was "chunked". The output had many, many files, and the site would download things as needed on demand.

Overall would be roughly the same amount downloaded, but you did get a faster startup (there has been a noticeable delay in page startup ever since we moved to vite as that mammoth file downloads). Also you got small gains if a user never accessed certain functions; the code would not get downloaded.

Another thing to possibly explore.

@james-rae
Copy link
Member Author

james-rae commented Feb 27, 2024

Looking at some stuff from @milespetrov build report.

ag-grid

We're importing ag-grid-community.cjs.js, which is 2.3MB. Can we target the cjs.min.js version instead? It's half the size in the current version.

fabric

Similar for fabric, the min.js version is 1/3 of the size of the lib we're currently pulling (1MB). We only use this to generate PNGs in the export module.

I'm assuming this library is required to convert the SVG stuff to images without much hassle. If someone wants to look into using OffscreenCanvas as an alternative, could drop the entire lib. Would need to ensure the SVG stuff works good.

esri

This is the biggest lib in the project. It is also the backbone lib (aside from Vue). That said, the modules appearing after "tree shaking" suggest the tree isn't shook very well. We import specific modules, so would expect things we are not using to be dropped.

E.g. 250k for 3D support, 230k for Widgets, 440k for Arcade support (tho maybe we want this later), and who knows how many internal things. It even looks like there is a 1MB index.js file being pulled in by a charting widget, which we don't use.

Worth looking into how our build prunes things not used? Maybe ESRI's internal build has references (i.e. you load a basic module and it pulls in other stuff JUST INCASE) and we're stuck.

@milespetrov
Copy link
Member

For ag-grid we certainly want to be targeting ag-grid-community.esm.js and not commonjs. Same for esri, not sure if we're doing that but a quick search shows esri supports esm.

@james-rae
Copy link
Member Author

For ESRI, we're grabbing them from npm and as I understand only ES modules are on npm.

That said, everything gets pulled to node_modules, which isn't that surprising. But it looks like either the build isn't culling stuff, or it can't. I could maybe start crawling through the files in node_modules/@arcgis using the vite graph as a guide, see if certain files have "must import" type links all the way up to modules we require. I would hope not, since they claim the AMD will only download needed things.

@milespetrov
Copy link
Member

I noticed a fairly recent change with vite/rollup that enables treeshaking for deterministic dynamic imports like:

const { foo } = await import('foo')
(await import('foo')).foo
import('bar').then(({ foo }) => {})

Vite: vitejs/vite#11080
Rollup: rollup/rollup#4951

I don't think we have a lot of dynamic imports except for two places that use import.meta.glob

const fixtureModules = import.meta.glob<{ default: typeof FixtureInstance }>(

const screenModules = import.meta.glob<Component>('../fixtures/*/screen.vue');

These modules would not be treeshaken. If possible update them with named imports and eager loading to make them deterministic, something like:

const modules = import.meta.glob('./dir/*.js', {
  import: 'default',
  eager: true,
})

I'll also brain dump two things I read somewhere (can't find them now) before I forget:

  1. default imports should be avoided like import aLib from someLib since the static analysis for treeshaking isn't the most reliable, especially as the code gets more complex. If you need to do a default import of a big library maybe try re-exporting only the things you need in the same module so static analysis is more reliable.
  2. Classes are not treeshakable.

@szczz szczz added Meeting Topic Topic of discussion for the next RAMP meeting and removed Meeting Topic Topic of discussion for the next RAMP meeting labels Apr 5, 2024
@szczz
Copy link
Member

szczz commented Apr 9, 2024

We need the production map to determine where to focus our efforts.

We should also proceed with testing out the dynamic imports.

Experiment going back to umd build files. May improve loading times but won't help the build size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flavour: build Changes to the build process priority: nice Would be excellent if this got done type: preventative Reduce complexity, improve quality. Preventative Management Bucket
Projects
None yet
Development

No branches or pull requests

4 participants