[PR #662] [MERGED] fix(image): strip priority prop before forwarding to UnpicImage to prevent DOM leak #751

New issue

Closed

opened 2026-05-06 13:09:56 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:09:56 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/662
Author: @james-elicx
Created: 3/22/2026
Status: ✅ Merged
Merged: 3/23/2026
Merged by: @james-elicx

Base: main ← Head: fix/priority-prop-dom-leak

📝 Commits (2)

68ad9f1 fix(image): strip priority prop before forwarding to UnpicImage to prevent DOM leak
5a8a0b1 test: add 50ms slack to compressed streaming timing assertion to fix CI flakiness

📊 Changes

3 files changed (+105 additions, -3 deletions)

View changed files

📝 packages/vinext/src/shims/image.tsx (+9 -2)
📝 tests/features.test.ts (+2 -1)
📝 tests/image-component.test.ts (+94 -0)

📄 Description

Problem

When using `next/image` with a remote URL and either `fill` or explicit `width`+`height`, the `priority` boolean prop was forwarded directly to `@unpic/react`'s `Image` component. Although `@unpic/core` is supposed to strip it via `transformSharedProps`, in practice it leaked through to the DOM `` element, triggering:

```
Received `true` for a non-boolean attribute `priority`.
If you want to write it to the DOM, pass a string instead: priority="true" or priority={value.toString()}.
```

The two affected code paths were both in `packages/vinext/src/shims/image.tsx`:

Remote URL + `fill` (fullWidth layout, line 282)
Remote URL + `width`+`height` (constrained layout, line 299)

Fix

Replace `priority={priority}` on both `` call sites with the equivalent HTML semantics that the local-image and custom-loader paths already use correctly:

```tsx
loading={priority ? "eager" : (loading ?? "lazy")}
fetchPriority={priority ? "high" : undefined}
```

`priority` is a Next.js-specific concept that has no equivalent HTML attribute. It must always be translated to `loading="eager"` + `fetchPriority="high"` before reaching the DOM.

Reproduction

Ten new tests added in `tests/image-component.test.ts` under `"priority prop — no DOM leak on remote URL paths"`:

Assert `priority=` does not appear in SSR output for both affected paths
Assert `loading="eager"` is present when `priority=true` (both paths)
Assert `fetchPriority="high"` is present when `priority=true`
Assert default `loading="lazy"` when `priority` is not set

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/662 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 3/22/2026 **Status:** ✅ Merged **Merged:** 3/23/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/priority-prop-dom-leak` --- ### 📝 Commits (2) - [`68ad9f1`](https://github.com/cloudflare/vinext/commit/68ad9f19ff00c75a524e2a54c46735ee3a5217f1) fix(image): strip priority prop before forwarding to UnpicImage to prevent DOM leak - [`5a8a0b1`](https://github.com/cloudflare/vinext/commit/5a8a0b1c789b07676d258cd7535a3f37257a7d9c) test: add 50ms slack to compressed streaming timing assertion to fix CI flakiness ### 📊 Changes **3 files changed** (+105 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/shims/image.tsx` (+9 -2) 📝 `tests/features.test.ts` (+2 -1) 📝 `tests/image-component.test.ts` (+94 -0) </details> ### 📄 Description ## Problem When using \`next/image\` with a remote URL and either \`fill\` or explicit \`width\`+\`height\`, the \`priority\` boolean prop was forwarded directly to \`@unpic/react\`'s \`Image\` component. Although \`@unpic/core\` is supposed to strip it via \`transformSharedProps\`, in practice it leaked through to the DOM \`<img>\` element, triggering: \`\`\` Received \`true\` for a non-boolean attribute \`priority\`. If you want to write it to the DOM, pass a string instead: priority="true" or priority={value.toString()}. \`\`\` The two affected code paths were both in \`packages/vinext/src/shims/image.tsx\`: - Remote URL + \`fill\` (fullWidth layout, line 282) - Remote URL + \`width\`+\`height\` (constrained layout, line 299) ## Fix Replace \`priority={priority}\` on both \`<UnpicImage>\` call sites with the equivalent HTML semantics that the local-image and custom-loader paths already use correctly: \`\`\`tsx loading={priority ? "eager" : (loading ?? "lazy")} fetchPriority={priority ? "high" : undefined} \`\`\` \`priority\` is a Next.js-specific concept that has no equivalent HTML attribute. It must always be translated to \`loading="eager"\` + \`fetchPriority="high"\` before reaching the DOM. ## Reproduction Ten new tests added in \`tests/image-component.test.ts\` under \`"priority prop — no DOM leak on remote URL paths"\`: - Assert \`priority=\` does not appear in SSR output for both affected paths - Assert \`loading="eager"\` is present when \`priority=true\` (both paths) - Assert \`fetchPriority="high"\` is present when \`priority=true\` - Assert default \`loading="lazy"\` when \`priority\` is not set --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>