Skip to content

fix Type bug in WebpOptions.#3758

Merged
lovell merged 2 commits intolovell:mainfrom
sho-xizz:feature/fix-webp-options-type-bug
Aug 9, 2023
Merged

fix Type bug in WebpOptions.#3758
lovell merged 2 commits intolovell:mainfrom
sho-xizz:feature/fix-webp-options-type-bug

Conversation

@sho-xizz
Copy link
Contributor

@sho-xizz sho-xizz commented Aug 9, 2023

While use sharp to convert to WebP with TypeScript, I found the Type Error of WebpOptions.minSize.
it param is boolean, but Type specify as number.
So we had going to use @ts-ignore when use this option.

This PR corrects this bug.

@sho-xizz sho-xizz changed the title fix Type bug in webpOptions. fix Type bug in WebpOptions. Aug 9, 2023
@lovell
Copy link
Owner

lovell commented Aug 9, 2023

Thank you, this looks like a bug inherited from when the TypeScript definitions were imported from DefinitelyTyped. Are you able to add a test for this also, to help prevent regression?

@sho-xizz
Copy link
Contributor Author

sho-xizz commented Aug 9, 2023

Already there is minSize test as Support minSize and mixed webp options, but it looks like wrong.
So, I fix it param to boolean from number.

If this change is bad commit, could you tell me an example in one of your test cases I can base myself on to respect your guidelines, if any...?

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

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

Brilliant, thank you, the existing (inherited) test case was wrong so thanks for fixing this too!

@lovell lovell added this to the v0.32.5 milestone Aug 9, 2023
@lovell lovell merged commit 87562a5 into lovell:main Aug 9, 2023
lovell added a commit that referenced this pull request Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants