feat: celebrate bundle size decreases and report no-change#137
feat: celebrate bundle size decreases and report no-change#137bartveneman wants to merge 8 commits intoe18e:mainfrom
Conversation
- Setting pack-size-threshold to -1 now shows 🎉 when bundle size decreases, and⚠️ when it increases (reporting all changes) - When packed bundle size diff is 0, a "No bundle size changes" message is posted instead of silently doing nothing - Add tests for all new bundle-size behaviors - Update README and action.yml docs for pack-size-threshold=-1 https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
|
| 📦 Package | 📋 Versions |
|---|---|
| @actions/http-client | 2 versions
|
| eslint-visitor-keys | 2 versions
|
| ignore | 2 versions
|
💡 To find out what depends on a specific package, run: npm ls example-package
…hold warnings The previous totalSizeChange === 0 check incorrectly triggered the "No bundle size changes" message when individual package changes cancelled each other out (e.g. one package shrinks by 100 kB while another grows by 100 kB). Replace with a per-package allUnchanged check so the message only fires when every package is truly unchanged. For the warning path, compare net total size change against the threshold rather than individual packages. This means if pkg-a shrinks 100 kB and pkg-b grows 100 kB with a 50 kB threshold, no warning is shown since the total bundle size is unchanged — consistent with how a size threshold over the total makes sense. Also add tests covering the two previously untested edge cases. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
|
generally looks good! i do wonder though if its better ux to have a merged table? i.e. show what do you think? i can probably be swayed either way 😅 |
…ld is -1 Instead of two separate messages (one for 🎉 decreases, one for⚠️ increases), all changed packages are now shown in a single table. The heading adapts based on direction: 🎉 for pure decreases,⚠️ for pure increases, 📦 for mixed. The size change column uses signed values (negative for decreases) so direction is clear at a glance. Update tests and README accordingly. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
Add a formatBytesSigned helper that prefixes positive values with '+' so the size change column is unambiguous in both the threshold=-1 path (which can mix increases and decreases) and the threshold>0 warning path. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
|
I think that's a very good point, made the change. Also added |
When pack-size-threshold is not set (defaults to 50000) and bundle sizes are unchanged, no message should appear — consistent with how all other checks behave: stay silent unless a threshold is exceeded. The 'No bundle size changes' message is now scoped to threshold=-1, which is the only mode where the user has opted into always seeing output. Add a test covering the no-op case for threshold>0 and move the explicit assertion for the no-change message into the threshold=-1 test. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
src/checks/bundle-size.ts
Outdated
| return; | ||
| } | ||
|
|
||
| const totalSizeChange = comparison.packChanges.reduce( |
There was a problem hiding this comment.
does this need Math.abs or is the change.sizeChange always positive?
i.e. so totalSizeChange < threshold works whether it is negative or positive
The message is only posted when pack-size-threshold is -1, not for the default threshold. Update the docs to reflect this. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
Summing all sizeChanges (positive and negative) meant a large decrease in one package could mask a large increase in another, causing the threshold check to silently skip a real warning. Now only positive changes are summed for the threshold comparison, consistent with the warning table which already filters to increases. sizeChange is signed ((sourceSize ?? 0) - (baseSize ?? 0)), so no Math.abs is needed — filtering to > 0 is sufficient and correct. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
… change Three new cases that directly encode the reasoning from the fix: 1. A pure decrease, however large, never triggers a warning — sizeChange is signed so a shrink produces a negative value that cannot exceed a positive threshold. 2. A large decrease in one package does not mask an increase in another even when the net total is negative — the old sum-all approach would have returned -400 KB < 50 KB and stayed silent. 3. Multiple sub-threshold increases are summed together and can collectively exceed the threshold — two ×30 KB increases (each below 50 KB) combine to 60 KB and trigger the warning. https://claude.ai/code/session_01Mdf48qd8iQh4WG1F1dbHT1
| .filter((change) => change.sizeChange > 0) | ||
| .reduce((sum, change) => sum + change.sizeChange, 0); | ||
|
|
||
| if (totalSizeChange < threshold) { |
There was a problem hiding this comment.
in main, we basically check if any packages exceeded the threshold. this checks if the sum of packages exceeds the threshold.
are we sure this bit is needed?
maybe we could just do what we did originally:
const packWarnings = comparison.packChanges.filter(
(change) => change.exceedsThreshold && change.sizeChange > 0
);
if (packWarnings.length > 0) {i.e. if any package exceeds the threshold, show the table.
otherwise this will show the table if no packages exceed the threshold, but their total change does.
There was a problem hiding this comment.
Oh, my brain went "that'd be nice as a guard rail" but no worries, I'll revert that bit 👌
The machines did all the implementation for me, but I prompted them extensively with test cases to cover, which is hopefully reflected in the amount of tests.
pack-size-thresholdto-1now shows 🎉 when bundle size decreases, andNo bundle size changesmessage is posted instead of silently doing nothingREADMEandaction.ymldocs forpack-size-threshold=-1closes #136