Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle undefined or empty image source values #1300

Merged

Conversation

ThimoDEV
Copy link
Contributor

References #1299

I have been thinking about the best solution for this issue. Typescript says by design that the src prop is string | undefined so we either need to narrow that down or allow it?

Vue by design always adds undefined type as far as I could see. When I removed the throw error it works more as expected and with the reuired property on the src prop I get a Vue warning that it is not set which is nice.

However when I set:

:src="undefined" I get a Vue warning the the type for the src prop is incorrect.

Is it better to match the typescript types (and it quite follows the normal behaviour or have it generate a Vue prop required warning with the above downside?

@toyi
Copy link

toyi commented Apr 8, 2024

Allowing undefined would also allow the developer to eventually catch this undefined src in a custom provider and do something else with it (showing a specific placeholder or something else).

Currently, we must pass a string (and '' doesn't work, it throws an error saying it received '' instead of a string). So when the image can be undefined, it must be handled directly where you use the component (or in a high order component).

@danielroe danielroe changed the title fix: src undefined, empty string or not set throws error fix: handle undefined or empty image source values Apr 22, 2024
@danielroe danielroe merged commit 597c70b into nuxt:main Apr 22, 2024
1 check passed
@github-actions github-actions bot mentioned this pull request Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants