jeanbmar/payload-s3-upload

formatOptions is not applied on base image

jonisapp opened this issue · 5 comments

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;

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.

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:

files.push({
buffer,
filename,
mimeType: data.mimeType,
});

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.

const getFilesToUpload: CollectionBeforeChangeHook = ({
data,
req,
}): File[] => {

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.

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?:

const reqFile = req.files?.file ?? req.file ?? null;
if (reqFile == null) return [];
const files: File[] = [
{
filename: data.filename,
mimeType: data.mimeType,
buffer: reqFile.data,
},
];
if (data.mimeType?.includes('image') && data.sizes != null) {
Object.entries<FileData>(data.sizes).forEach(([key, sizeData]) => {
const buffer = req.payloadUploadSizes[key];
const { filename } = sizeData;
if (buffer != null || filename != null) {
files.push({
buffer,
filename,
mimeType: data.mimeType,
});
}
});
}
return files;

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

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.