smartcrop-sharp icon indicating copy to clipboard operation
smartcrop-sharp copied to clipboard

sharp peer dependency could be any in range 0 =< 1

Open achepukov opened this issue 2 years ago • 5 comments

Sounds like sharp peer dependency needs manual resolve every time it's updated...

I googled around and found that using asterics is a bad practice

So, I think we are relatively safe with any version < 1, as 1 will be breaking change (fair to guess).

Also, this update does not enforce users who already work with old sharp version to update it, they are safe to keep using the old version.

Here is a nice version calculator to check if anything needed

achepukov avatar Aug 24 '23 01:08 achepukov

@jwagner wdyt?

achepukov avatar Aug 29 '23 05:08 achepukov

@jwagner any other ideas how it could be improved?

achepukov avatar Sep 04 '23 08:09 achepukov

Just had another idea on how this could be resolved. I could just change the interface so sharp is passed in as parameter (either directly to crop or to some factory). Using dependency injection that way I could just define the interface I expect of sharp in the type definition, and the direct dependency on sharp would be eliminated. If there were any incompatibilities in the future because of an API change on the sharp side, hopefully the type definitions would catch it - at least for typescript users.

The example from the README could the look like this:

// finds the best crop of src and writes the cropped and resized image to dest.
function applySmartCrop(src, dest, width, height) {
  return smartcropSharp(sharp).crop(src, { width: width, height: height })
    .then(function(result) {
      const crop = result.topCrop;
      return sharp(src)
        .extract({ width: crop.width, height: crop.height, left: crop.x, top: crop.y })
        .resize(width, height)
        .toFile(dest);
    })
}

Minimally less beginner friendly, but arguably still more so than fighting with broken peer dependencies. I think the main drawback would be that it's a breaking API change.

What do you think?

jwagner avatar Sep 05 '23 09:09 jwagner

I can live with that. I don't see this solution much optimistic, since it makes codebase a bit more complicated (import sharp, then inject it). Imo, peer dependency is cleaner. Would be great to check the review of somebody else :)

achepukov avatar Sep 06 '23 12:09 achepukov

@jwagner sounds like your approach solves extra issue 👍

achepukov avatar Sep 08 '23 06:09 achepukov