Github PR report integration

[vio]

• [x] generate PR comment from report data (total bundle size, changes by file type)
• [x] add Github PR comment integration support
• [x] allow to enable/disable the integration for each project

[jflayhart]

@vio we are starting to use Relative-CI as so far no other tool has come close nor is it easy to roll our own reporting software, so this might be easier to contribute towards instead!

What are we thinking for this?

[vio]

hi @jflayhart, thanks for trying RelativeCI and for your feedback 🙇

This feature is based on the feedback provided by existing users. Some developers will prefer to surface the information from the GitHub Check (ex: https://github.com/relative-ci/bundle-stats/pull/1101/checks?check_run_id=1542230522) as a PR comment and have the report information visible for the entire team. My personal experience is that PRs tend to become noisy as more tools are adding comments and that GitHub checks are a better way to provide this kind of information.

However, I think the feature is useful for many teams. The plan is to add it as an optional integration and have a minimal version of the Github Check report with links to the full detailed report on app.relative-ci.com.

I am still gathering feedback, would appreciate it if you can share your thoughts on what information would you find helpful to see in the PR comment ;)

[jflayhart]

I contribute to the NextJS project and there are things I like and don't like about their build size reporting comments.

1) It's hella noisy, and they do what they wanna do and it's working for them, but I easily have over 100 build size comments in one of my bigger PRs which drown out contributor comments. 2) That being said, I like that I see the build diff and how my changeset negatively or positively affected the overall lib size.

While NextJS isn't using your tool per se, it's the same idea. But, for relative-ci maybe we should only comment on PRs if it goes over some threshold a user sets and like you said show the same data in comment as what's currently in github check report.

You do have the Details link directly to Github check, so that is better than nothing. But the issue is the check is neutral and doesn't pass/fail.

[jflayhart]

I also see this project playing a bigger role as official webpack bundle reporter/audit, like Lighthouse but for webpack :D This is a useful tool!

[jflayhart]

All my team and I care about are what changed, what did I do to fail the build size, is there enough insight to fix the issue?

Alot like what codecov does already in that it gives coverage, inc/dec delta, and replaces any previous comment it left prior, so as to only generate 1 codecov comment for latest changeset.

[vio]

You do have the Details link directly to Github check, so that is better than nothing. But the issue is the check is neutral and doesn't pass/fail.

I agree that having the commit status reflect the build info is very useful, that's something planned for https://github.com/relative-ci/roadmap/issues/8.

I also see this project playing a bigger role as official webpack bundle reporter/audit, like Lighthouse but for webpack :D This is a useful tool!

Thank you, great to have some optimistic feedback from time to time :) While bundle-stats/relative-ci is not affiliated with any of the projects you mentioned, I've been a user & supporter of both for a long time.

All my team and I care about are what changed, what did I do to fail the build size, is there enough insight to fix the issue?

I think so. Also, I plan to add more insights to the report - https://github.com/relative-ci/bundle-stats/issues/429#issue-512955250

Alot like what codecov does already in that it gives coverage, inc/dec delta, and replaces any previous comment it left prior, so as to only generate 1 codecov comment for latest changeset.

Thanks for the example, one commit per PR sounds like a good trade-off

[jflayhart]

You wanna divvy up work or you got this? I'll help where I can, just might need to get caught up to speed with codebase. We definitely should do more insights and github checks (pass vs fail).

I am also using your GUI

[vio]

You wanna divvy up work or you got this?

Contributions are welcome! ;) The main body of work is on https://github.com/relative-ci/bundle-stats/issues/385, i will try to add more info to the ticket in the following days.

I'll help where I can, just might need to get caught up to speed with codebase.

Probably the best place to start is from extractors (the fns responsible for getting the data from the webpack stats) - https://github.com/relative-ci/bundle-stats/blob/master/packages/utils/src/webpack/extract/index.js

I am also using your GUI

you mean bundle-stats html report or app.relative-ci.com?

[jflayhart]

app.relative-ci.com

[jflayhart]

like lighthouse does, it would be nice if the "Details" github check link goes to the app.relative-ci.com report and then a PR comment is left with a codecov-like visual of the bundle size.

[vio]

@jflayhart the github check report is already having a link to the job report, but maybe that's not visible enough?

As we add more insights to the report, will be useful to have links to the individual sections (eg: /assets, /modules, /packages).

[jflayhart]

I think this might be a simple-enough summary for a PR comment:

Type	Size
JS	380KB (2%) 🔼
CSS	30KB (5%) 🔽
Duplicate Packages	0 (0%)

