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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃彈 Update @ampproject/filesize to 4.0.1 with new API #27898

Merged

Conversation

kristoferbaxter
Copy link
Contributor

@kristoferbaxter kristoferbaxter commented Apr 21, 2020

Filesize 4.0.1 was released with improved concurrency, the ability to filter types of compression, and an updated API.

These changes have been applied in this PR.

Local Travis
Before 4.23s 16s
After 2.55s 7.74s

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the bundle size check failed. Is this expected?

@kristoferbaxter
Copy link
Contributor Author

I restarted the bundle size check and it worked successfully.

@@ -196,6 +196,7 @@
"dist/v0/*-?.?.js",
"dist/*.mjs",
"dist/v0/*-?.?.mjs"
]
],
"trackFormat": ["brotli"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this filters out the reporting to just brotli correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option specifies which formats (compression formats) to track. In the case of AMP, the bundlesize check only cares about brotli.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that yarn add and prettier do not agree on how this setting should be formatted inside package.json. I've filed ampproject/filesize#83 to request that the suggestion in #26907 (comment) (a separate config file for filesize) be implemented.

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This halves the time taken on Travis. LGTM!

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

Successfully merging this pull request may close these issues.

None yet

5 participants