Support for Image variants using Sharp

devjayantmalik commented 1 year ago

Proposed changes

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Recently I have facing an issue where users upload images such as user avatar, we need to compress and downscale them to our custom resolution. This feature is not yet available in attachment-lite. I have added the feature with opt-in behavior, which means user can choose to use the feature by providing variants option in @attachment decorator.

Along with the above change I have also made few more changes in the package:

Updated all packages to their latest version possible(Which removes few security vulnerabilities you see when you npm install.
Added support of Image resize, conversion and compression
Added test for the above change.
Added support for github actions cache feature to speed up tests.

With the introduction of the feature the code will look something like this:

import { column, BaseModel } from '@ioc:Adonis/Lucid/Orm'
import { attachment, AttachmentContract } from '@ioc:Adonis/Addons/AttachmentLite'

export class User extends BaseModel {
  @column()
  public id: number

  @column()
  public email: string

  @attachment()
  public avatar: AttachmentContract

 // Details related to variants such as width, height etc. needs to be provided in `config/attachment.ts` file
 @attachment({variants: ['thumbnail', 'medium', 'large'] })
  public photo: AttachmentContract // variants will take care of generating images of provided size.
}

Types of changes

What types of changes does your code introduce?

Put an x in the boxes that apply

[ ] Bugfix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

[ ] I have read the CONTRIBUTING doc
[x] Lint and unit tests pass locally with my changes
[x] I have added tests that prove my fix is effective or that my feature works.
[ ] I have added necessary documentation (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

I also considered other alternatives such as https://www.npmjs.com/package/@jrmc/attachment-advanced But found few issue in those:

The package does not work with s3 as the logic is incorrect for creating image variants(I fixed it in this PR)
The package has lot of direct dependencies, which we need to install. (I choose to place them in devDependencies and peer dependencies)
The package is somewhat heavy weight for such a small feature.

Incase community feels like this should be part of core package, I would be happy to update documentation and contribute.

RomainLanz commented 1 year ago

Hey @devjayantmalik! 👋🏻

I don't believe we want to have this feature in this package. As the name suggests, we want to keep it light.

There are already community packages that do this if you would like to use this feature.

Coming to my mind:

Attachment Advanced
Adonis Responsive Attachment

devjayantmalik commented 1 year ago

Thanks @RomainLanz I would then create a new package for this. Thanks for the time. Just wanted to tell everyone that other packages mentioned above have bugs in them. So, Not considering them. It looks like I should close the PR now. Thanks everyone for your time and wish our community a good time. Looking forward for any contributions feasible from my side.

RomainLanz commented 1 year ago

Just wanted to tell everyone that other packages mentioned above have bugs in them.

Why not try to fix the bugs in them instead of creating a new package? 😄

adonisjs / attachment-lite