Skip to content

Revert user-agent check in performance-impl - #26730

Closed
micajuine-ho wants to merge 2 commits into
ampproject:masterfrom
micajuine-ho:remove_user_agent
Closed

Revert user-agent check in performance-impl#26730
micajuine-ho wants to merge 2 commits into
ampproject:masterfrom
micajuine-ho:remove_user_agent

Conversation

@micajuine-ho

@micajuine-ho micajuine-ho commented Feb 11, 2020

Copy link
Copy Markdown
Contributor

Check window.PerformancePaintTiming instead of user-agent to see if first-contentful-paint and first-paint are supported.

Reverts the installation of platform service in some places.

@amp-owners-bot
amp-owners-bot Bot requested a review from dvoytenko February 11, 2020 07:11
@micajuine-ho
micajuine-ho removed the request for review from dvoytenko February 11, 2020 07:11
@amp-bundle-size
amp-bundle-size Bot requested a review from powerivq February 11, 2020 07:20
@micajuine-ho

Copy link
Copy Markdown
Contributor Author

cc @powerivq for bundle size check

@powerivq

Copy link
Copy Markdown
Contributor

Looking at the code, I don't feel it should increase the bundle size. Ideas?

@micajuine-ho

micajuine-ho commented Feb 11, 2020

Copy link
Copy Markdown
Contributor Author

Looking at the code, I don't feel it should increase the bundle size. Ideas?

I agree, but not sure why this would happen. @danielrozenberg Do you have any ideas?

@kristoferbaxter

kristoferbaxter commented Feb 11, 2020

Copy link
Copy Markdown
Contributor

@micajuine-ho Can you post a gist of the output for before and after? I can take a look.

@micajuine-ho

Copy link
Copy Markdown
Contributor Author

@kristoferbaxter Here is the gist

@kristoferbaxter

Copy link
Copy Markdown
Contributor

The change here is as follows:

  1. Raw Output 206,473 -> 206,389
  2. Brotli (-Z) 55,873 -> 55,838
  3. Prettier 291,438 -> 291,333

I'm not certain why this is reported as an increase.

@powerivq

Copy link
Copy Markdown
Contributor

@danielrozenberg to help debugging this.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants