fix(FormField): added a utility type to fix some type errors

mauroerta commented 7 months ago

🔗 Linked issue

No Linked Issue (I can create one in case it is needed)

❓ Type of change

[ ] 📖 Documentation (updates to the documentation or readme)
[ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
[ ] 👌 Enhancement (improving an existing functionality)
[ ] ✨ New feature (a non-breaking change that adds functionality)
[x] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
[ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I wanted to contribute to this project, specifically, I wanted to pick this issue here, while working on it I saw the @ts-expect-error comments and I thought I could fix them easily in a separate PR since they're all related to the same error.

The Problem

In 2 places the type definition was accepting a generic parameter T without any constraint and trying to access one of its properties:

type Props<T> = {
   size?: T['size'] // <-- You can't assume T is an object and that it might have a property called "size"
}

Solution

I thought about 2 approaches:

Solution 1. (Proposed solution)

Instead of accessing the property directly (T['size']) we can use a utility type that extracts the property value in case it exists:

type Props<T> = {
   size?: GetObjectField<T, 'size'>
}

The utility type GetObjectField extracts the type of size contained in the type T in case T is object-like, never is returned otherwise.

I'm not sure GetObjectField is the right name for this utility type, in case this approach will be chosen we can think of another name

Solution 2

Simply constraint the type of the generic parameter:

type Props<T extends { size?: string }> = {
   size?: T['size']
}

This is the simplest way to fix it but I'm not aware if this was not done for some reason.

I chose to go for the Solution 1 because I didn't have to change anything in the type definition of the affected types, even though usually I'd have chosen the Solution 2

📝 Checklist

[ ] I have linked an issue or discussion.
[ ] I have updated the documentation accordingly.

benjamincanac commented 7 months ago

Thank you!! Didn't know how to fix that 😅 I saw you mentioned nuxt/ui#2140, it would be really helpful if you have a solution 😊

mauroerta commented 7 months ago

Thank you!! Didn't know how to fix that 😅 I saw you mentioned nuxt/ui#2140, it would be really helpful if you have a solution 😊

I'll work on it, I hope to open a PR soon

benjamincanac / ui3