[LHCI] Resolve Lighthouse CI Performance Test Error and Added Desktop Audit

[randychilau]

Description

This PR fixes the following LHCI issue:

When LHCI runs during GitHub Actions check for PRs, it will show the following performance audit error on the pages /[pagename].html (except for 404.html).

source

source

Here is the LHCI after changes:

---

This PR adds the following feature:

LHCI desktop audit

---

Here is the most recent LHCI workflow for this PR:
https://github.com/layer5io/layer5/actions/runs/5308808724/jobs/9608696217?pr=4397

Report for mobile version of "/company/about/index.html":

Report for desktop version of "/company/about/index.html":

---

Notes for Reviewers

The LHCI performance issue is caused by recent changes to resolve the flickering trailing slash issue.

When the LHCI workflow runs npm run build, because it is taking place in Github Actions, the env variable CI is true which triggers a conditional to create pages with urls [pagename].html instead of [pagename]\index.html. However, the Gatsby server on Github Pages is unable to serve the direct url (for an unknown reason), you can see this in action on the live site (test: https://layer5.io/about.html or watch the video below):

404_for_html_ext.webm

This is not an issue for the live site, because the user and all page references use /[pagename] format to access pages instead of using the /pagename.html url.

This is an issue for the LHCI tests since it attempts to reach /pagename.html for testing and sees a 404 page which is causing the error in the performance audit.

To resolve this issue, we set the env variable CI to false for the command npm run build in the LHCI GitHub Action:

and have it test pages with the \[pagename]\index.html format (which was how it was done before the trailing slash flicker changes were implemented).

---

Adding the desktop audit

LHCI can also audit the desktop version (the default is the mobile version). Updating the LHCI workflow to reference another config file allows an audit and upload reports of pages loading in the desktop.

While these tests are not as critical since pages perform generally well on the desktop, it is still useful to have a warning issued if pages fall below a performance metric (e.g. 90%). Currently the desktop audits only three pages (404, index, about), and will serve as an additional warning sign of serious site issues.

Cheers,
Randy

Signed commits

• Yes, I signed my commits.

[l5io]

🚀 Preview for commit 6da5ba2 at: https://648ff2539ea3322db5d1a352--layer5.netlify.app

[l5io]

🚀 Preview for commit 28c65e8 at: https://648ff6a012fa58295740d8d8--layer5.netlify.app

[l5io]

🚀 Preview for commit 29ffbfc at: https://6490002c12fa582eec402148--layer5.netlify.app

[vishalvivekm]

Thanks @randychilau
I'll add this as an agenda item in today's websites call to share with others on the call.

[l5io]

🚀 Preview for commit 20a05ef at: https://6490ebf0b32a79444546655d--layer5.netlify.app

[randychilau]

Hi @vishalvivekm,

Thanks for the message and adding it to the call, unfortunately I was not able to attend (hooray 🎉 for the youtube uploads).

Cheers,
Randy

[l5io]

🚀 Preview for commit 139bfdf at: https://6491c7b67dc74904d3808017--layer5.netlify.app

[ayushthe1]

Thanks for the changes @randychilau and giving such a detailed report. The performance audits are giving the correct results now.
For desktop audit ,could you please include all the other URLs also for testing.(which are commented currently). Also can we have a log output for desktop audit in a similar way as you have implemented for the default mobile audit (in #4405) as below :

This will help anyone to just take a glance across scores for all pages and then correspondingly open the report for that page which has poor score.

[l5io]

🚀 Preview for commit ccc667a at: https://64986c69dd209720f6da657b--layer5.netlify.app

[randychilau]

Hi @ayushthe1,

Thanks for the message and taking the time to look at the PR.

As mentioned, I've included the other URLs.

Regarding the log output, the desktop audit is configured to report the same as the mobile audit in #4405:

note: pages generally perform better in desktop audits (due to omission of the network throttling used in mobile audits), so there are usually less warnings in the desktop audit log output.

Look forward to any additional feedback.

Cheers,
Randy

[ayushthe1]

Thanks @randychilau for the changes.

[l5io]

🚀 Preview for commit e95e206 at: https://649e897b3d6e9940daef0cbc--layer5.netlify.app

[l5io]

🚀 Preview for commit 387fbc3 at: https://64ae35ad61d42a7ff9fbe387--layer5.netlify.app

[l5io]

🚀 Preview for commit b13289a at: https://64af17cf37723720c0a929b8--layer5.netlify.app

[l5io]

🚀 Preview for commit cebb008 at: https://64af24361109a406a75d4a15--layer5.netlify.app

[randychilau]

Hi All,

The CI Performance Test Error part of this PR overlapped with PR #4405, and when #4405 was merged it resolved the CI Performance Test Error.

image example of Performance Test Error resolved

Please note that the "desktop audit" part of this PR is still valid for merge consideration. There should not be any issues with #4405 already merged since it was shared code, however if the Maintainers wish to create a separate PR for this, please let me know.

---

re: fix: svgoConfig from "cleanupIDs" to "cleanupIds" commit

I just noticed in the build that this warning began appearing over and over again, which I believe started with the Bump @svgr/webpack from 6.5.1 to 8.0.1 PR merge:

warning You are trying to configure cleanupIDs which is not part of preset-default.
Try to put it before or after, for example

plugins: [
  {
    name: 'preset-default',
  },
  'cleanupIDs'
]

image example of repeated warning

This build log (view raw build log) happened on 7/11, the day before the PR merged, and did not show the warning .

This build log (view raw build log) happens after the PR merged, where it shows the warning.

I believe the bump introduced case-sensitivity for configs, and that the plugin setting with "cleanupIDs" needed to be updated to "cleanupIds" (as indicated here) which seems to have resolved the error (view raw build log).

I added the change in this PR, but again if the Maintainers wish to create a separate PR for this, please let me know.

Cheers,
Randy

[l5io]

🚀 Preview for commit 1db3920 at: https://64c197170eb4e619f6676058--layer5.netlify.app

[Chadha93]

LGTM, I'll wait for @Nikhil-Ladha's acknowledgment.