fix: infer configured providers and their modifiers in useImage#2141
fix: infer configured providers and their modifiers in useImage#2141DamianGlowala wants to merge 3 commits intomainfrom
useImage#2141Conversation
Deploying nuxt-image with
|
| Latest commit: |
92cf582
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b56fd4c8.nuxt-image.pages.dev |
| Branch Preview URL: | https://fix-use-image-generics.nuxt-image.pages.dev |
commit: |
📝 WalkthroughWalkthroughImage typings were generalized to be provider-generic and sizes were unified. In src/types/image.ts ImageOptions and Img signatures became generic over a provider type (TProvider), a Sizes alias was added, and modifiers/sizes typings were adjusted to be provider-aware. In src/runtime/image.ts multiple function signatures and call sites were updated to accept or cast to ImageOptions (including getImage, getMeta, getSizes, getVariantSrc, and resolveImage), and optional chaining was applied when spreading presets/modifiers. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🤖 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/types/image.ts`:
- Line 94: The declared getSizes signature is inconsistent with the runtime
which expects an options object containing a required sizes field
(ImageSizesOptions); update the type signature for getSizes to accept options:
ImageSizesOptions<TProvider> (remove the separate sizes?: string[] parameter) so
it matches the runtime implementation, referencing the existing types
ImageOptions<TProvider>, ImageSizesOptions and the return type ImageSizes.
- Around line 87-95: The getSizes signature is wrong: it declares a separate
optional sizes parameter but the implementation reads sizes from the options
object; update getSizes to accept options?: ImageSizesOptions (i.e. change the
third param into a single options parameter) so its signature becomes getSizes:
<TProvider extends keyof ConfiguredImageProviders = keyof
ConfiguredImageProviders>(source: string, options?: ImageSizesOptions) =>
ImageSizes; while you're here, align the default type parameters between the Img
methods and ImageOptions by using the same default (either keyof
ConfiguredImageProviders or DefaultProvider) so ImageOptions<TProvider> resolves
consistently for methods like getSizes, getImage, getMeta and the
options/modifiers types match at call sites.
src/types/image.ts
Outdated
| <TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders>( | ||
| source: string, | ||
| modifiers?: ImageOptions<TProvider>['modifiers'], | ||
| options?: ImageOptions<TProvider> | ||
| ): ResolvedImage['url'] | ||
| options: CreateImageOptions | ||
| getImage: (source: string, options?: ImageOptions) => ResolvedImage | ||
| getSizes: (source: string, options?: ImageOptions, sizes?: string[]) => ImageSizes | ||
| getMeta: (source: string, options?: ImageOptions) => Promise<ImageInfo> | ||
| getImage: <TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders>(source: string, options?: ImageOptions<TProvider>) => ResolvedImage | ||
| getSizes: <TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders>(source: string, options?: ImageOptions<TProvider>, sizes?: string[]) => ImageSizes | ||
| getMeta: <TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders>(source: string, options?: ImageOptions<TProvider>) => Promise<ImageInfo> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "DefaultProvider" --type=ts -C2Repository: nuxt/image
Length of output: 465
🏁 Script executed:
cat -n src/types/image.ts | head -100Repository: nuxt/image
Length of output: 3952
🏁 Script executed:
# Search for getSizes implementation and ImageSizesOptions definition
rg -n "getSizes|ImageSizesOptions" --type=ts -C3Repository: nuxt/image
Length of output: 5711
Fix getSizes type signature and consider aligning default type parameters.
The getSizes method signature is incorrect: it declares sizes as a separate optional parameter, but the runtime implementation expects it as part of the options object (via ImageSizesOptions). The type should be:
getSizes: <TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders>(source: string, options?: ImageSizesOptions) => ImageSizesAdditionally, there's an inconsistency in default type parameters: Img interface methods default to keyof ConfiguredImageProviders (all providers), while ImageOptions defaults to DefaultProvider (configured default). This causes ImageOptions<TProvider> to accept different modifiers depending on whether TProvider is inferred or explicitly matched against the method's default. Consider aligning these defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/image.ts` around lines 87 - 95, The getSizes signature is wrong: it
declares a separate optional sizes parameter but the implementation reads sizes
from the options object; update getSizes to accept options?: ImageSizesOptions
(i.e. change the third param into a single options parameter) so its signature
becomes getSizes: <TProvider extends keyof ConfiguredImageProviders = keyof
ConfiguredImageProviders>(source: string, options?: ImageSizesOptions) =>
ImageSizes; while you're here, align the default type parameters between the Img
methods and ImageOptions by using the same default (either keyof
ConfiguredImageProviders or DefaultProvider) so ImageOptions<TProvider> resolves
consistently for methods like getSizes, getImage, getMeta and the
options/modifiers types match at call sites.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2141 +/- ##
=======================================
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/image.ts (1)
235-243:⚠️ Potential issue | 🟡 MinorInconsistent
ImageOptionsusage—missing type parameter.All other functions in this file use
ImageOptions<any>, butgetVariantSrcuses bareImageOptions. WithImageOptionsdefaulting toDefaultProvider(which may beneverif no default provider is configured), this could cause type mismatches.Suggested fix for consistency
-function getVariantSrc(ctx: ImageCTX, input: string, variant: ImageSizesVariant, density: number, opts?: ImageOptions) { +function getVariantSrc(ctx: ImageCTX, input: string, variant: ImageSizesVariant, density: number, opts?: ImageOptions<any>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/image.ts` around lines 235 - 243, The function getVariantSrc uses the bare ImageOptions type which is inconsistent with the rest of the file and can default to an unwanted provider type; update its signature and any usage to use ImageOptions<any> (i.e., change the parameter opts?: ImageOptions to opts?: ImageOptions<any>) so it matches other functions (refer to getVariantSrc, ImageCTX, ImageSizesVariant, ctx.$img) and avoids type mismatches with DefaultProvider.
🧹 Nitpick comments (1)
src/types/image.ts (1)
84-94: Default type parameter inconsistency betweenImgmethods andImageOptions.The
Imginterface methods defaultTProvidertokeyof ConfiguredImageProviders, whileImageOptionsdefaults toDefaultProvider. WhenImageOptions<TProvider>is used without explicit type argument in a method body, it inherits the method's default, but standalone uses ofImageOptionsgetDefaultProvider. This can causemodifiersto resolve differently depending on inference context.Consider aligning the defaults—either both use
keyof ConfiguredImageProvidersor both useDefaultProvider:Option A: Align ImageOptions default with Img
-export interface ImageOptions<TProvider extends keyof ConfiguredImageProviders = DefaultProvider> { +export interface ImageOptions<TProvider extends keyof ConfiguredImageProviders = keyof ConfiguredImageProviders> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/image.ts` around lines 84 - 94, The issue is inconsistent default type parameters between Img methods (which default TProvider to keyof ConfiguredImageProviders) and ImageOptions (which defaults to DefaultProvider), causing modifiers and options to infer differently; fix by aligning defaults—update the generic default on the Img call signature and the getImage/getSizes/getMeta method signatures to use DefaultProvider instead of keyof ConfiguredImageProviders so ImageOptions<TProvider> resolves the same whether inferred or used directly; ensure the modifiers parameter types in the call signature reflect ImageOptions<DefaultProvider> and run type checks to confirm no other places rely on the old default.
🤖 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/runtime/image.ts`:
- Around line 180-185: The code is unsafely casting
opts?.modifiers?.width/height to number which can let string sizes slip through;
replace the direct "as number" casts with calls to the existing parseSize
utility (same one used at line 132) to normalize width/height into numeric
values (fallback to 0 when parseSize returns NaN/undefined), i.e. compute
_cWidth = parseSize(opts?.modifiers?.width) and _cHeight =
parseSize(opts?.modifiers?.height) so that downstream arithmetic on
variant._cWidth and variant._cHeight is safe and consistent with other parsing
logic.
---
Outside diff comments:
In `@src/runtime/image.ts`:
- Around line 235-243: The function getVariantSrc uses the bare ImageOptions
type which is inconsistent with the rest of the file and can default to an
unwanted provider type; update its signature and any usage to use
ImageOptions<any> (i.e., change the parameter opts?: ImageOptions to opts?:
ImageOptions<any>) so it matches other functions (refer to getVariantSrc,
ImageCTX, ImageSizesVariant, ctx.$img) and avoids type mismatches with
DefaultProvider.
---
Nitpick comments:
In `@src/types/image.ts`:
- Around line 84-94: The issue is inconsistent default type parameters between
Img methods (which default TProvider to keyof ConfiguredImageProviders) and
ImageOptions (which defaults to DefaultProvider), causing modifiers and options
to infer differently; fix by aligning defaults—update the generic default on the
Img call signature and the getImage/getSizes/getMeta method signatures to use
DefaultProvider instead of keyof ConfiguredImageProviders so
ImageOptions<TProvider> resolves the same whether inferred or used directly;
ensure the modifiers parameter types in the call signature reflect
ImageOptions<DefaultProvider> and run type checks to confirm no other places
rely on the old default.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/types/image.ts (1)
28-29: Provider-specific modifiers aren't wrapped inPartial, unlike base modifiers.The base modifiers use
Partial<Omit<...>>, but provider-specific modifiers fromConfiguredImageProviders[TProvider]['modifiers']are used directly. If a provider declares required modifier properties, they would become required inImageOptionsas well.If this strictness is intentional, consider documenting it. Otherwise, wrap in
Partialfor consistency:♻️ Suggested change for consistency
modifiers?: Partial<Omit<ImageModifiers, 'format' | 'quality' | 'background' | 'fit'>> - & ('modifiers' extends keyof ConfiguredImageProviders[TProvider] ? ConfiguredImageProviders[TProvider]['modifiers'] : Record<string, unknown>) + & ('modifiers' extends keyof ConfiguredImageProviders[TProvider] ? Partial<ConfiguredImageProviders[TProvider]['modifiers']> : Record<string, unknown>)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/image.ts` around lines 28 - 29, The provider-specific modifiers in the ImageOptions type are not wrapped in Partial, so required fields declared on ConfiguredImageProviders[TProvider]['modifiers'] will become required; update the modifiers union to wrap the provider-specific branch in Partial(...) (i.e., Partial<ConfiguredImageProviders[TProvider]['modifiers']>) so provider modifiers are optional like the base Partial<Omit<ImageModifiers, 'format' | 'quality' | 'background' | 'fit'>>; adjust the type expression referencing modifiers, ImageModifiers, ConfiguredImageProviders and TProvider accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/types/image.ts`:
- Around line 28-29: The provider-specific modifiers in the ImageOptions type
are not wrapped in Partial, so required fields declared on
ConfiguredImageProviders[TProvider]['modifiers'] will become required; update
the modifiers union to wrap the provider-specific branch in Partial(...) (i.e.,
Partial<ConfiguredImageProviders[TProvider]['modifiers']>) so provider modifiers
are optional like the base Partial<Omit<ImageModifiers, 'format' | 'quality' |
'background' | 'fit'>>; adjust the type expression referencing modifiers,
ImageModifiers, ConfiguredImageProviders and TProvider accordingly.
| export interface ImageSizesOptions extends ImageOptions { | ||
| sizes: Record<string, string | number> | string | ||
| & ('modifiers' extends keyof ConfiguredImageProviders[TProvider] ? ConfiguredImageProviders[TProvider]['modifiers'] : Record<string, unknown>) | ||
| sizes?: Sizes |
There was a problem hiding this comment.
does this work in other contexts? e.g. if not passed to getSizes?
if it isn't respected then ImageSizesOptions should still be a separate type
There was a problem hiding this comment.
from what I can see, ImageSizesOptions was used solely for the purpose of getSizes
There was a problem hiding this comment.
this is great!! 🙌
would you add some type tests in test/nuxt/use-image.test.ts, like the existing ones? 🙏
also, we need to check that this works correctly to respect the default provider, e.g. if the configured default provider is 'directus', then the options of useImage should be directus options unless a different provider is specified.
3 participants
🔗 Linked issue
resolves #2036
resolves #2107
📚 Description
Now, list of available providers is correctly autocompleted and there are no type errors when choosing a provider other than
ipx. Also, the second argument (modifiers) now inferes available modifiers for a selected provider, so that they are now connected and type-safe.