[GH Request] Add support for bundle size CI checks

[adamstankiewicz]

Firm Name

2U

Urgency

Low (2 weeks)

Problem/Request

Open edX has not had a large focus on optimizing MFE performance in terms of best practices such as bundle and code splitting. Keeping an eye on bundle size is easily forgotten and, if not managed, can become a performance bottleneck for users.

One potential way to mitigate this is to make use of CI checks around bundle size, to alert developers when their contributions have negative affects on the standards set (per repo) related to asset size in the resulting output in the dist directory from running npm run build.

For example, the two primary tools for this are bundlewatch and bundlesize.

I believe both would require a CI auth variable to be set up for the GitHub organization, for example:

• https://bundlewatch.io/#/getting-started/using-bundlewatch?id=setup-ci-auth-variables-needed-by-bundlewatch
• https://service.bundlewatch.io/setup-github

By exposing the BUNDLEWATCH_GITHUB_TOKEN auth variable (if we opted for bundlewatch), maintainers of repos could:

1. Specify a configuration file (e.g., bundlewatch.config.js) to define the rules around bundle size for their specific repo and use cases.
2. Add an NPM script to package.json to run bundlewatch CLI.
3. Run the added NPM script during CI workflows.

This request is primarily to make it possible to use a tool such as bundlewatch/bundlesize in the openedx GitHub organization; the decision to use it would remain with maintainers/owning teams of repos; though if we find value in it, could recommend its use more broadly.

Reasoning

Hypothesis: more proactive observability around frontend repos' bundle size for production builds should lead to more focus on frontend performance in terms of bundle/code splitting to keep the bundle/asset size for owned repos in check. Failing a PR's CI due to a contribution's code leading to file(s) going over a predefined, reasonable limit should help frontend engineers more proactively consider the performance impacts of their code.

[openedx-workflow-automation]

Thank you for your report! @openedx/axim-oncall will triage within a business day. Simple requests usually take 2-3 business days to resolve; more complex requests could take longer.

[ormsbee]

@arbrandes, @brian-smith-tcril: Any thoughts on this particular integration? This certainly seems reasonable to me, but I'm not knowledgable about the frontend ecosystem.

[brian-smith-tcril]

After some discovery and discussion, I think we have a decent sense of what a path forward for this looks like.

• BundleWatch

  • Pros:
    • Used by popular projects such as bootstrap
    • According to the GitHub README it is more actively maintained and developed than bundlesize
  • Cons:
    • It isn't clear exactly how actively maintained it is, most recent merged PR listed is from January 2022
    • Relies on a service hosted on bundlewatch.io
      • If this is an issue it can be worked around by using a custom server

• Moving forward with BundleWatch

  • Use a couple repos to pilot
    • @adamstankiewicz mentioned
      • Paragon is committed/willing to pilot it
      • Interested in having the Enterprise MFEs (frontend-app-learner-portal-enterprise & frontend-app-admin-portal) use it as part of that pilot
  • Add to frontend-template-application
    • We want this to be opt-out, meaning we
      • Default to blocking PRs if bundlewatch fails
      • Allow repo owners to continue running bundlewatch but not block PRs on failure
        • If we follow the GitHub actions workflow pattern bootstrap uses, we can do this by utilizing continue-on-error
          • We can add a commented out continue-on-error line to the frontend-template-application workflow to simplify this
    • @adamstankiewicz mentioned
      • frontend-template-application is going to need to define its own bundlewatch.config.json file
      • the files array could point to common, generic glob paths (e.g., dist/*/.js to just cover all JavaScript files that are built by npm run build) and that would be applicable as a starting point to all repos
      • owning teams should probably be aware they might need to adjust their bundlewatch.config.json config file as their application scales
    • We should update the frontend-template-application readme to mention
      • bundlewatch is set up to run on dist/*/.js
        • you will likely need to modify the bundlewatch config to fit your needs (link to example where that was needed)
      • the GitHub actions workflows are set up to not allow PR merging without passing bundlewatch
        • to allow PR merging without passing bundlewatch, uncomment line X in workflow Y

There was also discussion around having all of the repos reference one workflow. I'm not sure how that fits in with the commented-out continue-on-error strategy/if there's a better way to handle that.

edit: should just be able to pass a bool through to continue-on-error
edit again: based on how frontend-template-application does commitlint https://github.com/openedx/.github/blob/master/.github/workflows/commitlint.yml https://github.com/openedx/frontend-template-application/blob/master/.github/workflows/commitlint.yml it seems we can try a few things

jobs:
  bundlewatch:
    continue-on-error: true
    uses: openedx/.github/.github/workflows/bundlewatch.yml@master

or if that doesn't work, we can try using with

jobs:
  bundlewatch:
    uses: openedx/.github/.github/workflows/bundlewatch.yml@master
    with:
      shouldContinueOnError: true

and have the shared workflow use

continue-on-error: shouldContinueOnError

[brian-smith-tcril]

@PKulkoRaccoonGang requested https://www.codechecks.io/

couple notable things about CodeChecks

from the https://www.codechecks.io/ homepage:

Do not need our hosting? Try setting up self-hosted instance always for free.

from https://github.com/codechecks/docs

With Codechecks you can replace tools like:

• BundleWatch
• CodeCov

[brian-smith-tcril]

I drafted an ADR for this openedx/open-edx-proposals#515

[feanil]

@adamstankiewicz can you review openedx/open-edx-proposals#515 and see if it meets your needs. Once we have agreement and the decision lands, we can figure out the implementation steps.

[adamstankiewicz]

@feanil @brian-smith-tcril Left a few comments, but overall LGTM; approved!

[sarina]

@brian-smith-tcril is there more to be done on this ticket?

[brian-smith-tcril]

I believe the next step would be to choose a repo to pilot the integration with. I think that could work as a separate request, so closing this one seems reasonable to me.