fix(types): add ProviderDefaults to fix provider type resolution#2056
fix(types): add ProviderDefaults to fix provider type resolution#2056
Conversation
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2056 +/- ##
=====================================
Coverage 6.99% 6.99%
=====================================
Files 78 78
Lines 3629 3629
Branches 140 140
=====================================
Hits 254 254
Misses 3326 3326
Partials 49 49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // @ts-expect-error this is not a valid modifier for ipx | ||
| alkj: false, |
There was a problem hiding this comment.
this is a correct test to make sure that modifiers are typed correctly (in other words, you can't pass a nonsense modifier to them)
we wouldn't want to get rid of this test
There was a problem hiding this comment.
I've reverted the @ts-expect-error removal. I now understand this is a valid test to ensure invalid modifiers are caught by the type system.
src/index.d.ts
Outdated
| interface ProviderDefaults { | ||
| provider: 'ipx' | ||
| } |
There was a problem hiding this comment.
I don't believe this is a correct fix - this file is only used when developing nuxt/image and is not included in the dist files... it won't make any difference to the experience of an end user
There was a problem hiding this comment.
You're correct that this file isn't included in the dist, so it wouldn't help end users.
danielroe
left a comment
There was a problem hiding this comment.
I don't think this PR would have any effect on the built code of the module.
if you think it does, would you build and diff it with https://app.unpkg.com/@nuxt/image@2.0.0/files/dist/module.d.ts?
🙏
|
Thank you for taking the time to review and for the helpful feedback! 🙏 |
src/types/image.ts
Outdated
| } | ||
|
|
||
| type DefaultProvider = ProviderDefaults extends Record<'provider', unknown> ? ProviderDefaults['provider'] : never | ||
| type DefaultProvider = ProviderDefaults extends Record<'provider', infer P> ? P : keyof ConfiguredImageProviders |
There was a problem hiding this comment.
I still don't think this is the right fix - we need to know the specific default provider (likely ipx) in case provider is not passed into any image functions.
|
I've taken another look. Not entirely sure I got it right, but I added Could you please check if this is the correct approach?🙇🏻♂️ |
|
I don't think this is right either. that interface is populated dynamically via types that nuxt generates, which should certainly include ipx |
resolves #2014
🔗 Linked issue
resolves #2014
❓ Type of change
📚 Description
The
ProviderDefaultsinterface was missing fromsrc/index.d.ts, causingDefaultProvidertype to resolve toneverduring development.This happened because in
src/types/image.ts, theDefaultProvidertype is defined as:Without
ProviderDefaults.providerbeing defined, the conditional type resolves tonever, breaking provider type inference.Changes:
ProviderDefaultsinterface withprovider: 'ipx'tosrc/index.d.ts@ts-expect-errordirectives from tests (now that types work correctly)Testing:
pnpm test:typespasses