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 package size #110

Open
GreenImp opened this issue Apr 27, 2020 · 21 comments · Fixed by #126
Open

Reduce package size #110

GreenImp opened this issue Apr 27, 2020 · 21 comments · Fixed by #126
Assignees
Labels
dependencies Pull requests that update a dependency file enhancement help wanted testing
Milestone

Comments

@GreenImp
Copy link
Collaborator

GreenImp commented Apr 27, 2020

Is your feature request related to a problem? Please describe.
The NPM bundle size is higher than I'd like and I think there's a lot of superfluous files in there, such as the .travis.yml file, the demo files etc.
These files are handy for working on the library, but not needed for working with the library.

Describe the solution you'd like
We should use either an .npmignore file to exclude files we don't want in the package, or add a whitelist of files to include in the package.json file.

It may also be worth completely removing the demo files as everything is explained in detail in the readme.

There's also an argument that the source files and tests should be excluded, but I'm uncertain what's best here.

Updated table of package sizes as calculated by various things:

v4.5.1 v5.4.1 Latest
Latest version
Next
Next version
NPM tarball package size: 329.1 kB
unpacked size: 1.7 MB
package size: 107.0 kB
unpacked size: 580.4 kB
package size: 108.1 kB
unpacked size: 588.6 kB
Bundlephobia Bundlephobia minified 4.5.1
Bundlephobia minified + gzip 4.5.1
Bundlephobia minified 5.4.1
Bundlephobia minified + gzip 5.4.1
Bundlephobia minified Latest
Bundlephobia minified + gzip Latest
Bundlephobia minified Next
Bundlephobia minified + gzip Next
Packagephobia Packagephobia install size 4.5.1 Packagephobia install size Packagephobia install size Packagephobia install size
@GreenImp
Copy link
Collaborator Author

GreenImp commented Apr 27, 2020

The package is currently 1.4MB
The biggest part of the package is the compiled JS, which is compounded by the fact that we've got both ESM and UMD versions as they're ~507kb and ~561kb respectively.

I think this is likely because of the math library, but replacing that could be tricky as we would need to find a library with a similar range of abilities. Although, it looks like the main mathjs library now supports ES modules, so we could just bring in the bits we need: https://mathjs.org/docs/getting_started.html#es-modules

npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 142B    .babelrc                                       
npm notice 240B    .editorconfig                                  
npm notice 87B     .eslintrc                                      
npm notice 6.0kB   demo/index.html                                
npm notice 5.9kB   demo/index.umd.html                            
npm notice 507.1kB lib/esm/bundle.js                              
npm notice 561.4kB lib/umd/bundle.js                              
npm notice 2.2kB   src/ComparePoint.js                            
npm notice 9.0kB   tests/ComparePoint.test.js                     
npm notice 1.3kB   src/modifiers/ComparisonModifier.js            
npm notice 5.2kB   tests/modifiers/ComparisonModifier.test.js     
npm notice 708B    src/modifiers/CriticalFailureModifier.js       
npm notice 7.4kB   tests/modifiers/CriticalFailureModifier.test.js
npm notice 773B    src/modifiers/CriticalSuccessModifier.js       
npm notice 7.4kB   tests/modifiers/CriticalSuccessModifier.test.js
npm notice 204B    src/Dice.js                                    
npm notice 8.8kB   src/DiceRoll.js                                
npm notice 19.2kB  tests/DiceRoll.test.js                         
npm notice 5.2kB   src/DiceRoller.js                              
npm notice 9.8kB   tests/DiceRoller.test.js                       
npm notice 536B    src/modifiers/DropModifier.js                  
npm notice 11.4kB  tests/modifiers/DropModifier.test.js           
npm notice 3.0kB   src/modifiers/ExplodeModifier.js               
npm notice 16.1kB  tests/modifiers/ExplodeModifier.test.js        
npm notice 234B    src/parser/foo.js                              
npm notice 1.5kB   src/dice/FudgeDice.js                          
npm notice 11.4kB  tests/dice/FudgeDice.test.js                   
npm notice 632B    src/parser/grammars/generate.js                
npm notice 364B    src/index.js                                   
npm notice 2.9kB   src/modifiers/KeepModifier.js                  
npm notice 11.4kB  tests/modifiers/KeepModifier.test.js           
npm notice 990B    src/modifiers/Modifier.js                      
npm notice 775B    src/Modifiers.js                               
npm notice 743B    src/parser/Parser.js                           
npm notice 51.7kB  tests/parser/Parser.test.js                    
npm notice 465B    src/dice/PercentileDice.js                     
npm notice 9.1kB   tests/dice/PercentileDice.test.js              
npm notice 2.3kB   src/modifiers/ReRollModifier.js                
npm notice 8.9kB   tests/modifiers/ReRollModifier.test.js         
npm notice 1.2kB   src/RollGroup.js                               
npm notice 8.0kB   tests/rolling.test.js                          
npm notice 5.2kB   src/results/RollResult.js                      
npm notice 16.4kB  tests/results/RollResult.test.js               
npm notice 1.7kB   src/results/RollResults.js                     
npm notice 6.4kB   tests/results/RollResults.test.js              
npm notice 995B    rollup.config.js                               
npm notice 1.4kB   src/modifiers/SortingModifier.js               
npm notice 5.5kB   tests/modifiers/SortingModifier.test.js        
npm notice 4.8kB   src/dice/StandardDice.js                       
npm notice 9.4kB   tests/dice/StandardDice.test.js                
npm notice 3.9kB   src/modifiers/TargetModifier.js                
npm notice 9.9kB   tests/modifiers/TargetModifier.test.js         
npm notice 4.4kB   src/utilities/utils.js                         
npm notice 25B     lib/umd/package.json                           
npm notice 2.2kB   package.json                                   
npm notice 835B    .github/ISSUE_TEMPLATE/bug_report.md           
npm notice 603B    .github/ISSUE_TEMPLATE/feature-request.md      
npm notice 34.2kB  readme.md                                      
npm notice 4.3kB   src/parser/grammars/grammar.pegjs              
npm notice 150B    banner.txt                                     
npm notice 1.1kB   licence.txt                                    
npm notice 98B     .travis.yml                                    
npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       4.0.3                                   
npm notice package size:  252.1 kB                                
npm notice unpacked size: 1.4 MB                                  
npm notice shasum:        1b1df9da06230ed09c8b0643342271a074d1f4e9
npm notice integrity:     sha512-jtAapqgywB3R/[...]fbH3XfJtjNKEg==
npm notice total files:   63

Even if I remove all of the other files that aren't necessary for use, it comes in at 1.1MB:

npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 507.1kB lib/esm/bundle.js   
npm notice 561.4kB lib/umd/bundle.js   
npm notice 25B     lib/umd/package.json
npm notice 2.2kB   package.json        
npm notice 34.2kB  readme.md           
npm notice 1.1kB   licence.txt         
npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       4.0.3                                   
npm notice package size:  208.1 kB                                
npm notice unpacked size: 1.1 MB                                  
npm notice shasum:        764963ffb2f33cdc149d7614b30f2ede34779cbd
npm notice integrity:     sha512-M+lr8cq4TEcii[...]jwhtYgFzHTC6w==
npm notice total files:   6

If we publish the minified JS instead, this does drastically reduce the package size to only 343.2KB, so maybe that's the best option. I'm not sure what the general view is on only supplying minimised code though.

npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 148.8kB lib/esm/bundle.min.js
npm notice 156.9kB lib/umd/bundle.min.js
npm notice 25B     lib/umd/package.json 
npm notice 2.2kB   package.json         
npm notice 34.2kB  readme.md            
npm notice 1.1kB   licence.txt          
npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       4.0.3                                   
npm notice package size:  86.3 kB                                 
npm notice unpacked size: 343.2 kB                                
npm notice shasum:        50873f24eb7780d76bb321500c2ad994416d9312
npm notice integrity:     sha512-sqdUTp2FURRsi[...]9JJeddrIyyg2w==
npm notice total files:   6

@GreenImp
Copy link
Collaborator Author

GreenImp commented May 2, 2020

So it looks like not publishing the uncompressed files is a no-go, as it can make it difficult for devs to use the library, if they need to do thier own minifications.

I think this is a definitive list of all the files that should be included, and the file sizes:

npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 511.6kB lib/esm/bundle.js    
npm notice 570.7kB lib/umd/bundle.js    
npm notice 148.6kB lib/esm/bundle.min.js
npm notice 158.9kB lib/umd/bundle.min.js
npm notice 25B     lib/umd/package.json 
npm notice 2.5kB   package.json         
npm notice 10.9kB  CONTRIBUTING.md      
npm notice 34.4kB  readme.md            
npm notice 1.1kB   licence.txt          
npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       4.0.3                                   
npm notice package size:  290.2 kB                                
npm notice unpacked size: 1.4 MB                                  
npm notice shasum:        c47dc90ba453a37304c7522a661d40413365061c
npm notice integrity:     sha512-T7j5fZNCM0mxZ[...]FfDuTK9B9O+Pg==
npm notice total files:   9

It's still rather large, as expected, because the compiled scripts are quite big.

I'm going to play around with the mathJS library, to see if using the full version (Which supports ES modules and tree shaking!) can actually decrease the compiled size.

@GreenImp
Copy link
Collaborator Author

GreenImp commented May 3, 2020

I can't seem to get the mathJS library working with Jest - it complains about unexpected export. I think it's because the mathJS library is defined as an ES5 package, that contains some ES6 files, but there isn't anything specifying that they're ES6 modules, so Jest tries to load them as ES5 scripts.

Either way, I think I'll leave it as is for now. I've got an .npmignore file set up to exclude all the files that aren't required which, although it doesn't reduce the bundle size much, at least makes the directory structure cleaner.

@GreenImp GreenImp linked a pull request May 3, 2020 that will close this issue
@GreenImp GreenImp modified the milestones: 4.1.0, 4.2.0 May 3, 2020
@GreenImp GreenImp removed this from the 4.2.0 milestone May 15, 2020
@GreenImp
Copy link
Collaborator Author

GreenImp commented May 19, 2020

I've just found that Rollup can be told not to bundle external dependencies, which means that I could get it to ignore the MathJS library when bundling.
That means that the compiled scripts will be much smaller, but we can still leave MathJS as a dependency in the package.json so it gets brought in.

It looks like I might even be able to get Rollup to ignore any dependencies in my package.json file: https://medium.com/@kelin2025/so-you-wanna-use-es6-modules-714f48b3a953#726c

If this works, we'll need to add a note to the readme for anyone using the UMD file directly, rather than npm / yarn, that they'll need to include the mathJS script themselves.

@dixhuit
Copy link

dixhuit commented Jul 25, 2020

I've just been evaluating rpg-dice-roller as a replacement for another lib that I'm currently using but when compiled for production I'm seeing an increased bundle size of 0.8 MB. Granted, rpg-dice-roller offers a lot more but even then this is currently a bit hard to swallow.

I'm very interested in any of the improvements discussed in this thread 👍

@GreenImp
Copy link
Collaborator Author

Hey @danbohea. Yeah it's a lot larger than I'd like at the moment, because it's got the entire MathJs library, and random-js libraries included in the bundled files.

I tried decoupling them, so they're not included in the bundle but have had some difficulty getting it to work. I'm planning on taking another look at it as one of the next things, but also more than happy for any PRs that can sort it.

@GreenImp
Copy link
Collaborator Author

GreenImp commented Jul 28, 2020

So the main problem here is that both MathJS and Random-js don't properly support ES modules when not using a bundler, like Webpack.

When those libraries are bundled with the dice roller, they work fine, because Rollup handles them for us. But when unbundled and trying to import the libraries, it tries to bring in the CommonJS versions, which it doesn't like.

For example, in a nodeJS project using ES module import, rather than CommonJS require, it throws the error:

import { evaluate } from 'mathjs';
^^^^^^^^
SyntaxError: The requested module 'mathjs' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from 'mathjs';
const { evaluate } = pkg;

There doesn't seem to be anything I can do inside the dice roller library, but it looks fixable with some changes to the third party libraries themselves.
I've raised a ticket with MathJS to see if we can work on getting it sorted: josdejong/mathjs#1928

It's worth noting that externalising the library and trying it in a CommonJS project works okay.

@GreenImp
Copy link
Collaborator Author

GreenImp commented Aug 7, 2020

