diff options
Diffstat (limited to '')
18 files changed, 309 insertions, 41 deletions
diff --git a/.changeset/twenty-plums-sell.md b/.changeset/twenty-plums-sell.md new file mode 100644 index 000000000..e2a480cc2 --- /dev/null +++ b/.changeset/twenty-plums-sell.md @@ -0,0 +1,5 @@ +--- +"astro": minor +--- + +Adds initial support for performance audits to the dev toolbar diff --git a/.github/workflows/test-hosts.yml b/.github/workflows/test-hosts.yml index 40ed5691d..018f97162 100644 --- a/.github/workflows/test-hosts.yml +++ b/.github/workflows/test-hosts.yml @@ -2,7 +2,7 @@ name: Hosted tests on: schedule: - - cron: '0 0 * * 0' + - cron: '0 0 * * 0' env: ASTRO_TELEMETRY_DISABLED: true @@ -28,24 +28,21 @@ jobs: uses: actions/setup-node@v4 with: node-version: 18 - cache: "pnpm" + cache: 'pnpm' - name: Install dependencies run: pnpm install - + - name: Build Astro - run: pnpm turbo build --filter astro --filter @astrojs/vercel + run: pnpm turbo build --filter astro --filter @astrojs/vercel - name: Build test project working-directory: ./packages/integrations/vercel/test/hosted/hosted-astro-project - run: - pnpm run build - + run: pnpm run build + - name: Deploy to Vercel working-directory: ./packages/integrations/vercel/test/hosted/hosted-astro-project - run: - pnpm dlx vercel --prod --prebuilt + run: pnpm dlx vercel --prod --prebuilt - name: Test - run: - pnpm run test:e2e:hosts + run: pnpm run test:e2e:hosts diff --git a/.gitignore b/.gitignore index da6e08ed0..82c2ac20d 100644 --- a/.gitignore +++ b/.gitignore @@ -22,6 +22,7 @@ package-lock.json *.env packages/astro/src/**/*.prebuilt.ts +packages/astro/src/**/*.prebuilt-dev.ts !packages/astro/vendor/vite/dist packages/integrations/**/.netlify/ diff --git a/packages/astro/components/Image.astro b/packages/astro/components/Image.astro index 9c4fcf7e9..8704d4cc7 100644 --- a/packages/astro/components/Image.astro +++ b/packages/astro/components/Image.astro @@ -29,6 +29,10 @@ const additionalAttributes: HTMLAttributes<'img'> = {}; if (image.srcSet.values.length > 0) { additionalAttributes.srcset = image.srcSet.attribute; } + +if (import.meta.env.DEV) { + additionalAttributes['data-image-component'] = 'true'; +} --- <img src={image.src} {...additionalAttributes} {...image.attributes} /> diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro index adfc0fcab..82a97af5d 100644 --- a/packages/astro/components/Picture.astro +++ b/packages/astro/components/Picture.astro @@ -61,6 +61,10 @@ if (props.sizes) { if (fallbackImage.srcSet.values.length > 0) { imgAdditionalAttributes.srcset = fallbackImage.srcSet.attribute; } + +if (import.meta.env.DEV) { + imgAdditionalAttributes['data-image-component'] = 'true'; +} --- <picture {...pictureAttributes}> diff --git a/packages/astro/e2e/dev-toolbar-audits.test.js b/packages/astro/e2e/dev-toolbar-audits.test.js new file mode 100644 index 000000000..45c80a873 --- /dev/null +++ b/packages/astro/e2e/dev-toolbar-audits.test.js @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; +import { testFactory } from './test-utils.js'; + +const test = testFactory({ + root: './fixtures/dev-toolbar/', +}); + +let devServer; + +test.beforeAll(async ({ astro }) => { + devServer = await astro.startDevServer(); +}); + +test.afterAll(async () => { + await devServer.stop(); +}); + +test.describe('Dev Toolbar - Audits', () => { + test('can warn about perf issues zzz', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/audits-perf')); + + 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'); + + const count = await auditHighlights.count(); + expect(count).toEqual(2); + + for (const auditHighlight of await auditHighlights.all()) { + await expect(auditHighlight).toBeVisible(); + + const auditCode = await auditHighlight.getAttribute('data-audit-code'); + expect(auditCode.startsWith('perf-')).toBe(true); + + await auditHighlight.hover(); + const auditHighlightTooltip = auditHighlight.locator('astro-dev-toolbar-tooltip'); + await expect(auditHighlightTooltip).toBeVisible(); + } + + // Toggle app off + await appButton.click(); + }); +}); diff --git a/packages/astro/e2e/dev-toolbar.test.js b/packages/astro/e2e/dev-toolbar.test.js index e54e0e154..798e73461 100644 --- a/packages/astro/e2e/dev-toolbar.test.js +++ b/packages/astro/e2e/dev-toolbar.test.js @@ -98,17 +98,18 @@ test.describe('Dev Toolbar', () => { await appButton.click(); const auditCanvas = toolbar.locator('astro-dev-toolbar-app-canvas[data-app-id="astro:audit"]'); - const auditHighlight = auditCanvas.locator('astro-dev-toolbar-highlight'); - await expect(auditHighlight).toBeVisible(); + const auditHighlights = auditCanvas.locator('astro-dev-toolbar-highlight'); + + for (const auditHighlight of await auditHighlights.all()) { + await expect(auditHighlight).toBeVisible(); - await auditHighlight.hover(); - const auditHighlightTooltip = auditHighlight.locator('astro-dev-toolbar-tooltip'); - await expect(auditHighlightTooltip).toBeVisible(); + await auditHighlight.hover(); + const auditHighlightTooltip = auditHighlight.locator('astro-dev-toolbar-tooltip'); + await expect(auditHighlightTooltip).toBeVisible(); + } // Toggle app off await appButton.click(); - await expect(auditHighlight).not.toBeVisible(); - await expect(auditHighlightTooltip).not.toBeVisible(); }); test('audit shows no issues message when there are no issues', async ({ page, astro }) => { @@ -233,4 +234,17 @@ test.describe('Dev Toolbar', () => { await appButton.click(); await expect(myAppWindow).not.toBeVisible(); }); + + test('islands include their server and client render time', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/')); + + const island = page.locator('astro-island'); + await expect(island).toHaveCount(1); + + const serverRenderTime = await island.getAttribute('server-render-time'); + const clientRenderTime = await island.getAttribute('client-render-time'); + + expect(serverRenderTime).not.toBe(null); + expect(clientRenderTime).not.toBe(null); + }); }); diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/light_walrus.avif b/packages/astro/e2e/fixtures/dev-toolbar/src/light_walrus.avif Binary files differnew file mode 100644 index 000000000..89e1c3a14 --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/light_walrus.avif diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audit-no-warning.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audit-no-warning.astro index 9d0c28587..f4791ed76 100644 --- a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audit-no-warning.astro +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audit-no-warning.astro @@ -2,4 +2,4 @@ --- -<img src="https://astro.build/assets/press/astro-logo-dark.svg" alt="Astro logo" /> +<div>Hey, there's no errors here!</div> diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf.astro new file mode 100644 index 000000000..b5c359ada --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf.astro @@ -0,0 +1,10 @@ +--- +import { Image } from "astro:assets"; +import walrus from "../light_walrus.avif"; +--- + +<Image src={walrus} loading="lazy" alt="A walrus" /> + +<div style="height: 9000px;"></div> + +<Image src={walrus} loading="eager" alt="A walrus" /> diff --git a/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits.astro b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits.astro new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits.astro diff --git a/packages/astro/src/runtime/client/dev-toolbar/apps/audit/index.ts b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/index.ts index 639e16269..7ece32850 100644 --- a/packages/astro/src/runtime/client/dev-toolbar/apps/audit/index.ts +++ b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/index.ts @@ -8,6 +8,7 @@ import { } from '../utils/highlight.js'; import { createWindowElement } from '../utils/window.js'; import { a11y } from './a11y.js'; +import { perf } from './perf.js'; const icon = '<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 1 20 16"><path fill="#fff" d="M.6 2A1.1 1.1 0 0 1 1.7.9h16.6a1.1 1.1 0 1 1 0 2.2H1.6A1.1 1.1 0 0 1 .8 2Zm1.1 7.1h6a1.1 1.1 0 0 0 0-2.2h-6a1.1 1.1 0 0 0 0 2.2ZM9.3 13H1.8a1.1 1.1 0 1 0 0 2.2h7.5a1.1 1.1 0 1 0 0-2.2Zm11.3 1.9a1.1 1.1 0 0 1-1.5 0l-1.7-1.7a4.1 4.1 0 1 1 1.6-1.6l1.6 1.7a1.1 1.1 0 0 1 0 1.6Zm-5.3-3.4a1.9 1.9 0 1 0 0-3.8 1.9 1.9 0 0 0 0 3.8Z"/></svg>'; @@ -28,10 +29,20 @@ export interface ResolvedAuditRule { export interface AuditRuleWithSelector extends AuditRule { selector: string; - match?: (element: Element) => boolean | null | undefined | void; + match?: ( + element: Element + ) => + | boolean + | null + | undefined + | void + | Promise<boolean> + | Promise<void> + | Promise<null> + | Promise<undefined>; } -const rules = [...a11y]; +const rules = [...a11y, ...perf]; const dynamicAuditRuleKeys: Array<keyof AuditRule> = ['title', 'message']; function resolveAuditRule(rule: AuditRule, element: Element): ResolvedAuditRule { @@ -93,12 +104,16 @@ export default { matches = Array.from(elements); } else { for (const element of elements) { - if (rule.match(element)) { + if (await rule.match(element)) { matches.push(element); } } } for (const element of matches) { + // Don't audit elements that already have an audit on them + // TODO: This is a naive implementation, it'd be good to show all the audits for an element at the same time. + if (audits.some((audit) => audit.auditedElement === element)) continue; + await createAuditProblem(rule, element); } } @@ -146,10 +161,10 @@ export default { } </style> <header> - <h1><astro-dev-toolbar-icon icon="check-circle"></astro-dev-toolbar-icon>No accessibility issues detected.</h1> + <h1><astro-dev-toolbar-icon icon="check-circle"></astro-dev-toolbar-icon>No accessibility or performance issues detected.</h1> </header> <p> - Nice work! This app scans the page and highlights common accessibility issues for you, like a missing "alt" attribute on an image. + Nice work! This app scans the page and highlights common accessibility and performance issues for you, like a missing "alt" attribute on an image, or a image not using performant attributes. </p> ` ); @@ -197,7 +212,7 @@ export default { } const rect = originalElement.getBoundingClientRect(); - const highlight = createHighlight(rect, 'warning'); + const highlight = createHighlight(rect, 'warning', { 'data-audit-code': rule.code }); const tooltip = buildAuditTooltip(rule, originalElement); // Set the highlight/tooltip as being fixed position the highlighted element diff --git a/packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts new file mode 100644 index 000000000..197553a25 --- /dev/null +++ b/packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts @@ -0,0 +1,125 @@ +import type { AuditRuleWithSelector } from './index.js'; + +// A regular expression to match external URLs +const EXTERNAL_URL_REGEX = /^(?:[a-z+]+:)?\/\//i; + +export const perf: AuditRuleWithSelector[] = [ + { + code: 'perf-use-image-component', + title: 'Use the Image component', + message: 'This image could be replaced with the Image component to improve performance.', + selector: 'img:not([data-image-component])', + async match(element) { + const src = element.getAttribute('src'); + if (!src) return false; + + // Don't match data URIs, they're typically used for specific use-cases that the image component doesn't help with + if (src.startsWith('data:')) return false; + + // Ignore images that are smaller than 20KB, most of the time the image component won't really help with these, or they're used for specific use-cases (pixel tracking, etc.) + // Ignore this test for remote images for now, fetching them can be very slow and possibly dangerous + if (!EXTERNAL_URL_REGEX.test(src)) { + const imageData = await fetch(src).then((response) => response.blob()); + if (imageData.size < 20480) return false; + } + + return true; + }, + }, + { + code: 'perf-use-loading-lazy', + title: 'Use the loading="lazy" attribute', + message: (element) => + `This ${element.nodeName} tag is below the fold and could be lazy-loaded to improve performance.`, + selector: + '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 + if (htmlElement.offsetTop < window.innerHeight) return false; + + return true; + }, + }, + { + code: 'perf-use-loading-eager', + title: 'Use the loading="eager" attribute', + message: (element) => + `This ${element.nodeName} tag is above the fold and could be eagerly-loaded to improve performance.`, + selector: 'img[loading="lazy"], iframe[loading="lazy"]', + match(element) { + const htmlElement = element as HTMLImageElement | HTMLIFrameElement; + + // Ignore elements that are below the fold, they should be loaded lazily + if (htmlElement.offsetTop > window.innerHeight) return false; + + return true; + }, + }, + { + code: 'perf-use-videos', + title: 'Use videos instead of GIFs for large animations', + message: + 'This GIF could be replaced with a video to reduce its file size and improve performance.', + selector: 'img[src$=".gif"]', + async match(element) { + const src = element.getAttribute('src'); + if (!src) return false; + + // Ignore remote URLs + if (EXTERNAL_URL_REGEX.test(src)) return false; + + // Ignore GIFs that are smaller than 100KB, those are typically small enough to not be a problem + if (!EXTERNAL_URL_REGEX.test(src)) { + const imageData = await fetch(src).then((response) => response.blob()); + if (imageData.size < 102400) return false; + } + + return true; + }, + }, + { + code: 'perf-slow-component-server-render', + title: 'Server-rendered component took a long time to render', + message: (element) => + `This component took an unusually long time to render on the server (${getCleanRenderingTime( + element.getAttribute('server-render-time') + )}). This might be a sign that it's doing too much work on the server, or something is blocking rendering.`, + selector: 'astro-island[server-render-time]', + match(element) { + const serverRenderTime = element.getAttribute('server-render-time'); + if (!serverRenderTime) return false; + + const renderingTime = parseFloat(serverRenderTime); + if (Number.isNaN(renderingTime)) return false; + + return renderingTime > 500; + }, + }, + { + code: 'perf-slow-component-client-hydration', + title: 'Client-rendered component took a long time to hydrate', + message: (element) => + `This component took an unusually long time to render on the server (${getCleanRenderingTime( + element.getAttribute('client-render-time') + )}). This could be a sign that something is blocking the main thread and preventing the component from hydrating quickly.`, + selector: 'astro-island[client-render-time]', + match(element) { + const clientRenderTime = element.getAttribute('client-render-time'); + if (!clientRenderTime) return false; + + const renderingTime = parseFloat(clientRenderTime); + if (Number.isNaN(renderingTime)) return false; + + return renderingTime > 500; + }, + }, +]; + +function getCleanRenderingTime(time: string | null) { + if (!time) return 'unknown'; + const renderingTime = parseFloat(time); + if (Number.isNaN(renderingTime)) return 'unknown'; + + return renderingTime.toFixed(2) + 's'; +} diff --git a/packages/astro/src/runtime/client/dev-toolbar/apps/utils/highlight.ts b/packages/astro/src/runtime/client/dev-toolbar/apps/utils/highlight.ts index 1ceb1f4e6..bcf347c8e 100644 --- a/packages/astro/src/runtime/client/dev-toolbar/apps/utils/highlight.ts +++ b/packages/astro/src/runtime/client/dev-toolbar/apps/utils/highlight.ts @@ -1,10 +1,20 @@ import type { DevToolbarHighlight } from '../../ui-library/highlight.js'; import type { Icon } from '../../ui-library/icons.js'; -export function createHighlight(rect: DOMRect, icon?: Icon) { +export function createHighlight( + rect: DOMRect, + icon?: Icon, + additionalAttributes?: Record<string, string> +) { const highlight = document.createElement('astro-dev-toolbar-highlight'); if (icon) highlight.icon = icon; + if (additionalAttributes) { + for (const [key, value] of Object.entries(additionalAttributes)) { + highlight.setAttribute(key, value); + } + } + highlight.tabIndex = 0; if (rect.width === 0 || rect.height === 0) { diff --git a/packages/astro/src/runtime/server/astro-island.ts b/packages/astro/src/runtime/server/astro-island.ts index e1c73a5f0..400aeeb0e 100644 --- a/packages/astro/src/runtime/server/astro-island.ts +++ b/packages/astro/src/runtime/server/astro-island.ts @@ -185,9 +185,17 @@ declare const Astro: { ); throw e; } - await this.hydrator(this)(this.Component, props, slots, { + let hydrationTimeStart; + const hydrator = this.hydrator(this); + if (process.env.NODE_ENV === 'development') hydrationTimeStart = performance.now(); + await hydrator(this.Component, props, slots, { client: this.getAttribute('client'), }); + if (process.env.NODE_ENV === 'development' && hydrationTimeStart) + this.setAttribute( + 'client-render-time', + (performance.now() - hydrationTimeStart).toString() + ); this.removeAttribute('ssr'); this.dispatchEvent(new CustomEvent('astro:hydrate')); }; diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 6d2117545..72484a303 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -184,6 +184,7 @@ async function renderFrameworkComponent( } } + let componentServerRenderEndTime; // If no one claimed the renderer if (!renderer) { if (metadata.hydrate === 'only') { @@ -241,6 +242,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr if (metadata.hydrate === 'only') { html = await renderSlotToString(result, slots?.fallback); } else { + const componentRenderStartTime = performance.now(); ({ html, attrs } = await renderer.ssr.renderToStaticMarkup.call( { result }, Component, @@ -248,6 +250,8 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr children, metadata )); + if (process.env.NODE_ENV === 'development') + componentServerRenderEndTime = performance.now() - componentRenderStartTime; } } @@ -327,6 +331,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr metadata as Required<AstroComponentMetadata> ); + if (componentServerRenderEndTime && process.env.NODE_ENV === 'development') + island.props['server-render-time'] = componentServerRenderEndTime; + // Render template if not all astro fragments are provided. let unrenderedSlots: string[] = []; if (html) { diff --git a/packages/astro/src/runtime/server/scripts.ts b/packages/astro/src/runtime/server/scripts.ts index 2ab368625..76e0326e2 100644 --- a/packages/astro/src/runtime/server/scripts.ts +++ b/packages/astro/src/runtime/server/scripts.ts @@ -1,5 +1,6 @@ import type { SSRResult } from '../../@types/astro.js'; import islandScript from './astro-island.prebuilt.js'; +import islandScriptDev from './astro-island.prebuilt-dev.js'; const ISLAND_STYLES = `<style>astro-island,astro-slot,astro-static-slot{display:contents}</style>`; @@ -36,10 +37,9 @@ export function getPrescripts(result: SSRResult, type: PrescriptType, directive: // deps to be loaded immediately. switch (type) { case 'both': - return `${ISLAND_STYLES}<script>${getDirectiveScriptText( - result, - directive - )};${islandScript}</script>`; + return `${ISLAND_STYLES}<script>${getDirectiveScriptText(result, directive)};${ + process.env.NODE_ENV === 'development' ? islandScriptDev : islandScript + }</script>`; case 'directive': return `<script>${getDirectiveScriptText(result, directive)}</script>`; case null: diff --git a/scripts/cmd/prebuild.js b/scripts/cmd/prebuild.js index 3e206f25e..de3a36910 100644 --- a/scripts/cmd/prebuild.js +++ b/scripts/cmd/prebuild.js @@ -29,12 +29,12 @@ export default async function prebuild(...args) { )) ); - function getPrebuildURL(entryfilepath) { + function getPrebuildURL(entryfilepath, dev = false) { const entryURL = pathToFileURL(entryfilepath); const basename = path.basename(entryfilepath); const ext = path.extname(entryfilepath); const name = basename.slice(0, basename.indexOf(ext)); - const outname = `${name}.prebuilt${ext}`; + const outname = dev ? `${name}.prebuilt-dev${ext}` : `${name}.prebuilt${ext}`; const outURL = new URL('./' + outname, entryURL); return outURL; } @@ -61,7 +61,8 @@ export default async function prebuild(...args) { } tscode = newTscode; } - const esbuildresult = await esbuild.build({ + + const esbuildOptions = { stdin: { contents: tscode, resolveDir: path.dirname(filepath), @@ -73,19 +74,40 @@ export default async function prebuild(...args) { minify, bundle: true, write: false, - }); - const code = esbuildresult.outputFiles[0].text.trim(); - const rootURL = new URL('../../', import.meta.url); - const rel = path.relative(fileURLToPath(rootURL), filepath); - const mod = `/** + }; + + const results = await Promise.all( + [ + { + build: await esbuild.build(esbuildOptions), + dev: false, + }, + filepath.includes('astro-island') + ? { + build: await esbuild.build({ + ...esbuildOptions, + define: { 'process.env.NODE_ENV': '"development"' }, + }), + dev: true, + } + : undefined, + ].filter((entry) => entry) + ); + + for (const result of results) { + const code = result.build.outputFiles[0].text.trim(); + const rootURL = new URL('../../', import.meta.url); + const rel = path.relative(fileURLToPath(rootURL), filepath); + const mod = `/** * This file is prebuilt from ${rel} * Do not edit this directly, but instead edit that file and rerun the prebuild * to generate this file. */ export default \`${escapeTemplateLiterals(code)}\`;`; - const url = getPrebuildURL(filepath); - await fs.promises.writeFile(url, mod, 'utf-8'); + const url = getPrebuildURL(filepath, result.dev); + await fs.promises.writeFile(url, mod, 'utf-8'); + } } await Promise.all(entryPoints.map(prebuildFile)); |
