jeanbmar / payload-s3-upload

Send Payload CMS uploads to Amazon S3
MIT License
56 stars 9 forks source link

formatOptions is not applied on base image #15

Open jonisapp opened 1 year ago

jonisapp commented 1 year ago

When uploading image files, the formatOptions are only applied to "imageSizes" and not to the base image.

import { S3UploadCollectionConfig } from 'payload-s3-upload';

const TextImages: S3UploadCollectionConfig = {
    slug: 'texts_images',
    labels: { singular: 'Texte - image', plural: 'Textes - images' },
    access: {
        read: () => true,
    },
    admin: {
        group: 'Fichiers',
        disableDuplicate: true,
    },
    upload: {
        staticURL: '/texts/images',
        disableLocalStorage: true,
        s3: {
            bucket: 'lepitre-public',
            prefix: 'texts/images',
            commandInput: {
                ACL: 'public-read',
            },
        },
        imageSizes: [
            {
                name: 'thumbnail',
                width: 200,
                height: undefined,
                formatOptions: {
                    format: 'webp',
                    options: {
                        quality: 70,
                    },
                },
            },
        ],
        formatOptions: {
            format: 'webp',
            options: {
                quality: 70,
            },
        },
        adminThumbnail: 'thumbnail',
        mimeTypes: ['image/*'],
    },
    hooks: {
        afterRead: [
            async ({ doc }) => {
                doc.url = `https://sos-ch-gva-2.exo.io/lepitre-public/texts/images/${doc.filename}`;

                Object.keys(doc.sizes).forEach(
                    (k) =>
                        (doc.sizes[
                            k
                        ].url = `https://sos-ch-gva-2.exo.io/lepitre-public/texts/images/${doc.sizes[k].filename}`)
                );
            },
        ],
    },
};

export default TextImages;
jeanbmar commented 1 year ago

Hello, is it something that works properly in Payload? We don't do anything specific with this option in the plugin, as we just process the file buffer passed by Payload. I've checked the official plugin and they do the same.

JosefBredereck commented 1 year ago

Not quite correct:

Payload does adjust the base image a bit.

https://github.com/payloadcms/payload/blob/0d8d7f358d390184f6f888d77858b4a145e94214/src/uploads/generateFileData.ts#L88-L105

Moreover, you can pass formatOptions for each size, which could lead to a mimeType change of the size image. Therefore, it is not recommended to use the mime type of the original:

https://github.com/jeanbmar/payload-s3-upload/blob/3e0417cfd871bfe00d742ae3cd79997efe17c261/src/buildUploadHook.ts#L30-L34

The result is not included in req.payloadUploadSizes, it contains only the separate sizes. https://github.com/payloadcms/payload/blob/0d8d7f358d390184f6f888d77858b4a145e94214/src/uploads/generateFileData.ts#L156

Maybe the data object could give a general idea about the content contained in the edited file. https://github.com/jeanbmar/payload-s3-upload/blob/3e0417cfd871bfe00d742ae3cd79997efe17c261/src/buildUploadHook.ts#L11-L14

jeanbmar commented 1 year ago

I was talking about the cloud storage plugin of Payload, sorry if it was misleading.

The problem I see is that the fileBuffer created at https://github.com/payloadcms/payload/blob/0d8d7f358d390184f6f888d77858b4a145e94214/src/uploads/generateFileData.ts#L115 isn't attached to the object that is returned later.

As a result plugins can't access the transformed image buffer. It's a bug from Payload imo, I see no reason not to pass the transformed buffer to the plugin chain. For sure we don't want to duplicate the image transformation logic in a s3 storage plugin.

jeanbmar commented 1 year ago

I'm wrong Payload does pass the transformed image buffer downstream. I don't see where the issue is located. Payload does all the transformations first, then the plugin simply retrieves the buffers passed downstream and upload objects to S3.

What should be done differently in here?: https://github.com/jeanbmar/payload-s3-upload/blob/3e0417cfd871bfe00d742ae3cd79997efe17c261/src/buildUploadHook.ts#L15-L38

We just get data passed by Payload after transformations are applied.

JosefBredereck commented 1 year ago

I think payload does not expose the result file buffer directly, it is included in someway, but not correctly referenced.

Line 33 ignores the mime type of the created file size completely

I think req.files?.file ?? req.file contains the original file. And the transformed is included in data.files.

But if data is just the upload config, then req.files should be type of array. If req contains this result object: { data: newData, files: filesToSave }

I'm not quite sure what payload is exactly giving to the plugins. I'm just trying to give an initial idea of what could be the cause.