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

[Spike][WIP]Add size limit configuration #3076

Closed
wants to merge 3 commits into from
Closed

Conversation

domoscargin
Copy link
Contributor

No description provided.

@domoscargin domoscargin force-pushed the bk-spike-size-limit branch 4 times, most recently from 5fe9fbd to 700b2ec Compare December 6, 2022 16:31
@domoscargin domoscargin changed the title Add size limit configuration [Spike][WIP]Add size limit configuration Dec 6, 2022
Testing that imports work from a central file, and that we can test against singular exports (in this case, the Accordion).
@domoscargin domoscargin reopened this Dec 14, 2022
@domoscargin
Copy link
Contributor Author

domoscargin commented Dec 14, 2022

Transferring some random slack thoughts here:

  • The associated Github issue is unverified, so we'd have some light hoop-jumping to do to get it approved by the @alphagov org
  • Using the webpack and webpack-why plugins is probably the most interesting setup, as we can get an HTML report on things like included modules, duplicate modules, etc.
  • In my examples, the govuk-esm/all.mjs is no longer working, for some reason (it did before)
  • I think we'd have to write our own plugins for failing workflows on things like duplicate modules (anything other than file size/time to download)

Checking file size/time to download, and being able to dig into failures using webpack-why seems like a good improvement as a quick and dirty mvp. Things that may be trickier, but worthwhile:

  • Long-term trends - we could save run datas to a spreadsheet or something in the roughest case
  • Plugins for failing workflows for custom reasons (and actions to comment this on PRs?)

Random thought:
Could we skip size-limit and run Statoscope or similar on our Webpack example?

@romaricpascal
Copy link
Member

A couple of thoughts regarding tracking the size of our bundle:

  • Which files should we run it in and what should we check regarding tree shaking? To be complete, feels like we should check dist, the compiled CJS files (to check if there's not a sudden bump in size due to surprise polyfills/transpiling) in the package and the "public" MJS files in package, both imported as a whole and each individual export. That feels like quite a lot though and may be complex to figure the exports for each MJS file (though we can always import in a node script)😬
  • do we want it to be failing checks or just informative for now (eg. add a comment on the PR or something to help review)?

I like the idea of running Statoscope on Webpack directly. In the end, we'll still have to handle failing the check/commenting whichever path we chose, so that'd be one less layer to run through than if we use size-limit, and maybe more control on options as well? I'm also quite intrigued by their --reference option for comparing branches inside pull requests

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3076 December 15, 2022 11:21 Inactive
@romaricpascal
Copy link
Member

Spike work is complete and perf work carries on in #3279

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.

3 participants