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

Change bundlesize reporter to use brotli size #21275

Closed
jridgewell opened this issue Mar 5, 2019 · 4 comments · Fixed by #24418
Closed

Change bundlesize reporter to use brotli size #21275

jridgewell opened this issue Mar 5, 2019 · 4 comments · Fixed by #24418

Comments

@jridgewell
Copy link
Contributor

The gzip file size is largely irrelevant now, and certain optimizations we make that reduce the size of the gzip output can actually increase the size of the brotli output.

@rsimha
Copy link
Contributor

rsimha commented Mar 5, 2019

Switching the calculation should be fairly straightforward by adding -c brotli to the npx bundlesize invocation.

/**
* Get the gzipped bundle size of the current build.
*
* @return {string} the bundle size in KB rounded to 2 decimal points.
*/
function getGzippedBundleSize() {
const cmd = `npx bundlesize -f "${runtimeFile}"`;
log('Running', cyan(cmd) + '...');
const output = getStdout(cmd).trim();
const bundleSizeOutputMatches = output.match(/PASS .*: (\d+.?\d*KB) .*/);
if (bundleSizeOutputMatches) {
const bundleSize = parseFloat(bundleSizeOutputMatches[1]);
log('Bundle size is', cyan(`${bundleSize}KB`));
return bundleSize;
}
throw Error('could not infer bundle size from output.');
}

However, there will be a period when PRs will have base commits for which we only have the gzipped bundle size. I suppose we can ease into this by initially computing both sizes, and then switching exclusively from gzip to brotli at some point.

Assigning to @danielrozenberg.

@danielrozenberg
Copy link
Member

danielrozenberg commented Aug 5, 2019

My work plan on changing this is as follows:

  1. Modify the reporter for push builds to double-write the bundle size on each commit to bundle-size/[SHA] (gzip) and bundle-size/[SHA].br (brotli), do this for 1 month
    1. Modify the reporter for pull requests to report both gzip and brotli bundle-sizes to the app
    2. Modify the bundle-size app to read the [SHA].br instead of the [SHA] file
    1. Stop reporting the gzip size for pull-requests and push-builds
    2. Delete all gzip sizes in amphtml-build-artifacts
    3. Cleanup any leftover gzip code

@rsimha
Copy link
Contributor

rsimha commented Oct 3, 2019

Reverted in #24882

@danielrozenberg
Copy link
Member

Unreverted in #24888

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants