fix ipx provider options - #976
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #976 +/- ##
==========================================
+ Coverage 89.63% 89.73% +0.09%
==========================================
Files 44 44
Lines 2924 2971 +47
Branches 329 331 +2
==========================================
+ Hits 2621 2666 +45
- Misses 302 305 +3
+ Partials 1 0 -1
☔ View full report in Codecov by Sentry. |
| maxAge: providerOptions.options?.maxAge || moduleOptions.ipx?.maxAge, | ||
| domains: moduleOptions.domains || moduleOptions.ipx?.domains, | ||
| sharp: moduleOptions.sharp || moduleOptions.ipx?.sharp, | ||
| alias: moduleOptions.alias || moduleOptions.ipx?.alias |
There was a problem hiding this comment.
New options are updated for ipx v2 interface (https://github.com/nuxt/image/blob/main/src/ipx.ts#L32)
We probably don't need this anymore (but we can also spread entire moduleOptions.ipx (typed as new IPXRuntimeConfig)
| strapi?: any | ||
| imageengine?: any | ||
| uploadcare?: Partial<UploadcareOptions> | ||
| ipx?: Partial<StaticOptions> |
| resolvedProvider, | ||
| BuiltInProviders.includes(resolvedProvider) | ||
| ? { provider: resolvedProvider, options: options[resolvedProvider] } | ||
| : options[resolvedProvider] |
There was a problem hiding this comment.
Was this change necessary? (I think it would be nice to keep this arg unified with { options } interface always.)
There was a problem hiding this comment.
It's necessary.
The resolveProviders that take ModuleOptions as arguments also do this processing, so they need to do the same for ModuleOptions in the nitro:init hook.
https://github.com/nuxt/image/blob/v1.0.0-rc.3/src/provider.ts#L67-L71
There was a problem hiding this comment.
Considering this logic running within if (!options.provider || options.provider === 'ipx' || options.provider === 'ipxStatic'), possible values for resolvedProvider are ipx, ipxStatic and none, all are built-in so i think this line would not also reach...
|
Hi. I have merged in a slightly different fix via #1056 for options handling (sorry since was in rush for 1.0.0 release). Would be nice if we can repurpose this changes to modifiers type+fix. (And thanks for all your helps on both nuxt image and IPX all the time ❤️ ) |
|
Thank you for reviewing! And sorry for the delay in getting back. This PR makes some conflicts, so I remade it following. |
4 participants
https://image.nuxt.com/get-started/configuration#modifiers
According to the document, setting to default modifiers should be possible, but it wasn't working in typescript, so I fixed it.