Add size-limit bundle size analysis tool

andresz1 commented 4 years ago

Performance budget tool

This PR is an initial idea (something that I thought that maybe could be useful after @jaredpalmer tweet) of how a performance budget tool could be added automatically in every template. Having this kind of tool helps you to keep your library bundle size as small as possible. I like size-limit because besides the size it can calculate the time it would take a browser to download and execute your JS.

I added @size-limit/preset-small-lib by default but in case that library size increases, it could be easily replaced by @size-limit/preset-big-lib (adding time).

Bundle size check

yarn size

Bundle analyze

yarn analyze

GitHub action

The action added (size-limit-action) will execute size-limit a comment the PRs made with the comparison of it.

Any feedback would be appreciated and I'd be more than happy to iterate over a solution if you like the idea. Thanks in advance and for the great tool!

andresz1 commented 4 years ago

Hi @agilgur5! please let me know if you have any thoughts about this! 🙏

agilgur5 commented 4 years ago

Hey @andresz1 I definitely like the changes, thank you for the PR! I took a look when you first made the PR, but I just haven't had the time to dive deeper and review all the pieces of this. There are a handful of related issues and some code that was commented out from removed features related to this that I need to sort through, understand how size-limit may differ from those, think through template vs. dependency route, investigate any potential impact on multi-entry and/or preserveModules, and actually review the code

andresz1 commented 4 years ago

Hi @agilgur5! I'm glad you like the idea 🎉 and thank you for the reply. Let me know if you need help. Is there any tsdx discord or spectrum channel where we could discuss ?

agilgur5 commented 4 years ago

Sorry for the delay on reviewing this, I've been incredibly stressed and busy, so larger PRs to add features got pretty backlogged as they take a good bit of time to really dive into.

There are a handful of related issues and some code that was commented out from removed features related to this that I need to sort through

Here's a few past references:

Summarizing, rollup-plugin-size-snapshot was previously used and still has some unused / commented out code references to it, rollup-plugin-visualizer was previously considered, and rollup-plugin-analyzer was previously recommended

understand how size-limit may differ from those

Did a quick glance through on NPM trends and size-limit seems to have a lot of nicer features:

currently maintained (some alternatives are not)
also does timing (none of the others do)
also does visualization (most of the others don't)
doesn't need to be added to Rollup build (some of the others do)

That being said, it does have more dependencies like webpack which I'm not sure are necessary for our use-case considering TSDX bundles everything with Rollup. It looks like webpack is a plugin though and can be disabled, but is included in @size-limit/preset-small-lib. The visualizations aren't as fancy as rollup-plugin-visualizer but probably nbd.

think through template vs. dependency route

Can probably stick to template initially as a testing ground and then move to a dependency later if there is enough usage. Though with #634 we're moving a bit away from dependencies.

investigate any potential impact on multi-entry and/or preserveModules

I think this will require something like webpack to bundle pieces. Multi-entry it would need to show each entry and preserveModules should theoretically all be bundled up from one entry. It looks like this can be configured with size-limit

and actually review the code

In progress.

Is there any tsdx discord or spectrum channel where we could discuss ?

No, and I don't think that would be helpful either. Keeping discussions async and transparent on a PR is ideal. Watching those channels would take more time as well.

agilgur5 commented 4 years ago

Ah, also

Two references from above comment should be removed here too:
- https://github.com/formium/tsdx/blob/79dddc4ed6aeee89472999cede9e44ac951c548c/src/createRollupConfig.ts#L197-L199
- https://github.com/formium/tsdx/blob/5e5c3f804e68609083be978a080ab905dc7ec19d/src/env.d.ts#L15
Should really add some docs somewhere about this. The template READMEs may be appropriate places for that since they mention CI

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/formium/tsdx/5e8p1xxco
✅ Preview: https://tsdx-git-fork-andresz1-master.formium.vercel.app

agilgur5 commented 4 years ago

@andresz1 I'd make the changes myself and merge but there were some unresolved questions remaining from previous review that I was hoping you could answer

andresz1 commented 4 years ago

@agilgur5 sorry I was on vacations and thank you so much for the great feedback! I just answered your questions and made so changes!

agilgur5 commented 4 years ago

@andresz1 no worries on the delay or if you're on vacation, everyone does that. Was just following up, especially since I couldn't find the answers to some of these myself.

agilgur5 commented 4 years ago

@all-contributors please add @andresz1 for code, doc, example

allcontributors[bot] commented 4 years ago

@agilgur5

I've put up a pull request to add @andresz1! :tada:

jaredpalmer / tsdx