payloadcms / payload

Payload is the open-source, fullstack Next.js framework, giving you instant backend superpowers. Get a full TypeScript backend and admin panel instantly. Use Payload as a headless CMS or for building powerful applications.
https://payloadcms.com
MIT License
22.29k stars 1.35k forks source link

[3.0.0-beta.34] Error with Payload config validation on field "admin.meta" #6425

Closed nvti closed 1 month ago

nvti commented 2 months ago

Link to reproduction

No response

Describe the Bug

I see you guys change the type of field admin.meta in version 3.0.0-beta.34 (or 3.0.0-beta.33) but you do not change that in validation task. The validation task still check old type, so it throw errors:

[11:29:13] ERROR: There were 2 errors validating your Payload config
[11:29:13] ERROR: 1: "admin.meta.icons" must be an array
[11:29:13] ERROR: 2: "admin.meta.openGraph.images" must be one of [array]
[11:29:14] ERROR: There were 2 errors validating your Payload config
[11:29:14] ERROR: 1: "admin.meta.icons" must be an array
[11:29:14] ERROR: 2: "admin.meta.openGraph.images" must be one of [array]

New type is defined:

export type OpenGraphConfig = {
  description?: string
  images?: OGImageConfig | OGImageConfig[]
  siteName?: string
  title?: string
}

export type IconConfig = {
  color?: string
  /**
   * @see https://developer.mozilla.org/docs/Web/API/HTMLImageElement/fetchPriority
   */
  fetchPriority?: 'auto' | 'high' | 'low'
  media?: string
  /** defaults to rel="icon" */
  rel?: string
  sizes?: string
  type?: string
  url: string
}

To Reproduce

Instal payload version 3.0.0-beta.34, add this config to payload config file:

admin: {
  meta: {
    openGraph: {
      images: {
        url: `${serverUrl}/logo.svg`
      }
    },
    icons: {
      url: `${serverUrl}/favicon.ico`
    }
  }
// others config
}

Payload Version

3.0.0-beta.34

Adapters and Plugins

No response

jacobsfletch commented 1 month ago

Hey @nvti, meta.icons is supposed to be an array. That way you can set multiple icons in the head of your document, i.e. light / dark mode. Looks like we might've overlooked your PR unfortunately. I really appreciate the contribution regardless. This change did go out in the last beta release, so I went ahead and opened #6759 to resolve this—let me know what you think.

nvti commented 1 month ago

Hey @nvti, meta.icons is supposed to be an array. That way you can set multiple icons in the head of your document, i.e. light / dark mode. Looks like we might've overlooked your PR unfortunately. I really appreciate the contribution regardless. This change did go out in the last beta release, so I went ahead and opened #6759 to resolve this—let me know what you think.

Hi @jacobsfletch, thank you for your reply. My PR does only 1 thing: sync between MetaConfig type and its validation schema. I think changing meta.icons to array is OK, your PR is great and I don't have any comment about that.

BTW, there is another small thing in my PR about OpenGraphConfig type (this type is changed, too). Please check in my PR (#6426). If it looks OK, please copy it into your PR. Thanks packages/payload/src/config/types.ts

export const openGraphSchema = joi.object({
  description: joi.string(),
  images: joi.alternatives().try(ogImageObj, joi.array().items(ogImageObj)),
  siteName: joi.string(),
  title: joi.string(),
})
jacobsfletch commented 1 month ago

Got it 👍 that has been carried over here: https://github.com/payloadcms/payload/blob/81f67f4d21d44e06a3160ef14f7d2ab0abf6e0c8/packages/payload/src/config/shared/openGraphSchema.ts#L13

nvti commented 1 month ago

@jacobsfletch you still forgot the siteName field 😄 It's OK, just a small thing. You can add this in another PR. the new type of OpenGraphConfig:

export type OpenGraphConfig = {
    description?: string;
    images?: OGImageConfig | OGImageConfig[];
    siteName?: string;
    title?: string;
};
jacobsfletch commented 1 month ago

Ahh, rip. Would you be willing to PR for this? Otherwise I will get to this as soon as I can.

nvti commented 1 month ago

@jacobsfletch ok, let me open a new PR for this

nvti commented 1 month ago

@jacobsfletch I created PR #6764 to address this

jacobsfletch commented 1 month ago

Merged. Thank you @nvti 🙌