feat(provider): add Tencent Cloud EdgeOne Pages image provider#2173
Open
q153877011 wants to merge 2 commits intonuxt:mainfrom
Open
feat(provider): add Tencent Cloud EdgeOne Pages image provider#2173q153877011 wants to merge 2 commits intonuxt:mainfrom
q153877011 wants to merge 2 commits intonuxt:mainfrom
Conversation
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new EdgeOne Pages image provider named Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
playground/app/pages/tencent-cloud.vue (1)
254-258: Duplicated fitMap constant — consider importing from provider.This
fitMapduplicates the constant defined insrc/runtime/providers/tencentCloud.ts(lines 37-43). While acceptable for a playground demo, any future changes to fit mode mappings could cause inconsistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/pages/tencent-cloud.vue` around lines 254 - 258, The fitMap constant in this file duplicates the mapping defined in the provider (the fitMap in src/runtime/providers/tencentCloud.ts); replace the local const fitMap with an import from the provider to avoid divergence: remove the local fitMap definition and import the provider's exported fitMap (or the provider module that exposes it) and use that symbol wherever fitMap is referenced in this component so future updates to the provider mapping stay consistent.src/runtime/providers/tencentCloud.ts (1)
157-159: Blur value of 0 is filtered out — verify if intentional.The condition
typeof blur !== 'undefined' && blurfilters outblur=0. If a user explicitly setsblur: 0to disable blur, it works as expected. However, if the Tencent Cloud API acceptsblur/0x0as a valid operation (e.g., to reset), this would silently skip it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/tencentCloud.ts` around lines 157 - 159, The current check `typeof blur !== 'undefined' && blur` drops falsy values like 0, so an explicit blur: 0 is skipped; update the guard around the variable blur used before calling operations.push (the blur/${blur}x${blur} push) to only exclude undefined/null rather than all falsy values — e.g., test for blur !== undefined && blur !== null or use Number.isFinite(blur) / typeof blur !== 'undefined' && blur !== null so that blur=0 is accepted and still pushed to operations.docs/content/3.providers/tencent-cloud.md (2)
141-149: Consider adding references for all documented advanced modifiers.You already cite core imageMogr2 docs; adding links for
auto-orient,interlace, andstrip(if available) would make this page fully self-contained.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/3.providers/tencent-cloud.md` around lines 141 - 149, The References section (## References) currently lists imageMogr2 topics but is missing links for the advanced modifiers; add official COS imageMogr2 documentation URLs for the `auto-orient`, `interlace`, and `strip` modifiers (or the closest COS docs if exact pages differ) to the References list so the page includes direct links for those modifiers alongside the existing scaling/cropping/rotation entries.
46-52: Documentinsideandoutsidefit aliases explicitly.The PR behavior includes
insideandoutside, but this table currently documents onlycontain,cover, andfill. Adding those aliases here will prevent ambiguity.📚 Suggested doc patch
-| `fit` | thumbnail suffix | `contain` → `<W>x<H>`, `cover` → `!<W>x<H>r`, `fill` → `<W>x<H>!` | +| `fit` | thumbnail suffix | `contain` / `inside` → `<W>x<H>`, `cover` / `outside` → `!<W>x<H>r`, `fill` → `<W>x<H>!` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/3.providers/tencent-cloud.md` around lines 46 - 52, Update the modifiers table entry for `fit` in the NuxtImg → COS mapping to explicitly document the `inside` and `outside` aliases: state that `inside` is an alias for `contain` (maps to `<W>x<H>`), and `outside` is an alias for `cover` (maps to `!<W>x<H>r`), alongside the existing `contain`, `cover`, and `fill` descriptions; edit the `fit` row in the table so readers see the alias names and their corresponding COS thumbnail suffixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playground/app/pages/tencent-cloud.vue`:
- Around line 231-239: The watchEffect that mutates runtimeConfig
(runtimeConfig, watchEffect, baseURL) won't change the NuxtImg provider used by
NuxtImg because providers are resolved at module init (see provider resolution
in src/runtime/image.ts); either update the playground to state this is
preview-only (document that NuxtImg uses nuxt.config.ts and optimizedUrl is only
illustrative) or switch to calling the $img composable directly with explicit
provider options (use $img({ provider: 'tencentCloud', baseURL: baseURL.value,
... }) or construct the full optimizedUrl manually) so changes to baseURL.value
affect generated URLs immediately.
In `@test/e2e/__snapshots__/tencentCloud.json5`:
- Around line 1-8: The snapshot in test/e2e/__snapshots__/tencentCloud.json5
only contains one URL but the tencentCloud provider defines 6 samples (see
provider.samples in playground/app/providers.ts), so regenerate or update the
snapshot to include all six expected sample URLs (contain, cover, fill, webp
quality/format, rotate, quality/blur) so the e2e test that asserts images.length
=== provider.samples.length will match; update the JSON5 array "requests" and
"sources" to include the six URLs listed in the review comment.
---
Nitpick comments:
In `@docs/content/3.providers/tencent-cloud.md`:
- Around line 141-149: The References section (## References) currently lists
imageMogr2 topics but is missing links for the advanced modifiers; add official
COS imageMogr2 documentation URLs for the `auto-orient`, `interlace`, and
`strip` modifiers (or the closest COS docs if exact pages differ) to the
References list so the page includes direct links for those modifiers alongside
the existing scaling/cropping/rotation entries.
- Around line 46-52: Update the modifiers table entry for `fit` in the NuxtImg →
COS mapping to explicitly document the `inside` and `outside` aliases: state
that `inside` is an alias for `contain` (maps to `<W>x<H>`), and `outside` is an
alias for `cover` (maps to `!<W>x<H>r`), alongside the existing `contain`,
`cover`, and `fill` descriptions; edit the `fit` row in the table so readers see
the alias names and their corresponding COS thumbnail suffixes.
In `@playground/app/pages/tencent-cloud.vue`:
- Around line 254-258: The fitMap constant in this file duplicates the mapping
defined in the provider (the fitMap in src/runtime/providers/tencentCloud.ts);
replace the local const fitMap with an import from the provider to avoid
divergence: remove the local fitMap definition and import the provider's
exported fitMap (or the provider module that exposes it) and use that symbol
wherever fitMap is referenced in this component so future updates to the
provider mapping stay consistent.
In `@src/runtime/providers/tencentCloud.ts`:
- Around line 157-159: The current check `typeof blur !== 'undefined' && blur`
drops falsy values like 0, so an explicit blur: 0 is skipped; update the guard
around the variable blur used before calling operations.push (the
blur/${blur}x${blur} push) to only exclude undefined/null rather than all falsy
values — e.g., test for blur !== undefined && blur !== null or use
Number.isFinite(blur) / typeof blur !== 'undefined' && blur !== null so that
blur=0 is accepted and still pushed to operations.
🪄 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: 15bb0cfc-101f-47c6-8403-e002df4f5c9b
📒 Files selected for processing (9)
docs/content/3.providers/tencent-cloud.mdplayground/app/pages/tencent-cloud.vueplayground/app/providers.tsplayground/nuxt.config.tssrc/provider.tssrc/runtime/providers/tencentCloud.tstest/e2e/__snapshots__/tencentCloud.json5test/nuxt/providers.test.tstest/providers.ts
Comment on lines
+231
to
+239
| watchEffect(() => { | ||
| const cfg = runtimeConfig as any | ||
| cfg.public = cfg.public || {} | ||
| cfg.public.image = cfg.public.image || {} | ||
| cfg.public.image.providers = cfg.public.image.providers || {} | ||
| cfg.public.image.providers.tencentCloud = { | ||
| baseURL: baseURL.value, | ||
| } | ||
| }) |
There was a problem hiding this comment.
Runtime config mutation won't affect NuxtImg provider behavior.
The watchEffect attempts to dynamically update the tencentCloud provider's baseURL via runtimeConfig. However, per the provider architecture in src/runtime/image.ts (lines 109-115), providers are resolved from a pre-initialized context (ctx.options.providers) at module load time, not from runtime config mutations.
This means:
- The
NuxtImgcomponent will always use thebaseURLconfigured innuxt.config.ts - Changes to
baseURL.valuein this component won't affect the generated URLs
The playground appears to work only because nuxt.config.ts already defines a default baseURL for tencentCloud.
Consider either:
- Documenting that this is a preview-only page showing what URLs would look like (the
optimizedUrlcomputed works correctly) - Or using the
$imgcomposable directly with explicit options if dynamic baseURL is needed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/app/pages/tencent-cloud.vue` around lines 231 - 239, The
watchEffect that mutates runtimeConfig (runtimeConfig, watchEffect, baseURL)
won't change the NuxtImg provider used by NuxtImg because providers are resolved
at module init (see provider resolution in src/runtime/image.ts); either update
the playground to state this is preview-only (document that NuxtImg uses
nuxt.config.ts and optimizedUrl is only illustrative) or switch to calling the
$img composable directly with explicit provider options (use $img({ provider:
'tencentCloud', baseURL: baseURL.value, ... }) or construct the full
optimizedUrl manually) so changes to baseURL.value affect generated URLs
immediately.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
playground/app/pages/tencent-cloud.vue (2)
231-239: Runtime config mutation has no effect on NuxtImg provider.This
watchEffectmutatesruntimeConfigto updatebaseURL, but providers are initialized at module load time fromnuxt.config.ts, not from runtime config mutations. TheNuxtImgcomponent will always use the statically configuredbaseURL.The playground still works because
nuxt.config.tsdefines a defaultbaseURL, and theoptimizedUrlcomputed correctly shows what the URL would look like.Consider adding a comment clarifying this is for demonstration only, or removing the ineffective mutation:
♻️ Option 1: Document the limitation
watchEffect(() => { + // Note: This mutation is for demonstration purposes only. + // NuxtImg uses the baseURL from nuxt.config.ts at build time; + // changing this at runtime does not affect the actual NuxtImg URLs. const cfg = runtimeConfig as any cfg.public = cfg.public || {}♻️ Option 2: Remove ineffective code
-watchEffect(() => { - const cfg = runtimeConfig as any - cfg.public = cfg.public || {} - cfg.public.image = cfg.public.image || {} - cfg.public.image.providers = cfg.public.image.providers || {} - cfg.public.image.providers.tencentCloud = { - baseURL: baseURL.value, - } -})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/pages/tencent-cloud.vue` around lines 231 - 239, The watchEffect block that mutates runtimeConfig (watchEffect -> runtimeConfig) to set cfg.public.image.providers.tencentCloud.baseURL does not affect the NuxtImg provider (NuxtImg) because providers are initialized from nuxt.config.ts at module load time; remove the ineffective mutation or replace it with a clear inline comment stating this is demonstration-only and will not change NuxtImg behavior, and instead document that the true provider baseURL must be configured in nuxt.config.ts (or reloaded at build-time) — update or remove the watchEffect that sets baseURL and ensure the optimizedUrl computed remains only for illustration.
254-290: Code duplication with provider implementation.The
fitMapand URL-building logic inoptimizedUrlduplicate the implementation insrc/runtime/providers/tencentCloud.ts. If the provider logic changes, this playground page needs to be updated separately.For a playground demo, this is acceptable since it allows showing the URL without roundtripping through the provider. Just be aware of the maintenance overhead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/pages/tencent-cloud.vue` around lines 254 - 290, The playground duplicates Tencent Cloud URL-building logic (see fitMap and optimizedUrl); instead of duplicating, call the provider's canonical URL-builder (the exported Tencent Cloud provider function that constructs optimized image URLs) or extract the shared logic into a utility and import it; update optimizedUrl to delegate to that shared function and remove the local fitMap and inline ops construction so changes in the provider are reflected automatically.src/runtime/providers/tencentCloud.ts (1)
48-55: Consider validating hex color format.The function assumes valid hex input but doesn't validate it. Invalid colors (e.g.,
"notacolor") will be encoded and sent to COS, potentially causing API errors or unexpected behavior.♻️ Optional: Add hex validation
function encodeCosColor(color: string): string { const hex = color.startsWith('#') ? color : `#${color}` + if (!/^#[0-9A-Fa-f]{3,8}$/.test(hex)) { + console.warn(`[tencentCloud] Invalid hex color: ${color}`) + } if (typeof globalThis.btoa === 'function') { return globalThis.btoa(hex).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') } return Buffer.from(hex).toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/tencentCloud.ts` around lines 48 - 55, The encodeCosColor function currently encodes any string even if it's not a valid hex color; add validation to ensure the input (with or without leading '#') matches a hex color pattern (allowing 3 or 6 hex digits, optionally 8 if you want alpha) before encoding; if the value is invalid, either throw a descriptive error (e.g., "Invalid hex color: <value>") or normalize to a safe default like "#000000" and then continue to perform the existing Base64 URL-safe encoding logic. Use the function name encodeCosColor to locate the code and apply a regex test (e.g., /^#?([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6})$/) to validate the input, trimming/adding '#' as currently done before encoding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@playground/app/pages/tencent-cloud.vue`:
- Around line 231-239: The watchEffect block that mutates runtimeConfig
(watchEffect -> runtimeConfig) to set
cfg.public.image.providers.tencentCloud.baseURL does not affect the NuxtImg
provider (NuxtImg) because providers are initialized from nuxt.config.ts at
module load time; remove the ineffective mutation or replace it with a clear
inline comment stating this is demonstration-only and will not change NuxtImg
behavior, and instead document that the true provider baseURL must be configured
in nuxt.config.ts (or reloaded at build-time) — update or remove the watchEffect
that sets baseURL and ensure the optimizedUrl computed remains only for
illustration.
- Around line 254-290: The playground duplicates Tencent Cloud URL-building
logic (see fitMap and optimizedUrl); instead of duplicating, call the provider's
canonical URL-builder (the exported Tencent Cloud provider function that
constructs optimized image URLs) or extract the shared logic into a utility and
import it; update optimizedUrl to delegate to that shared function and remove
the local fitMap and inline ops construction so changes in the provider are
reflected automatically.
In `@src/runtime/providers/tencentCloud.ts`:
- Around line 48-55: The encodeCosColor function currently encodes any string
even if it's not a valid hex color; add validation to ensure the input (with or
without leading '#') matches a hex color pattern (allowing 3 or 6 hex digits,
optionally 8 if you want alpha) before encoding; if the value is invalid, either
throw a descriptive error (e.g., "Invalid hex color: <value>") or normalize to a
safe default like "#000000" and then continue to perform the existing Base64
URL-safe encoding logic. Use the function name encodeCosColor to locate the code
and apply a regex test (e.g., /^#?([A-Fa-f0-9]{3}|[A-Fa-f0-9]{6})$/) to validate
the input, trimming/adding '#' as currently done before encoding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 963cf513-3448-4313-9994-1f58617760bd
📒 Files selected for processing (9)
docs/content/3.providers/tencent-cloud.mdplayground/app/pages/tencent-cloud.vueplayground/app/providers.tsplayground/nuxt.config.tssrc/provider.tssrc/runtime/providers/tencentCloud.tstest/e2e/__snapshots__/tencentCloud.json5test/nuxt/providers.test.tstest/providers.ts
✅ Files skipped from review due to trivial changes (1)
- docs/content/3.providers/tencent-cloud.md
🚧 Files skipped from review as they are similar to previous changes (4)
- playground/nuxt.config.ts
- test/providers.ts
- playground/app/providers.ts
- test/nuxt/providers.test.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2173 +/- ##
=======================================
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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/nuxt/providers.test.ts (1)
584-617: Please coverinside/outsideexplicitly.These tests exercise
contain,cover, andfill, but not the documented alias valuesinsideandoutside. Adding two small expectations here would keep those mappings from regressing independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/nuxt/providers.test.ts` around lines 584 - 617, Add explicit expectations for the `inside` and `outside` alias mappings in the same test block that calls edgeonePages().getImage so the aliases don't regress: call edgeonePages().getImage with modifiers.fit = 'inside' and assert it matches the same URL as 'contain' (thumbnail/200x200) and call modifiers.fit = 'outside' and assert it matches the same URL as 'cover' (thumbnail/!300x200r) (use the same providerOptions/getEmptyContext pattern used in the existing tests).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/nuxt/providers.test.ts`:
- Line 35: Create a new provider implementation file named
src/runtime/providers/edgeonePages.ts that exports the edgeonePages provider
matching the shape and conventions used by the other providers in the directory
(same export name edgeonePages and same factory/signature pattern). Mirror the
implementation details (default export or named export as used elsewhere,
hooks/handlers, and any options interface) so it registers consistently with the
existing provider registration in src/provider.ts and ensures the build emits
../../dist/runtime/providers/edgeonePages for the test import; follow other
provider files for imports/exports and TypeScript typings to avoid build errors.
---
Nitpick comments:
In `@test/nuxt/providers.test.ts`:
- Around line 584-617: Add explicit expectations for the `inside` and `outside`
alias mappings in the same test block that calls edgeonePages().getImage so the
aliases don't regress: call edgeonePages().getImage with modifiers.fit =
'inside' and assert it matches the same URL as 'contain' (thumbnail/200x200) and
call modifiers.fit = 'outside' and assert it matches the same URL as 'cover'
(thumbnail/!300x200r) (use the same providerOptions/getEmptyContext pattern used
in the existing 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: 0a03f280-fc9e-4c47-ad78-b0f148d721b2
📒 Files selected for processing (7)
docs/content/3.providers/edgeone-pages.mdplayground/app/pages/edgeone-pages.vueplayground/app/providers.tsplayground/nuxt.config.tssrc/provider.tstest/e2e/__snapshots__/edgeOnePages.json5test/nuxt/providers.test.ts
✅ Files skipped from review due to trivial changes (4)
- playground/nuxt.config.ts
- test/e2e/snapshots/edgeOnePages.json5
- playground/app/providers.ts
- docs/content/3.providers/edgeone-pages.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/provider.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/nuxt/providers.test.ts (1)
593-729: Good test coverage; consider adding edge case tests.The tests thoroughly cover fit modes, individual modifiers, and combined operations. For completeness, consider adding tests for:
dx/dyoffsets with croppadmodifier with background color- Error handling when
baseURLis missing💡 Optional: Additional edge case tests
it('edgeonePages crop with offsets', () => { const providerOptions = { baseURL: 'https://nuxt-mix-template.edgeone.site', } const generated = edgeonePages().getImage('/ssg-img.png', { modifiers: { crop: '300x400', gravity: 'northwest', dx: 10, dy: 20 }, ...providerOptions, }, getEmptyContext()) expect(generated).toMatchObject({ url: 'https://nuxt-mix-template.edgeone.site/ssg-img.png?imageMogr2/crop/300x400/gravity/northwest/dx/10/dy/20', }) }) it('edgeonePages pad with background', () => { const providerOptions = { baseURL: 'https://nuxt-mix-template.edgeone.site', } const generated = edgeonePages().getImage('/ssg-img.png', { modifiers: { width: 300, height: 200, background: 'ffffff' }, ...providerOptions, }, getEmptyContext()) // Base64 of '#ffffff' URL-safe encoded expect(generated.url).toContain('pad/1/color/') }) it('edgeonePages throws without baseURL', () => { expect(() => edgeonePages().getImage('/ssg-img.png', { modifiers: {}, } as any, getEmptyContext())).toThrow('EdgeOne Pages provider requires baseURL to be set') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/nuxt/providers.test.ts` around lines 593 - 729, Add the suggested edge-case tests for the EdgeOne Pages provider by updating the test suite around edgeonePages().getImage: add a test for crop with offsets verifying dx/dy appear (use modifiers { crop, gravity, dx, dy }), add a pad/background test that asserts the URL contains the pad/1/color/ base64-url-safe encoded background value (use modifiers { width, height, background }), and add a test that calling edgeonePages().getImage without providerOptions.baseURL throws a clear error (assert it throws something like "EdgeOne Pages provider requires baseURL to be set"); ensure these tests reference edgeonePages().getImage and getEmptyContext() so they locate the same helpers and verify proper URL encoding/throwing behavior.src/runtime/providers/edgeonePages.ts (1)
48-55: Consider validating hex color format.The function assumes the input is a hex color string. Invalid formats like
rgb(...)or named colors would produce incorrect Base64 output. Consider adding basic validation or documenting the expected input format.💡 Optional: Add hex color validation
function encodeColor(color: string): string { const hex = color.startsWith('#') ? color : `#${color}` + // Validate hex format: `#RGB`, `#RRGGBB`, or `#RRGGBBAA` + if (!/^#([0-9A-Fa-f]{3}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$/.test(hex)) { + console.warn(`[edgeonePages] Invalid hex color format: ${color}`) + } if (typeof globalThis.btoa === 'function') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/edgeonePages.ts` around lines 48 - 55, The encodeColor function currently assumes its input is a hex color and may produce incorrect output for non-hex inputs; update encodeColor to validate the input (e.g., accept /^#?([0-9a-fA-F]{6}|[0-9a-fA-F]{3})$/), normalize to a full 7-char form (prepend '#' if missing and expand 3-digit to 6-digit), and either throw a clear error or return a safe default when validation fails; implement this validation inside encodeColor so callers sending values like "rgb(...)" or named colors are handled predictably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/provider.ts`:
- Line 50: The providers array in src/provider.ts is out of alphabetical order:
move the 'edgeonePages' entry so it appears between 'directus' and 'fastly'
(i.e., alphabetize the list as requested); locate the providers array and
reposition the 'edgeonePages' string accordingly to restore the alphabetical
ordering rule referenced in the file.
In `@src/runtime/providers/edgeonePages.ts`:
- Around line 188-192: The code builds `query` and appends it to `src` which
breaks URLs when `src` already contains query params (e.g., `/img.png?v=1`)
causing `/img.png?v=1?imageMogr2/...`; update the return in the function that
constructs the URL (references: `operations`, `query`, `src`, `joinURL`,
`baseURL`, `imageMogr2`) to merge parameters properly instead of simple string
concatenation—either use a URL helper like `withQuery` from `ufo` to add the
`imageMogr2` param path or parse `src` with the URL API to preserve existing
query/search params and append the `imageMogr2/...` as an additional query value
before calling `joinURL`.
---
Nitpick comments:
In `@src/runtime/providers/edgeonePages.ts`:
- Around line 48-55: The encodeColor function currently assumes its input is a
hex color and may produce incorrect output for non-hex inputs; update
encodeColor to validate the input (e.g., accept
/^#?([0-9a-fA-F]{6}|[0-9a-fA-F]{3})$/), normalize to a full 7-char form (prepend
'#' if missing and expand 3-digit to 6-digit), and either throw a clear error or
return a safe default when validation fails; implement this validation inside
encodeColor so callers sending values like "rgb(...)" or named colors are
handled predictably.
In `@test/nuxt/providers.test.ts`:
- Around line 593-729: Add the suggested edge-case tests for the EdgeOne Pages
provider by updating the test suite around edgeonePages().getImage: add a test
for crop with offsets verifying dx/dy appear (use modifiers { crop, gravity, dx,
dy }), add a pad/background test that asserts the URL contains the pad/1/color/
base64-url-safe encoded background value (use modifiers { width, height,
background }), and add a test that calling edgeonePages().getImage without
providerOptions.baseURL throws a clear error (assert it throws something like
"EdgeOne Pages provider requires baseURL to be set"); ensure these tests
reference edgeonePages().getImage and getEmptyContext() so they locate the same
helpers and verify proper URL encoding/throwing behavior.
🪄 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: 171fb23f-c6ea-4d7b-bd94-2b9c9477fea3
📒 Files selected for processing (8)
docs/content/3.providers/edgeone-pages.mdplayground/app/pages/edgeone-pages.vueplayground/app/providers.tsplayground/nuxt.config.tssrc/provider.tssrc/runtime/providers/edgeonePages.tstest/e2e/__snapshots__/edgeOnePages.json5test/nuxt/providers.test.ts
✅ Files skipped from review due to trivial changes (3)
- playground/nuxt.config.ts
- test/e2e/snapshots/edgeOnePages.json5
- docs/content/3.providers/edgeone-pages.md
🚧 Files skipped from review as they are similar to previous changes (2)
- playground/app/providers.ts
- playground/app/pages/edgeone-pages.vue
| 'strapi', | ||
| 'strapi5', | ||
| 'supabase', | ||
| 'edgeonePages', |
There was a problem hiding this comment.
Provider not added alphabetically.
The comment on line 14 states "Please add new providers alphabetically to the list below." The provider 'edgeonePages' (starting with 'e') should be placed between 'directus' and 'fastly', not after 'supabase'.
🔧 Proposed fix
Move 'edgeonePages' to the correct alphabetical position:
'cloudinary',
'contentful',
'directus',
+ 'edgeonePages',
'fastly',
'filerobot',
...
'strapi5',
'supabase',
- 'edgeonePages',
'twicpics',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/provider.ts` at line 50, The providers array in src/provider.ts is out of
alphabetical order: move the 'edgeonePages' entry so it appears between
'directus' and 'fastly' (i.e., alphabetize the list as requested); locate the
providers array and reposition the 'edgeonePages' string accordingly to restore
the alphabetical ordering rule referenced in the file.
Comment on lines
+188
to
+192
| const query = operations.length ? `?imageMogr2/${operations.join('/')}` : '' | ||
|
|
||
| return { | ||
| url: joinURL(baseURL, src + query), | ||
| } |
There was a problem hiding this comment.
Potential issue with existing query parameters in src.
If src already contains query parameters (e.g., /image.png?v=123), concatenating query directly will produce an invalid URL like /image.png?v=123?imageMogr2/.... Consider using proper query string handling.
🔧 Proposed fix using withQuery from ufo
-import { joinURL } from 'ufo'
+import { joinURL, withQuery, parseQuery, parseURL } from 'ufo'
import type { ImageModifiers } from '@nuxt/image'
import { defineProvider } from '../utils/provider'Then update the return statement:
- const query = operations.length ? `?imageMogr2/${operations.join('/')}` : ''
+ const operationsPath = operations.length ? `imageMogr2/${operations.join('/')}` : ''
return {
- url: joinURL(baseURL, src + query),
+ url: operationsPath
+ ? withQuery(joinURL(baseURL, src), { imageMogr2: undefined, [operationsPath]: '' })
+ : joinURL(baseURL, src),
}Alternatively, if the imageMogr2 API doesn't support standard query parameters, you could parse and preserve existing params:
- const query = operations.length ? `?imageMogr2/${operations.join('/')}` : ''
-
- return {
- url: joinURL(baseURL, src + query),
- }
+ const baseUrl = joinURL(baseURL, src)
+ if (!operations.length) {
+ return { url: baseUrl }
+ }
+ const separator = baseUrl.includes('?') ? '&' : '?'
+ return {
+ url: `${baseUrl}${separator}imageMogr2/${operations.join('/')}`,
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/providers/edgeonePages.ts` around lines 188 - 192, The code
builds `query` and appends it to `src` which breaks URLs when `src` already
contains query params (e.g., `/img.png?v=1`) causing
`/img.png?v=1?imageMogr2/...`; update the return in the function that constructs
the URL (references: `operations`, `query`, `src`, `joinURL`, `baseURL`,
`imageMogr2`) to merge parameters properly instead of simple string
concatenation—either use a URL helper like `withQuery` from `ufo` to add the
`imageMogr2` param path or parse `src` with the URL API to preserve existing
query/search params and append the `imageMogr2/...` as an additional query value
before calling `joinURL`.
q153877011
force-pushed
the
main
branch
2 times, most recently
from
7f1d225 to
5ea87fa
Compare
q153877011
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
N/A (new feature, no existing issue)
📚 Description
Add a built-in provider for Tencent Cloud EdgeOne Pages image processing (
imageMogr2).What it does:
This provider maps Nuxt Image's standard modifiers (
width,height,fit,quality,format,background,blur) to Tencent Cloud COS imageMogr2 API parameters. It also supports COS-specific extended modifiers includingcrop,gravity,rotate,sharpen,strip,scrop(smart face crop),iradius(circle crop),autoOrient,interlace, andpad.Fit mode mapping:
contain/inside→/thumbnail/<W>x<H>cover/outside→/thumbnail/!<W>x<H>rfill→/thumbnail/<W>x<H>!Changes:
src/runtime/providers/tencentCloud.ts— provider implementationsrc/provider.ts— registertencentCloudin built-in providersdocs/content/3.providers/tencent-cloud.md— documentationtest/providers.ts— 6 standard test casestest/nuxt/providers.test.ts— 11 unit tests (fit modes, blur, rotate, crop, sharpen, scrop, interlace, iradius, combined operations)test/e2e/__snapshots__/tencentCloud.json5— e2e snapshottest/unit/bundle.test.ts— updated bundle size snapshotplayground/— config, sample data, and interactive preview page