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

General Performance Improvements #313

Open
ags1773 opened this issue Dec 14, 2020 · 2 comments
Open

General Performance Improvements #313

ags1773 opened this issue Dec 14, 2020 · 2 comments

Comments

@ags1773
Copy link
Contributor

ags1773 commented Dec 14, 2020

To Do:

  1. return html as a stream - might improve Reduce initial server response time
  2. Lighthouse audit warns Links to cross-origin destinations are unsafe. This article suggests adding rel="noopener" or rel="noreferrer" on external links
  3. Avoid serving legacy JavaScript to modern browsers. Refer this article
  4. Lighthouse audit warns to Properly size images
  5. serve images in nextGen format
  6. Ensure text remains visible during webfont load coming on this story - try adding fonts using @font-face instead of <link> refer this doc
@ags1773
Copy link
Contributor Author

ags1773 commented Dec 22, 2020

perf optimizations that are already done:

Things done:

  1. return html as a stream: Can't be done because of styled components. We are using react purely on server-side, there is no hydration happening on client side. Styled components' streams API needs hydration to happen client side in order to work with node streams.
  2. adding nofollow noopener on external links
  3. Properly size images: Adding more srcset options with x descriptors instead of absolute widths srcset = thumbor-stg.assettype.com/image.png?w=640 1x, thumbor-stg.assettype.com/image.png?w=1200 2x, thumbor-stg.assettype.com/image.png?w=2400 4x
  4. serve images in nextGen format: add format:auto for getting gumlet images in nextGen format
  5. remove regex wherever possible, use string.startsWith, string.includes etc. instead

This was referenced Dec 29, 2020
@ags1773
Copy link
Contributor Author

ags1773 commented Jan 7, 2021

Note for QA:

I've released a beta version of framework 4.5.10-amp-perf.7 for testing.
Vikatan branch containing this > amp-perf-testing
amp test repo branch > performance-fixes
can deploy these branches and test

Image fixes

Previously, following components were using their own <amp-img>. This PR changes them to use our custom <Image> component. So we need to see if these components are breaking/look different than before

  • heroImage
  • related stories card
  • visual stories - 1st card (cover image)
  • visual stories - all subsequent cards

In addition to this, following components use <Image> component.

  • The story element for image
  • carousel story elelemt

Checks to be done for <Image> component:

  • query param format=auto should be added if the cdn-image is gumlet (i.e. if gumlet is enabled)
  • focus point should work
  • the image should be of proper width & height if it has the correct metadata info available (it shouldn't look weird). But if metadata info isn't present, it should be of aspect ratio 16:9 (this might look weird, but that's okay)
  • it should have a srcset attribute containing following four widths: 480, 960, 1200, 2048. If you emulate a low-end phone, it should load a low-res image (can check network tab)

Special cases

In additipn to above checks these are the special cases, additional things that need to be checked

Visual stories:

  • check if focus-point is being correctly set
  • check if aspect ratio is correctly set

Related stories:

  • if there is a related story without a hero image, the related story card should use a fallback image for that card

Other Checks

  • external links should have a rel=”nofollow noopener” target="_blank" attributes
  • in the page source, the link tag which fetches fonts should have a display=swap attribute. For example
<link data-react-helmet="true" rel="preload" as="style" crossorigin="anonymous" href="https://fonts.googleapis.com/css?family=Mukta+Malar:300,400,600,700">

should change to

<link data-react-helmet="true" rel="preload" as="style" crossorigin="anonymous" href="https://fonts.googleapis.com/css?family=Mukta+Malar:300,400,600,700&display=swap">

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

No branches or pull requests

1 participant