I've raised a PR on mathjs that should resolve this, for the most part; josdejong/mathjs#1941

Assuming that gets merged, it will help a lot, and enable mathJS to be used without a package bundler (e.g. directly in node).

The next step is being able to use the smaller version of mathjs (mathjs/number), which my PR does fix. However, the Jest tests then fail because Jest doesn't currently support the packages.json's export property: jestjs/jest#9771

So this is all a big plus, and heading in the right direction!

@GreenImp
Copy link
Collaborator Author

The needed MathJS changes are now in PR josdejong/mathjs#1962

@dixhuit
Copy link

dixhuit commented Sep 17, 2020

This is great news!

@GreenImp
Copy link
Collaborator Author

GreenImp commented Sep 22, 2020

Testing with the MathJS#v8 branch, I can reduce the dice roller package to less than 600kb:

npm notice 📦  [email protected]
npm notice === Tarball Contents === 
npm notice 192.2kB lib/esm/bundle.js                           
npm notice 213.4kB lib/umd/bundle.js                           
npm notice 46.5kB  lib/esm/bundle.min.js                       
npm notice 50.6kB  lib/umd/bundle.min.js  
...
npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       4.5.1                                   
npm notice package size:  106.5 kB                                
npm notice unpacked size: 574.0 kB                                
npm notice shasum:        17f10e7a445fcd8e077efddc85ba8d60b4654444
npm notice integrity:     sha512-8LbHMHYifffWx[...]1z5iFRTnJxblg==
npm notice total files:   49  

That's with mathjs externalised for both ESM and UMD files, and randomjs externalised for UMD (It's not ESM compatible so it needs to be bundled for ESM, but it's small anyway).

The actual installed package size will be slightly bigger, because it will include Mathjs - but only once! The current package includes it three times (In the ESM, UMD bundles, and in the package.json).

Once Mathjs v8 has been released, then I can look to getting this sorted!

@GreenImp GreenImp self-assigned this Sep 25, 2020
@GreenImp GreenImp added this to the 5.0.0 milestone Sep 25, 2020
@GreenImp
Copy link
Collaborator Author

GreenImp commented Nov 7, 2020

Math-js v8 has been released!
I've merged code into the develop branch that removes math-js from the compiled bundles, which greatly reduces the bundle sizes.

I had hoped to do the same with random-js, but it's not currently fully ESM compatible, so I'm only able to remove it from the UMD bundle. I have raised a PR with random-js, but it seems unlikely to be merged soon.

This is very much a breaking change for two main reasons:

  1. Using the UMD bundle through the browser will also need to include math-js and ramdom-js as dependencies. e.g.:
    <script src="path/to/mathjs/lib/browser/math.js"></script>
    <script src="path/to/random-js/lib/random-js.min.js"></script>
    <script src="https://cdn.jsdelivr.net/npm/rpg-dice-roller@VERSION/lib/umd/bundle.min.js"></script>
    
  2. Using the ESM bundle directly in the browser (Without a compiler) will no longer work, because the import statements for random-js and math-js will no longer work.

As most users are probably either using compilers (e.g. Webpack, Rollup etc.), or the browser, these changes hopefully won't affect many users.
Anyone using the scripts in node, or using a compiler shouldn't have to make any changes.

I'm going to push this out with v5, but I've got a few more things that need to be done for that release, so it's going to be a little while.

@dixhuit
Copy link

dixhuit commented Nov 8, 2020

Fantastic news! I look forward to v5.

@GreenImp
Copy link
Collaborator Author

It's been a long while, but I've just released v5.0.0, which has these changes in.

The Tarball size is:

npm notice === Tarball Details === 
npm notice name:          rpg-dice-roller                         
npm notice version:       5.0.0                                   
npm notice filename:      rpg-dice-roller-5.0.0.tgz               
npm notice package size:  105.0 kB                                
npm notice unpacked size: 573.5 kB                                
npm notice shasum:        c11f0be83d3502c35082b6225259de209f7ba755
npm notice integrity:     sha512-UPE9RVFa9Eveq[...]JWQ/gKmLiElHw==
npm notice total files:   49  

And you can check the Bndlephobia and Packagephobia sizes in the issue description.

@GreenImp
Copy link
Collaborator Author

GreenImp commented Apr 20, 2021

Okay, this is just really strange. Even though the tarball size has decreased a lot, for some reason Packagephobia says that the package size has increased by quite a large amount. I'm not sure why, as the dependencies are the same, but have been remove from the compiled scripts.

I'm not sure what Bundlephobia thinks of it, as the site appears to be down (I'm seeing a 502 status).

@GreenImp GreenImp reopened this Apr 20, 2021
@GreenImp
Copy link
Collaborator Author

Okay, I'm guessing that it's because Mathjs is around 10mb: https://packagephobia.com/result?p=mathjs install size

When it was included in the compiled scripts, although it was in both UMD and ESM, I wonder if it only included parts of the library.

@GreenImp GreenImp removed this from the 5.0.0 milestone Apr 23, 2021
@SageRalph
Copy link

SageRalph commented Jun 25, 2021

I am interested in this library but hesitant about the size. In an empty Webpack project with this library as the only dependency I am seeing a production build size of 597KB. Analysis of the bundle shows this is almost all from Mathjs.

It seems the only use of Mathjs is the evaluate function. Perhaps this could benefit from custom bundling, as outlined here: https://mathjs.org/docs/custom_bundling.html ?

@GreenImp
Copy link
Collaborator Author

Yeah, mathjs is massive, unfortunately.

I did look into custom bundling, however, the problem with using the evaluate method is that it then uses pretty much everything else.
But I think it's definitely worth another look. There's got to be something we can remove from it.

Frustratingly, it looks like around 12% of the bundle size is documentation:

About 25% of the bundle size comes from the expression parser. Half of this comes from the embedded docs.

If anyone wants to take a shot at this, it's more than welcome.

@GreenImp
Copy link
Collaborator Author

GreenImp commented May 9, 2023

I've been looking at this again recently, and have had some success with custom bundling MathJS.

Part of the issue was that both MathJS and random-js were being bundled into the compiled JS files, but were also in the dependencies list, so they were getting included twice - knocking the installed bundle size up to nearly 12mb.

I've now moved them over to devDependencies, so they don't get re-downloaded when the library is installed.
And I've used the custom bundling to only bring in a sub-set of the MathJS functionality. The issue here is that there's no easy way of determining which parts of MathJS I need to bring in, so I may have missed some, or included some that are unnecessary.

I've also knocked down the file sizes from:

Old Old version New New version
Bundlephobia minified 5.1.1 Bundlephobia minified
Bundlephobia minified + gzip 5.1.1 Bundlephobia minified + gzip
Packagephobia install size Packagephobia install size

Which is quite a decent drop.

I've manually tested the ESM, UMD and browser versions and they all still seem to work okay, and all the tests are currently passing though, so that's a good sign.

I'd like to get this out in a release but, before I do it would be really nice if anyone is willing to give it a try?
You can just install the alpha branch, and use it as normal, with:

// to install the latest alpha release
npm install @dice-roller/rpg-dice-roller@next

// or to install this specific alpha release
npm install @dice-roller/[email protected]

I'll leave this ticket open for a bit to hopefully get some feedback 😀

@danbohea , @DavidRalph any chance you're still using this library and willing to give the alpha release a try for file size and see if it still works for you?
I think the only thing to be aware of is that, if you're using the browser version, then you no longer need to separately include MathJS and random-js as described in the docs.

@GreenImp GreenImp added testing dependencies Pull requests that update a dependency file labels May 22, 2023
@GreenImp GreenImp pinned this issue Jun 10, 2023
@mparpaillon
Copy link

Just installed the alpha version and... YES <3

Before
Stat size: 1.97Mb
Parsed size: 614kb
Gzipped size: 170kb
Capture d’écran 2023-06-15 à 09 54 01

After
Stat size: 745kb
Parsed size: 224kb
Gzipped size: 65kb
Capture d’écran 2023-06-15 à 09 57 18

@GreenImp
Copy link
Collaborator Author

Nice! Thanks for testing it. Can you confirm that it's all still working for you?
Also, what are you using to check the file sizes? It looks really handy.

@GreenImp GreenImp added this to the 6.0.0 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement help wanted testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants