-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Comments
The package is currently 1.4MB 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 📦 rpg-dice-roller@4.0.3
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 📦 rpg-dice-roller@4.0.3
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 📦 rpg-dice-roller@4.0.3
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 |
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:
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. |
I can't seem to get the mathJS library working with Jest - it complains about unexpected Either way, I think I'll leave it as is for now. I've got an |
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. It looks like I might even be able to get Rollup to ignore any dependencies in my 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. |
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 👍 |
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. |
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 For example, in a nodeJS project using ES module
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. It's worth noting that externalising the library and trying it in a CommonJS project works okay. |
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 ( So this is all a big plus, and heading in the right direction! |
The needed MathJS changes are now in PR josdejong/mathjs#1962 |
This is great news! |
Testing with the MathJS#v8 branch, I can reduce the dice roller package to less than 600kb:
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! |
Math-js v8 has been released! 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:
As most users are probably either using compilers (e.g. Webpack, Rollup etc.), or the browser, these changes hopefully won't affect many users. 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. |
Fantastic news! I look forward to v5. |
It's been a long while, but I've just released v5.0.0, which has these changes in. The Tarball size is:
And you can check the Bndlephobia and Packagephobia sizes in the issue description. |
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). |
Okay, I'm guessing that it's because Mathjs is around 10mb: https://packagephobia.com/result?p=mathjs 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. |
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 ? |
Yeah, mathjs is massive, unfortunately. I did look into custom bundling, however, the problem with using the Frustratingly, it looks like around 12% of the bundle size is documentation:
If anyone wants to take a shot at this, it's more than welcome. |
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 I've now moved them over to I've also knocked down the file sizes from:
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? // 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/rpg-dice-roller@6.0.0-alpha.4 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? |
Nice! Thanks for testing it. Can you confirm that it's all still working for you? |
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 thepackage.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:
unpacked size: 1.7 MB
unpacked size: 580.4 kB
unpacked size: 588.6 kB
The text was updated successfully, but these errors were encountered: