Add image loading hints#1391
Conversation
🦋 Changeset detectedLatest commit: f06430c The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
All contributors have signed the CLA ✍️ ✅ |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
This PR adds fetchpriority="high" to the EmDashImage component when the priority prop is set, pairing it with the existing loading="eager" behavior. This is a focused, backwards-compatible performance improvement.
Approach: The approach is sound. fetchpriority is the standard HTML attribute for giving browsers a priority hint on image requests, and applying it only to images explicitly marked priority is the correct pattern. The author initially experimented with a data-early-lazy-load attribute on lazy images but wisely removed it in a follow-up commit, keeping the scope tight.
What I checked:
- Read the full diff and every changed file.
- Verified
fetchpriorityis only set whenpriorityis truthy, and falls back toundefined(omitted) otherwise — consistent with established patterns in the codebase (e.g.,target={blank ? "_blank" : undefined}inLink.astro). - Confirmed
EmDashImageis the publicImagecomponent exported fromemdash/ui, which is what theme authors use directly. - Reviewed sibling image components (
Image.astrofor Portable Text,EmDashMedia.astro,Gallery.astro,Cover.astro) and confirmed they either don't expose apriorityprop by design (Portable Text blocks) or are outside the PR's stated scope. - Checked docs changes: the updated examples correctly switch from
astro:assetstoemdash/ui'sImage, remove the non-existentfeatured_image_altfield, and add responsible guidance about usingpriorityonly for above-the-fold images. - Verified the changeset is present and correctly describes the change.
Conclusion: No logic bugs, no regressions, no AGENTS.md violations. The implementation is clean and the docs are accurate. LGTM.
| {post?.data.featured_image && ( | ||
| <Image | ||
| src={post.data.featured_image} | ||
| alt={post.data.featured_image_alt ?? ""} |
There was a problem hiding this comment.
@mvvmm Should we be concerned about dropping this line?
What does this PR do?
Adds image loading hints to EmDash image components so consuming sites can prioritize above-the-fold images and opt into early promotion of deferred lazy images after initial page load.
Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
pnpm format:checkpnpm --filter emdash typecheck