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

💅 🏗️ Decrease bundlewatch maxSizes, #1043

Merged
merged 2 commits into from
Oct 13, 2021
Merged

Conversation

tomalec
Copy link
Member

@tomalec tomalec commented Oct 12, 2021

Changes proposed in this Pull Request:

Set index.js bundle size to 150% of median JS file size from
https://httparchive.org/reports/page-weight#bytesJs.
Set other *.js size, to twice the size of the biggest one.

So we could set more ambitious goals, and get warned/blocked at more proportional change.

IMHO, it's still very forgiving, as we would have to make the biggest JS bundles twice a size to get blocked.
So I'm open to discussion to make those limits even smaller.

  • Maybe 120% of current file sizes?

Screenshots:

before

image

after

image

Detailed test instructions:

  1. See the bundlewatch details

Changelog entry

Set `index.js` bundle size to 150% of median JS file size from
https://httparchive.org/reports/page-weight#bytesJs.
Set other `*.js` size, to twice the size of the biggest one.
@tomalec tomalec self-assigned this Oct 12, 2021
@tomalec tomalec requested a review from a team October 12, 2021 14:30
@tomalec tomalec marked this pull request as ready for review October 12, 2021 14:31
@eason9487
Copy link
Member

So I'm open to discussion to make those limits even smaller.

  • Maybe 120% of current file sizes?

Upvote for this.

Among the planned features I know, there will be a new dependency library to be added from #882. But seems that it could be imported by dependency extraction in WC-admin 2.4.1(included within WC 5.5), so it's not expected to increase the index.js file size much. Refs:

120% should give us a better chance of detecting unusual changes in file size, such as the one found in #821, which we might not have noticed if it had not increased by 2.5 times, but only by 1.5 times. And it gives us a mindset that whether the increase is really in need or reasonable. Even if future features require the maximum size to be increased, it's not a problem at all - just change to the next reasonable value along with that feature PR. 😎

Set them to 120% of the biggest file (rounding down).
@tomalec
Copy link
Member Author

tomalec commented Oct 13, 2021

Changed to 120% of the current files:
image

Copy link
Member

@eason9487 eason9487 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tomalec tomalec merged commit ae74f48 into develop Oct 13, 2021
@tomalec tomalec deleted the update/bundlesize-limits branch October 13, 2021 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants