Skip to content

fix: handle undefined or empty image source values#1300

Merged
danielroe merged 4 commits into
nuxt:mainfrom
ThimoDEV:fix/src-undefined-or-empty-string-throws-error
Apr 22, 2024
Merged

fix: handle undefined or empty image source values#1300
danielroe merged 4 commits into
nuxt:mainfrom
ThimoDEV:fix/src-undefined-or-empty-string-throws-error

Conversation

@ThimoDEV

Copy link
Copy Markdown
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 Image behaviour or have it generate a Vue prop required warning with the above downside?

@toyi

toyi commented Apr 8, 2024

Copy link
Copy Markdown

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
@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