-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
[sharp] Update typings to 0.23 #39001
Conversation
@fdebijl Thank you for submitting this PR! 🔔 @lith-light-g @wooseopkim @BTOdell @JamieWoodbury - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? These typings are for a version of sharp that doesn’t yet exist on master, so I’ve compared them with v0. Comparison details 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
More testing in my current project revealed that the parameters for the composite method are also outdated, I changed it into
|
Updated numbers for you here from ac2549a. Comparison details 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
* [sharp] Update typings to 0.23 * Move input to OverlayOptions and Create to input * Switch array of OverlayOptions to simple type * Fix indentation
npm test
.)npm run lint package-name
(ortsc
if notslint.json
is present).If changing an existing definition:
smartSubsample
andreductionEffort
parameters to Webpoptions: WebP: expose the 'method' encoding parameter lovell/sharp#1545cutout
parameter from OverlayOptions: https://github.com/lovell/sharp/blob/master/lib/composite.js#L72 (This parameter is not mentioned in the changelogs, but seemingly not present)premultiplied
parameter to OverlayOptions: Add premultiplied option to composite lovell/sharp#1835skipBlanks
parameter to TileOptions: add skipBlanks support for dz and zoomify layout lovell/sharp#1687layout
parameter in TileOptions to a union type. I believe a union type is more apt forlayout
since only three string values are valid.Blend
also uses a 16-string union type, so using one for the layout makes more sense thanstring
in my opinion.tslint.json
containing{ "extends": "dtslint/dt.json" }
. If for reason the any rule need to be disabled, disable it for that line using// tslint:disable-next-line [ruleName]
and not for whole package so that the need for disabling can be reviewed.