feat(provider): Add umbraco image provider#2165
Conversation
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2165 +/- ##
=======================================
Coverage 32.52% 32.52%
=======================================
Files 7 7
Lines 372 372
Branches 131 131
=======================================
Hits 121 121
Misses 194 194
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jemomi
changed the title
📝 WalkthroughWalkthroughA new Umbraco image provider is added to the Nuxt Image module. The implementation includes a runtime provider that maps Nuxt Image modifiers to Umbraco/ImageSharp query parameters, provider registration in the built-in providers list, playground configuration and examples, comprehensive documentation explaining the provider's usage and modifier mappings, and corresponding unit/snapshot tests. All changes are additions with no modifications to existing code. Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
test/providers.ts (1)
43-43: Consider adding fixture cases for Umbraco-specific modifiers in this unit matrix.
focalPointXY(rxy),quality,sampler, andanchorPositionmappings are not covered here, so regressions in those mappings rely mainly on e2e snapshots.Also applies to: 86-86, 128-128, 170-170, 212-212, 254-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/providers.ts` at line 43, The test matrix entry using umbracoImage lacks coverage for Umbraco-specific modifiers; add fixture cases that include focalPointXY (rxy), quality, sampler, and anchorPosition to the unit test matrix for the umbracoImage variants so mappings are validated individually. Locate the umbracoImage test entries (symbol: umbracoImage) in test/providers.ts and augment the matrix with separate cases covering rxy values, different quality levels, explicit sampler names, and anchorPosition values mirroring the e2e scenarios; ensure each new fixture asserts the expected mapped output so regressions in the mapping functions are caught by unit tests.src/runtime/providers/umbracoImage.ts (1)
20-21:defaultModifiers+defuis currently a no-op.
defaultModifiersis empty, so this merge adds overhead without changing behavior.♻️ Optional simplification
-const defaultModifiers = {} @@ - const mergedModifiers = defu(normalizedModifiers, defaultModifiers) - const operations = operationsGenerator(mergedModifiers) + const operations = operationsGenerator(normalizedModifiers)Also applies to: 40-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/umbracoImage.ts` around lines 20 - 21, defaultModifiers is an empty object so merging it with defu is a no-op — remove the unnecessary merge: replace any defu(modifiers, defaultModifiers) usage with either the incoming modifiers (or modifiers ?? {}) or supply actual meaningful default values in defaultModifiers; also remove the now-unused defaultModifiers constant and any unused defu import/usage (references: defaultModifiers, defu).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/3.providers/umbracoImage.md`:
- Line 185: The heading "HMAC protected image URLs" should use the hyphenated
adjective form; update the heading text (the line containing "HMAC protected
image URLs" / the "HMAC protected" phrase) to "HMAC-protected image URLs" so the
documentation uses correct grammar and style.
- Line 7: Replace the incorrect fork URL used in the Markdown link (the "to:
https://github.com/jemomi/nuxt-image/blob/main/src/runtime/providers/umbracoImage.ts"
reference) with the canonical repository path
"https://github.com/nuxt/image/blob/main/src/runtime/providers/umbracoImage.ts"
so the Source link in docs/content/3.providers/umbracoImage.md points to the
main project and avoids stale/broken references.
In `@playground/app/providers.ts`:
- Line 1405: Replace the external Umbraco image URLs used in the sample objects
(the src fields for the Umbraco sample images) with a stable, repo-controlled
static fixture image and ensure the sample objects point to that fixture; then
update test/e2e/__snapshots__/umbracoImage.json5 to match the new fixture
URL/data so E2E snapshot tests are not dependent on third-party hosts.
In `@src/runtime/providers/umbracoImage.ts`:
- Around line 31-41: In getImage, avoid assuming modifiers exists and don't
mutate the caller's object: default the parameter to an empty object (handle
undefined) and create a new local copy (e.g., mergedModifiers) before
adjustments; perform the 'contain' -> 'crop' remap on that local copy (not on
the incoming modifiers) and then pass mergedModifiers into operationsGenerator
and defu with defaultModifiers so the original modifiers remains unchanged and
no runtime error occurs when modifiers is omitted.
---
Nitpick comments:
In `@src/runtime/providers/umbracoImage.ts`:
- Around line 20-21: defaultModifiers is an empty object so merging it with defu
is a no-op — remove the unnecessary merge: replace any defu(modifiers,
defaultModifiers) usage with either the incoming modifiers (or modifiers ?? {})
or supply actual meaningful default values in defaultModifiers; also remove the
now-unused defaultModifiers constant and any unused defu import/usage
(references: defaultModifiers, defu).
In `@test/providers.ts`:
- Line 43: The test matrix entry using umbracoImage lacks coverage for
Umbraco-specific modifiers; add fixture cases that include focalPointXY (rxy),
quality, sampler, and anchorPosition to the unit test matrix for the
umbracoImage variants so mappings are validated individually. Locate the
umbracoImage test entries (symbol: umbracoImage) in test/providers.ts and
augment the matrix with separate cases covering rxy values, different quality
levels, explicit sampler names, and anchorPosition values mirroring the e2e
scenarios; ensure each new fixture asserts the expected mapped output so
regressions in the mapping functions are caught by unit tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5deee012-f6af-46d2-8282-c7fa75e37769
📒 Files selected for processing (8)
docs/content/3.providers/umbracoImage.mdplayground/app/providers.tsplayground/nuxt.config.tssrc/provider.tssrc/runtime/providers/umbracoImage.tstest/e2e/__snapshots__/umbracoImage.json5test/nuxt/providers.test.tstest/providers.ts
| links: | ||
| - label: Source | ||
| icon: i-simple-icons-github | ||
| to: https://github.com/jemomi/nuxt-image/blob/main/src/runtime/providers/umbracoImage.ts |
There was a problem hiding this comment.
Point Source to the canonical repository path.
Line 7 currently links to a fork URL. Use the main project URL (https://github.com/nuxt/image/blob/main/src/runtime/providers/umbracoImage.ts) to avoid stale/broken references.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/3.providers/umbracoImage.md` at line 7, Replace the incorrect
fork URL used in the Markdown link (the "to:
https://github.com/jemomi/nuxt-image/blob/main/src/runtime/providers/umbracoImage.ts"
reference) with the canonical repository path
"https://github.com/nuxt/image/blob/main/src/runtime/providers/umbracoImage.ts"
so the Source link in docs/content/3.providers/umbracoImage.md points to the
main project and avoids stale/broken references.
|
|
||
| Umbraco supports using set crops for images. These however, aren't currently supported. | ||
|
|
||
| ### HMAC protected image URLs |
There was a problem hiding this comment.
Use hyphenated form: “HMAC-protected”.
Minor docs grammar fix on Line 185.
🧰 Tools
🪛 LanguageTool
[grammar] ~185-~185: Use a hyphen to join words.
Context: ...r, aren't currently supported. ### HMAC protected image URLs If your Umbraco se...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/3.providers/umbracoImage.md` at line 185, The heading "HMAC
protected image URLs" should use the hyphenated adjective form; update the
heading text (the line containing "HMAC protected image URLs" / the "HMAC
protected" phrase) to "HMAC-protected image URLs" so the documentation uses
correct grammar and style.
| name: 'umbracoImage', | ||
| samples: [ | ||
| { | ||
| src: 'https://umbraco.com/media/hvjlhtfw/home-full-screen-4.png', |
There was a problem hiding this comment.
Use a stable fixture source for samples to avoid flaky E2E snapshots.
These lines depend on a third-party image URL that may disappear, which can break snapshot-based tests unexpectedly. Prefer a repo-owned/static fixture (or other controlled asset) and update test/e2e/__snapshots__/umbracoImage.json5 accordingly.
Also applies to: 1412-1412, 1419-1419
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/app/providers.ts` at line 1405, Replace the external Umbraco image
URLs used in the sample objects (the src fields for the Umbraco sample images)
with a stable, repo-controlled static fixture image and ensure the sample
objects point to that fixture; then update
test/e2e/__snapshots__/umbracoImage.json5 to match the new fixture URL/data so
E2E snapshot tests are not dependent on third-party hosts.
| getImage: (src, { | ||
| modifiers, | ||
| baseURL = '', | ||
| }) => { | ||
| // modifier.fit - 'contain' is remapped to use 'crop', since this is the value used by ImageSharp. | ||
| if (modifiers.fit === 'contain') { | ||
| modifiers.fit = 'crop' | ||
| } | ||
|
|
||
| const mergedModifiers = defu(modifiers, defaultModifiers) | ||
| const operations = operationsGenerator(mergedModifiers) |
There was a problem hiding this comment.
Handle optional modifiers safely and avoid mutating input.
Line 36 assumes modifiers is always defined, which can throw when getImage is called without modifiers. Line 37 also mutates caller-provided input.
🔧 Proposed fix
export default defineProvider<UmbracoImageOptions>({
getImage: (src, {
modifiers,
baseURL = '',
}) => {
+ const normalizedModifiers = { ...(modifiers || {}) }
+
// modifier.fit - 'contain' is remapped to use 'crop', since this is the value used by ImageSharp.
- if (modifiers.fit === 'contain') {
- modifiers.fit = 'crop'
+ if (normalizedModifiers.fit === 'contain') {
+ normalizedModifiers.fit = 'crop'
}
- const mergedModifiers = defu(modifiers, defaultModifiers)
+ const mergedModifiers = defu(normalizedModifiers, defaultModifiers)
const operations = operationsGenerator(mergedModifiers)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getImage: (src, { | |
| modifiers, | |
| baseURL = '', | |
| }) => { | |
| // modifier.fit - 'contain' is remapped to use 'crop', since this is the value used by ImageSharp. | |
| if (modifiers.fit === 'contain') { | |
| modifiers.fit = 'crop' | |
| } | |
| const mergedModifiers = defu(modifiers, defaultModifiers) | |
| const operations = operationsGenerator(mergedModifiers) | |
| getImage: (src, { | |
| modifiers, | |
| baseURL = '', | |
| }) => { | |
| const normalizedModifiers = { ...(modifiers || {}) } | |
| // modifier.fit - 'contain' is remapped to use 'crop', since this is the value used by ImageSharp. | |
| if (normalizedModifiers.fit === 'contain') { | |
| normalizedModifiers.fit = 'crop' | |
| } | |
| const mergedModifiers = defu(normalizedModifiers, defaultModifiers) | |
| const operations = operationsGenerator(mergedModifiers) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/providers/umbracoImage.ts` around lines 31 - 41, In getImage,
avoid assuming modifiers exists and don't mutate the caller's object: default
the parameter to an empty object (handle undefined) and create a new local copy
(e.g., mergedModifiers) before adjustments; perform the 'contain' -> 'crop'
remap on that local copy (not on the incoming modifiers) and then pass
mergedModifiers into operationsGenerator and defu with defaultModifiers so the
original modifiers remains unchanged and no runtime error occurs when modifiers
is omitted.
2 participants
🔗 Linked issue
Resolves discussion: #1629
📚 Description
As requested in aforementioned discussion. Here is provider for Umbraco images.
Use cases in regards to "generate on demand" images are implemented. But currently no support is or will be made for set crop definitions (Because i currently do not have a setup that uses set crops).
Provider could potentially be renamed to "imageSharp", but since I expect most use cases will be from umbraco, I kept it as "umbracoImage"
The image used for testing, is one that is currently present on the frontpage of umbraco's website. Full disclosure, Umbraco has nothing to do with the development of this provider, and har not been informed that an image from their page is being used for these tests. So it might disappear. But I will try and get into contact with them to see if I can get a stable image source from them.