Add `bundlewatch/bundlewatch` to dev dependencies. #819

tomalec · 2021-06-24T11:09:10Z

Changes proposed in this Pull Request:

Add bundlewatch/bundlewatch to dev dependencies.
to monitor our progress in reducing bundle size.

Fixes #818.

Screenshots:

Detailed test instructions:

check out this branch
npm install
npx bundlewatch
Click on the link for the detailed report

To be done:

Authorize the app, to enable GitHub check. https://service.bundlewatch.io/setup-github

Changelog entry

Tweak - Add bundlewatch to monitor bundle size in GitHub PRs.

to monitor our progress in reducing bundle size. Fixes #818.

tomalec · 2021-07-02T11:45:37Z

Small research for `bundlewatch` usage in Woo

gutenberg-products-block repo tried it then removed it over a year ago in favor of custom GH workflow, for the following reasons:

We get inline feedback (as a comment) in the pull on any size changes.

Personally, I find it a disadvantage:

It comments only the initial PR post (commits set), so we don't get bundle size after commits that may follow during the review. In GLA we tend to make quite a number of commits after the initial PR.
This way we cannot block the merge if we enforce a bundle size budget

This compares overall changes against the base branch for the pull (which we weren't getting with bundlewatch).

If I understood the docs correctly, the current version of bundlewatch can pick the PR base from Travis and compare against it.

A list of all files and their changes is shown in a collapsed list in the posted comment. With bundlewatch, their linking system was broken, so if we wanted that granularity we had to go into the travis-ci job (which only showed us the sizes, not the difference from the base branch).

For my local test, linking was working fine. Also, I think in GLA we do not have so many bundles, so hopefully, it's not a big issue for us.

Removes the time for the bundlewatch checks from the linting travis job (not a huge difference but helps).

If we're not experiencing hitting Travis limit yet, maybe we can still use it, and worry about it once we start exceeding it.

woocommerce.com is using bundlesize the ancestor of bundlewatch

In woocommerce-admin some efforts were made to try to setup a custom github action, but it seems it wasn't finished
https://github.com/woocommerce/woocommerce-admin/actions/workflows/bundle-size.yml

tomalec · 2021-07-02T11:51:07Z

I think the above gives us precedence to trust the bundlewatch enough to authorize it, WDYT @jconroy?

ecgan · 2021-07-05T13:14:13Z

@tomalec , I love the analysis you posted, good stuff 🙂 👍

woocommerce.com is using bundlesize the ancestor of bundlewatch

That got me curious. I looked into both bundlesize and bundlewatch, and it seems like bundlesize is much more popular than bundlewatch.

https://github.com/siddharthkp/bundlesize: 4.2k stars, 207 forks, 82k weekly downloads in https://www.npmjs.com/package/bundlesize.
https://github.com/bundlewatch/bundlewatch: 263 stars, 28 forks, 69k weekly downloads in https://www.npmjs.com/package/bundlewatch.

I tried to google for comparisons between the two to find out which is better, but there isn't any helpful info on this.

So my question here is: given its popularity, should we consider using bundlesize instead of bundlewatch? Why or why not? 🤔

tomalec · 2021-07-05T14:16:50Z

[…] given its popularity, should we consider using bundlesize instead of bundlewatch? Why or why not? thinking

AFAIK bundlewatch is kindof community fork of bundlesize, as -size got somewhat unmaintained.
You can check bundlewatch'es list of reasons

Bundlesize has entered maintenance mode and pull requests are left hanging, so we wanted to reboot the community through creating BundleWatch

Check the latest release dates:

https://github.com/siddharthkp/bundlesize/releases - 0.18.0 Jun 29, 2019
https://github.com/bundlewatch/bundlewatch/releases - 0.3.2 Jan 26, 2021

jconroy · 2021-07-06T12:25:08Z

Thanks @tomalec for looking into this and especially for searching around in this comment to look for existing usages within Woo/A8C

Authorize the app, to enable GitHub check

I think both BundleSize and BundleWatch area already approved for the Woo org.

I like the idea of using Actions (especially now the repo is public) - we'll likely use more - but happy for us to try BundleWatch and revisit it and see if the previous areas of concern have improved.

I leave @ecgan and @eason9487 to comment/feedback/review before merging

ecgan · 2021-07-06T14:38:23Z

@tomalec ,

You can check bundlewatch'es list of reasons

Hey, thanks for that! I didn't notice that. Google search should have shown me that 😆

Check the latest release dates:

siddharthkp/bundlesize/releases - 0.18.0 Jun 29, 2019

And I did not notice that too. I did notice that on https://www.npmjs.com/package/bundlesize, the latest release is 0.18.1 and it is published 6 months ago. I looked into the repo and the release is related to axios security vulnerability issue (siddharthkp/bundlesize#369), and from that issue I notice there is another repo https://github.com/siddharthkp/bundlesize2, which does not seem to be very active either.

With all the above said (including your good findings), I think I would lean towards bundlewatch now. Just hoping that it can do everything that bundlesize does, and there are good documentations that can be easily googled. 😄

ecgan

Tested as per instruction, working good, LGTM 👍

ecgan · 2021-07-06T16:32:25Z

package.json

+	"bundlewatch": {
+		"files": [
+			{
+				"path": "./js/build/*.js",
+				"maxSize": "2 kB"
+			},
+			{
+				"path": "./js/build/index.js",
+				"maxSize": "1 mB"
+			},
+			{
+				"path": "./js/build/*.css",
+				"maxSize": "1 kB"
+			},
+			{
+				"path": "./js/build/index.css",
+				"maxSize": "8 kB"
+			},
+			{
+				"path": "./google-listings-and-ads.zip",
+				"maxSize": "12 mB",
+				"compression": "none"
+			}
+		]


💅 Personally I would prefer to put these into a separate file like .bundlewatch.config.js and have bundlewatch to automatically read the file, unfortunately currently bundlewatch requires us to specify it in the command line like bundlewatch --config .bundlewatch.config.js (see the documentation), which is an unnecessary friction in my opinion. Hopefully they can support that in the future.

So with that said, good to have this bundlewatch section here in this package.json so that we can just run npx bundlewatch. 👍

tomalec · 2021-07-07T21:02:44Z

I tried adding GitHub action for bundlewatch, for example below.
I tried to make it run for every push, not just initial PR.
But there is a bug in bundlewatch <- ci-env bundlewatch/bundlewatch#422
We could hard-code it to compare only against trunk and develop, but in my opinion comparing against base makes the most sense.

So I fall back to running it on Travis-CI, where it works out of the box.

name: bundlewatch

on: [push]

jobs:
  build:
    if: "!contains(github.event.commits[0].message, '[skip ci]')"
    runs-on: ubuntu-latest
    timeout-minutes: 10

    steps:
    - uses: actions/checkout@v2

    - name: Install npm dependencies
      run: npm ci

    - name: Build
      run: npm run build

    - name: Run bundlewatch
      run: npx bundlewatch
      env:
        BUNDLEWATCH_GITHUB_TOKEN: ${{ secrets.BUNDLEWATCH_GITHUB_TOKEN }}
        CI_COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
        CI_BRANCH_BASE: ${{ github.base_ref || 'trunk' }}

GitHub Actions are a bit too problematic, see #819 (comment)

GitHub Actions are a bit too problematic, see #819 (comment) Remove redundant `npm ci` from `Coding standard check`. Increase maxSizes of bundles.

ecgan · 2021-07-08T13:31:57Z

package.json

+        "ci": {
+            "trackBranches": ["trunk", "develop"]
+        }


@tomalec , just noticed this, really minor nitpick: the three lines here are using space indentation instead of tabs like other lines. 😄

I'd address it in #869

Addresses #819 (comment)

Add bundlewatch/bundlewatch to dev dependencies.

1ab5cf8

to monitor our progress in reducing bundle size. Fixes #818.

tomalec self-assigned this Jun 24, 2021

tomalec requested a review from a team June 24, 2021 11:09

tomalec mentioned this pull request Jun 24, 2021

Bump @wordpress/scripts to 16.1.3 (from 12.5.0) #821

Closed

ecgan approved these changes Jul 6, 2021

View reviewed changes

Merge branch 'trunk' into add/818-bundlewatch

9c42641

tomalec added a commit that referenced this pull request Jul 7, 2021

Add bundlewatch to .travis.yml

490c980

GitHub Actions are a bit too problematic, see #819 (comment)

tomalec force-pushed the add/818-bundlewatch branch from 1487571 to 490c980 Compare July 7, 2021 21:39

Add bundlewatch to .travis.yml

ed9cbc7

GitHub Actions are a bit too problematic, see #819 (comment) Remove redundant `npm ci` from `Coding standard check`. Increase maxSizes of bundles.

tomalec force-pushed the add/818-bundlewatch branch from 163c4b1 to ed9cbc7 Compare July 7, 2021 22:33

tomalec merged commit 172096f into trunk Jul 7, 2021

tomalec deleted the add/818-bundlewatch branch July 7, 2021 23:04

ecgan reviewed Jul 8, 2021

View reviewed changes

tomalec added a commit that referenced this pull request Jul 8, 2021

Convert spaces to tabs in package.json.

93389ce

Addresses #819 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `bundlewatch/bundlewatch` to dev dependencies. #819

Add `bundlewatch/bundlewatch` to dev dependencies. #819

tomalec commented Jun 24, 2021

tomalec commented Jul 2, 2021 •

edited

tomalec commented Jul 2, 2021

ecgan commented Jul 5, 2021

tomalec commented Jul 5, 2021 •

edited

jconroy commented Jul 6, 2021 •

edited

ecgan commented Jul 6, 2021

ecgan left a comment

ecgan Jul 6, 2021

tomalec commented Jul 7, 2021 •

edited

ecgan Jul 8, 2021

tomalec Jul 8, 2021

Add bundlewatch/bundlewatch to dev dependencies. #819

Add bundlewatch/bundlewatch to dev dependencies. #819

Conversation

tomalec commented Jun 24, 2021

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions:

To be done:

Changelog entry

tomalec commented Jul 2, 2021 • edited

Small research for bundlewatch usage in Woo

tomalec commented Jul 2, 2021

ecgan commented Jul 5, 2021

tomalec commented Jul 5, 2021 • edited

jconroy commented Jul 6, 2021 • edited

ecgan commented Jul 6, 2021

ecgan left a comment

Choose a reason for hiding this comment

ecgan Jul 6, 2021

Choose a reason for hiding this comment

tomalec commented Jul 7, 2021 • edited

ecgan Jul 8, 2021

Choose a reason for hiding this comment

tomalec Jul 8, 2021

Choose a reason for hiding this comment

Add `bundlewatch/bundlewatch` to dev dependencies. #819

Add `bundlewatch/bundlewatch` to dev dependencies. #819

tomalec commented Jul 2, 2021 •

edited

Small research for `bundlewatch` usage in Woo

tomalec commented Jul 5, 2021 •

edited

jconroy commented Jul 6, 2021 •

edited

tomalec commented Jul 7, 2021 •

edited