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

types: added missing types for OverlayOptions #4048

Merged
merged 3 commits into from Apr 2, 2024
Merged

Conversation

ike-gg
Copy link
Contributor

@ike-gg ike-gg commented Apr 1, 2024

animated property was missing from declaration types, just added it

@ike-gg ike-gg changed the title types: added missing animated property for OverlayOptions types: added missing types for OverlayOptions Apr 1, 2024
@ike-gg
Copy link
Contributor Author

ike-gg commented Apr 1, 2024

added also: density, failOn, limitInputPixels

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.

Thanks for the PR, great work finding these, the OverlayOptions definitions were inherited from the original DefinitelyTyped repo that we merged into sharp itself so these properties have always been missing.

Would it be possible for you to add a couple of tests for these properties to https://github.com/lovell/sharp/blob/main/test/types/sharp.test-d.ts ?

@ike-gg
Copy link
Contributor Author

ike-gg commented Apr 1, 2024

I have added type tests for these properties. please review and confirm if everything is okay

@lovell lovell merged commit 0981b24 into lovell:main Apr 2, 2024
15 checks passed
@lovell
Copy link
Owner

lovell commented Apr 2, 2024

Thank you very much for reporting and fixing this.

@lovell lovell added this to the v0.33.4 milestone Apr 2, 2024
lovell added a commit that referenced this pull request Apr 2, 2024
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.

None yet

2 participants