jeanbmar / payload-s3-upload

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

Issue with s3 in media.ts :: CollectionConfig #3

Closed outdatedx closed 2 years ago

outdatedx commented 2 years ago

Refrence to this Issue: #https://github.com/payloadcms/payload/issues/512

I'm not able to figure out the error I'm getting cause of s3 been in the "upload" object

Object literal may only specify known properties, and 's3' does not exist in type 'IncomingUploadType'.ts(2322)
[types.d.ts(185, 5): ]()The expected type comes from property 'upload' which is declared here on type 'CollectionConfig'

Would love some help regarding this

The full source code

import { CollectionConfig } from "payload/types";

export type MediaType = {
  filename: string;
  width: number;
  height: number;
  alt: string;
  sizes: {
    card?: {
      filename: string;
      width: number;
      height: number;
    };
    feature?: {
      filename: string;
      width: number;
      height: number;
    };
  };
};

const Media: CollectionConfig = {
  slug: "media",
  labels: {
    singular: "Media",
    plural: "Media",
  },
  access: {
    read: (): boolean => true, // Everyone can read Media
  },
  upload: {
    staticURL: "/media",
    s3: {
      bucket: process.env.AMAZONWS_BUCKET_NAME,
      prefix: "outdated/assets",
      commandInput: {
        ACL: "public-read",
      },
    },
    staticDir: "media",
    disableLocalStorage: true,
    adminThumbnail: ({ doc }) =>
      `https://${process.env.AMAZONWS_BUCKET_NAME}.s3.${process.env.AMAZON_WS_REGION}.amazonaws.com/outdated/assets/${doc.filename}`,
    imageSizes: [
      {
        name: "card",
        width: 640,
        height: 480,
      },
      {
        name: "feature",
        width: 1024,
        height: 576,
      },
    ],
    mimeTypes: ["image/*"],
  },
  fields: [
    {
      name: "alt",
      label: "Alt Text",
      type: "text",
      required: true,
    },
    {
      name: "url",
      type: "text",
      access: {
        create: () => false,
      },
      admin: {
        disabled: true,
      },
      hooks: {
        afterRead: [
          ({ data: doc }) =>
            `https://${process.env.AMAZONWS_BUCKET_NAME}.s3.${process.env.AMAZON_WS_REGION}.amazonaws.com/website/assets/${doc.filename}`,
        ],
      },
    },
  ],
};

export default Media;
jmikrut commented 2 years ago

I can shine a light on this:

For the strongest typing, Payload configs do not allow for unknown keys to be written directly to Payload config objects. This prevents spelling mistakes and enforces that configs are clean / expected.

In the case above, it looks like this plugin is requiring you to add an s3 key to your media collection upload config. When you go to build, TypeScript is rejecting that key as it is unknown.

The best way to build plugins in Payload is to pass in all config properties needed directly into the instantiator of the plugin. That would require a few changes to this plugin, though. I imagine @jeanbmar may not be using this plugin with TypeScript, so it works for those cases, but just no TS compatibility.

Here's an example for how the plugin API could be updated to avoid this issue:

export default buildConfig({
  serverURL: process.env.PAYLOAD_PUBLIC_SERVER_URL,
  collections: [Projects, Media],
  plugins: [
    s3Upload({
      collectionOptions: {
        media: {
          bucket: 'my-bucket',
          prefix: 'images/xyz', // files will be stored in bucket folder images/xyz
          // prefix: ({ doc }) => `assets/${doc.type}`, // dynamic prefixes are possible too
          commandInput: {
            // optionally, use here any valid PutObjectCommandInput property
            // https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-s3/interfaces/putobjectcommandinput.html
            ACL: 'public-read', 
          },
        },
      },
      region: process.env.AMAZON_WS_REGION,
      credentials: {
        accessKeyId: process.env.AMAZON_WS_KEY,
        secretAccessKey: process.env.AMAZON_WS_SECRET,
      },
    }),
  ],
});

Just basically passing all collection-specific s3 config through to the plugin itself. This makes sure that all plugins encapsulate their own logic and there are no potentially conflicting keys in the future as more and more Payload plugins are published.

What do we think?

outdatedx commented 2 years ago

@jmikrut Thank you soo much for the help!!!

jeanbmar commented 2 years ago

Passing collection configuration inside plugin instantiation is bad design imo @jmikrut.

It makes things ugly and hard to maintain for big projects that would use lots of plugins. Plugin configuration for a collection should be inside the collection file like any other native collection config.

To avoid typing issues, Payload could expose a plugins property inside collection config that does not enforce anything. That's what https://www.serverless.com/ does in their config file for instance.