ℹ️ View more details on RelativeCI

---

alternatively we could allow config-based selection of what types to show via a package.json property, which could also extend to setting thresholds.

[jflayhart]

ideally, it would be nice to do something like this:

OR

[vio]

I think this might be a simple-enough summary for a PR comment

Thanks @jflayhart, planning to pick it up soon.

alternatively we could allow config-based selection of what types to show via a package.json property, which could also extend to setting thresholds.

I think for the PR comment can show only the types that changed. As for config, we might be able to provide a different comment template as a setting, though not sure how helpful is going to be for users.

ideally, it would be nice to do something like this:

We have issues to add tree maps for packages and modules, but it should be helpful for assets and size by type as well. For the standalone report i was looking for a very slim library (the js code is inlined in the standalone report), though app.relative-ci.com is already using recharts for the dashboard. I will prefer a light version that we could share between the standalone report and app.relative-ci.com, let me know if you are familiar with a light library for treemap charts.

[jflayhart]

@vio i took a look at the code and couldnt find where to add comment functionality

[jflayhart]

im fine with a simple tabular report for now but charting eventually would be awesome contained in a comment.

here are a few lightweight libs ive used in the past:

• https://www.chartjs.org/ (OSS)
• https://www.anychart.com/ (small, performant)
• https://www.npmjs.com/package/highcharts

[vio]

@vio i took a look at the code and couldnt find where to add comment functionality

@jflayhart the github integration is part of the service(app.relative-ci.com) and is closed source atm. Since the data that we want to display (total bundle size, size by type) is already available in the bundle-stats report, all the changes required for this issue are at the service level - https://github.com/relative-ci/roadmap/issues/17#issue-612944198. Will add some examples here once I have something running ;)

We talked earlier about the budget feature and that's something that we need to add as an insight first and after we can work on surfacing the data on the UI and on the integrations(Github Check/PR, Slack - app.relative-ci.com). Let me know if you have any feedback/questions about it ;)

[vio]

hi @jflayhart, here is the first version of the PR comment:

1. bundle size change in the title
2. changed metrics
3. changed assets by type
4. link to the full report

Let me know what do you think

[jflayhart]

oh so changes assets by type is dynamic in that comment? that's cool!

I dont see the point of cache invalidation. I only care about bundle size, so the less info necessary to still get point across, the better imo.

[vio]

@jflayhart yes, metrics and totals by type are displayed only if changed. The comment is just a compressed version of the check report - https://github.com/relative-ci/bundle-stats/pull/1148/checks?check_run_id=1658046801

Cache invalidation metric is useful to spot changes that are wider than expected, duplicated code across the chunks or it can be used to optimize the chunk splitting, but i understand that is not something to check on every change.

To simplify even more the comment, every section can be wrapped in <details> to hide the data by default:

[jflayhart]

@vio That looks great! compressed, simplified, and if we want to know more we can dig deeper as desired.

For most cases all we care about is the overall app size increase as that is also likely what thresholds will be compared against.

Mental note: might be nice to have little info linke next to stats to explain exactly what they are and how their size is affected. You and I might know what and why, but I'd say MOST frontend devs do not in my experience.

[jflayhart]

For example @vio , we could link to some web.dev report on why JS size is important and can negatively affect page load times and therefore conversion rates. MDN is also a solid ref. Would love all this to be self-documenting in a way. Descriptive as well as even prescriptive, not just for JS files but for images too.

things like https://web.dev/reduce-javascript-payloads-with-code-splitting/

Nobody likes waiting. Over 50% of users abandon a website if it takes longer than 3 seconds to load.

this effectively should enable JS devs to build more performant PWAs.

Not saying all this needs to be avail for MVP (as a first-pass) but something to consider to educate on how these things affect core web vitals.

[vio]

@vio That looks great! compressed, simplified, and if we want to know more we can dig deeper as desired.

@jflayhart Thanks for the feedback! The details wrappers are definitely better since the content of comment can vary a lot and create a bit of cognitive overload.

Mental note: might be nice to have little info linke next to stats to explain exactly what they are and how their size is affected. You and I might know what and why, but I'd say MOST frontend devs do not in my experience.

That's a great idea and will be really useful to show that type of information. Feel free to create a new issue for it ;) I think the information that is general available can be surfaced quite easily, and for the one that is depending on the value of a metric/asset/module, we can use insights.

[jflayhart]

or get more ppl to use the dashboard ui by including better insights like you started doing here:

[vio]

https://twitter.com/Relative_CI/status/1354425186561437697 ;)