From 9ce003802109f704cc1f081759f3d2af2c1ea2c2 Mon Sep 17 00:00:00 2001 From: Kynson Szetau <46522440+Kynson@users.noreply.github.com> Date: Mon, 20 Jan 2025 17:59:41 +0800 Subject: [PATCH] fix: audit incorrectly flagging images as above the fold (#12993) (#12998) * fix: audit incorrectly flag images as above the fold (#12993) * chore: add changeset * Add additional tests for perf audit --- .changeset/cuddly-rings-sing.md | 5 ++ packages/astro/e2e/dev-toolbar-audits.test.js | 49 +++++++++++++++++++ .../src/pages/audits-perf-absolute.astro | 9 ++++ .../pages/audits-perf-body-unscrollable.astro | 38 ++++++++++++++ .../src/pages/audits-perf-relative.astro | 9 ++++ .../dev-toolbar/apps/audit/rules/perf.ts | 15 +++++- 6 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 .changeset/cuddly-rings-sing.md create mode 100644 packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-absolute.astro create mode 100644 packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-body-unscrollable.astro create mode 100644 packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-relative.astro diff --git a/.changeset/cuddly-rings-sing.md b/.changeset/cuddly-rings-sing.md new file mode 100644 index 000000000000..a5200c47ca2c --- /dev/null +++ b/.changeset/cuddly-rings-sing.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes the issue that audit incorrectly flag images as above the fold when the scrolling container is not body diff --git a/packages/astro/e2e/dev-toolbar-audits.test.js b/packages/astro/e2e/dev-toolbar-audits.test.js index ca7476314212..50a4ab17514d 100644 --- a/packages/astro/e2e/dev-toolbar-audits.test.js +++ b/packages/astro/e2e/dev-toolbar-audits.test.js @@ -44,6 +44,55 @@ test.describe('Dev Toolbar - Audits', () => { await appButton.click(); }); + test('does not warn about perf issue for below the fold image after mutation when body is unscrollable', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/audits-perf-body-unscrollable')); + + const toolbar = page.locator('astro-dev-toolbar'); + const appButton = toolbar.locator('button[data-app-id="astro:audit"]'); + await appButton.click(); + + const auditCanvas = toolbar.locator('astro-dev-toolbar-app-canvas[data-app-id="astro:audit"]'); + const auditHighlights = auditCanvas.locator('astro-dev-toolbar-highlight'); + + expect(auditHighlights).toHaveCount(1); + + await page.click('body'); + + let consolePromise = page.waitForEvent('console'); + await page.locator('#mutation-button').click(); + await consolePromise; + + await appButton.click(); + + expect(auditHighlights).toHaveCount(1); + }); + + test('does not warn about perf issue for below the fold image in relative container', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/audits-perf-relative')); + + const toolbar = page.locator('astro-dev-toolbar'); + const appButton = toolbar.locator('button[data-app-id="astro:audit"]'); + await appButton.click(); + + const auditCanvas = toolbar.locator('astro-dev-toolbar-app-canvas[data-app-id="astro:audit"]'); + const auditHighlights = auditCanvas.locator('astro-dev-toolbar-highlight'); + + expect(auditHighlights).toHaveCount(0); + }); + + test('can warn about perf issue for below the fold image in absolute container', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/audits-perf-absolute')); + + const toolbar = page.locator('astro-dev-toolbar'); + const appButton = toolbar.locator('button[data-app-id="astro:audit"]'); + await appButton.click(); + + const auditCanvas = toolbar.locator('astro-dev-toolbar-app-canvas[data-app-id="astro:audit"]'); + const auditHighlights = auditCanvas.locator('astro-dev-toolbar-highlight'); + + expect(auditHighlights).toHaveCount(1); + }); + test('can handle mutations', async ({ page, astro }) => { await page.goto(astro.resolveUrl('/audits-mutations')); diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-absolute.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-absolute.astro new file mode 100644 index 000000000000..2ab42edde1eb --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-absolute.astro @@ -0,0 +1,9 @@ +--- +import { Image } from "astro:assets"; +import walrus from "../light_walrus.avif"; +--- +
+
+ A walrus +
+
\ No newline at end of file diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-body-unscrollable.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-body-unscrollable.astro new file mode 100644 index 000000000000..bad3142e7e7a --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-body-unscrollable.astro @@ -0,0 +1,38 @@ +--- +import { Image } from "astro:assets"; +import walrus from "../light_walrus.avif"; +--- + + +
+ A walrus +
+ + A walrus + +
+
+ + + + + \ No newline at end of file diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-relative.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-relative.astro new file mode 100644 index 000000000000..70e0c32039e6 --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf-relative.astro @@ -0,0 +1,9 @@ +--- +import { Image } from "astro:assets"; +import walrus from "../light_walrus.avif"; +--- + +
+
+ A walrus +
diff --git a/packages/astro/src/runtime/client/dev-toolbar/apps/audit/rules/perf.ts b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/rules/perf.ts index 6a1749d00156..18c0f7d35171 100644 --- a/packages/astro/src/runtime/client/dev-toolbar/apps/audit/rules/perf.ts +++ b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/rules/perf.ts @@ -35,8 +35,14 @@ export const perf: AuditRuleWithSelector[] = [ 'img:not([loading]), img[loading="eager"], iframe:not([loading]), iframe[loading="eager"]', match(element) { const htmlElement = element as HTMLImageElement | HTMLIFrameElement; + // Ignore elements that are above the fold, they should be loaded eagerly - const elementYPosition = htmlElement.getBoundingClientRect().y + window.scrollY; + let currentElement = element as HTMLElement; + let elementYPosition = 0; + while (currentElement) { + elementYPosition += currentElement.offsetTop; + currentElement = currentElement.offsetParent as HTMLElement; + } if (elementYPosition < window.innerHeight) return false; // Ignore elements using `data:` URI, the `loading` attribute doesn't do anything for these @@ -55,7 +61,12 @@ export const perf: AuditRuleWithSelector[] = [ const htmlElement = element as HTMLImageElement | HTMLIFrameElement; // Ignore elements that are below the fold, they should be loaded lazily - const elementYPosition = htmlElement.getBoundingClientRect().y + window.scrollY; + let currentElement = element as HTMLElement; + let elementYPosition = 0; + while (currentElement) { + elementYPosition += currentElement.offsetTop; + currentElement = currentElement.offsetParent as HTMLElement; + } if (elementYPosition > window.innerHeight) return false; // Ignore elements using `data:` URI, the `loading` attribute doesn't do anything for these