Skip to content

Conversation

@hywax
Copy link
Member

@hywax hywax commented Nov 1, 2025

πŸ”— Linked issue

None

❓ 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)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

before after
image image

optional?: boolean - why was this left as is?
To avoid adding the undefined type to all components in the model.

Adds more typing

πŸ“ Checklist

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

@hywax hywax requested a review from benjamincanac as a code owner November 1, 2025 11:16
@github-actions github-actions bot added the v4 #4488 label Nov 1, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 1, 2025

npm i https://pkg.pr.new/@nuxt/ui@5361

commit: 4066c63

@hywax
Copy link
Member Author

hywax commented Nov 1, 2025

@benjamincanac added screenshots with an example

@benjamincanac benjamincanac changed the title refactor(Input, InputNumber, Textarea): make modelModifiers generic fix(Input/InputNumber/Textarea): make modelModifiers generic Nov 1, 2025
@sandros94
Copy link
Member

Oh, this is a nice one! At first glance everything seems alright, but I'll be home tomorrow evening to properly double-check

@sandros94
Copy link
Member

I haven't been able to play with this PR yet, but something was feeling off and tested it in a typescript playground to double check it, and found what it was:

Doing a Something extends SomethingElse make typescritp check that every property of Something can extend SomethingElse, but having the : never makes the final output always truthy in the wrong way. The proper check for this use-case would be SomethingElse extends Something, to be more precise:

  nullable?: null extends T ? true : never

You can also check this typescritp playground

But for some unknown reasons to me Vue seems to break with optional (tested in a vue playground for now), while typescript says that it is a legal operation to do

Copy link
Member

@sandros94 sandros94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

As of now I would merge it like this, then if I ever understand what causes optional to break I will either create a new PR or report upstream (if it is something related to vue-tsc or whatever). Because even in a Vue playground it seems to work just fine...

@vlaaadislav
Copy link

@sandros94 I'm not sure if it's relevant, but if you set exactOptionalPropertyTypes: true in playground/nuxt/tsconfig, then the optional modifier will work nicely. It seems that without it, undefined is excluded from the generic type since model-value is already an optional prop.

@sandros94
Copy link
Member

@sandros94 I'm not sure if it's relevant, but if you set exactOptionalPropertyTypes: true in playground/nuxt/tsconfig, then the optional modifier will work nicely. It seems that without it, undefined is excluded from the generic type since model-value is already an optional prop.

@vlaaadislav that could explain it, which in that case I would leave it as is anyway

@benjamincanac benjamincanac changed the title fix(Input/InputNumber/Textarea): make modelModifiers generic fix(Input/InputNumber/Textarea): make modelModifiers generic Nov 4, 2025
@benjamincanac benjamincanac merged commit 5c347af into nuxt:v4 Nov 4, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v4 #4488

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants