Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(richtext-lexical): incorrect UploadData types #11288

Merged
merged 1 commit into from
Feb 19, 2025
Merged

fix(richtext-lexical): incorrect UploadData types #11288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions ...es/richtext-lexical/src/exports/react/components/RichText/converter/converters/upload.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,29 @@ export const UploadJSXConverter: JSXConverters<SerializedUploadNode> = {
upload: ({ node }) => {
// TO-DO (v4): SerializedUploadNode should use UploadData_P4
const uploadDocument = node as UploadDataImproved
if (typeof uploadDocument.value !== 'object') {
if (typeof uploadDocument?.value !== 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess ts was asking for optional chaining here? any reason why it is needed now? afaik value should be defined

return null
}
const url = uploadDocument.value.url

const value = uploadDocument.value
const url = value.url

/**
* If the upload is not an image, return a link to the upload
*/
if (!uploadDocument.value.mimeType.startsWith('image')) {
if (!value.mimeType.startsWith('image')) {
return (
<a href={url} rel="noopener noreferrer">
{uploadDocument.value.filename}
{value.filename}
</a>
)
}

/**
* If the upload is a simple image with no different sizes, return a simple img tag
*/
if (!Object.keys(uploadDocument.value.sizes).length) {
return (
<img
alt={uploadDocument.value.filename}
height={uploadDocument.value.height}
src={url}
width={uploadDocument.value.width}
/>
)
if (!Object.keys(value.sizes).length) {
return <img alt={value.filename} height={value.height} src={url} width={value.width} />
}
Comment on lines +15 to 34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now almost all things derived from value are "any"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a type assertion in https://github.com/payloadcms/payload/pull/11292/files.

We could have it fall back to FileData & TypeWithID, but I don't know if we should. If it has to fall back, that means the collection slug likely doesn't exist, or it isn't an uploads-enabled collection.


/**
Expand All @@ -44,8 +39,8 @@ export const UploadJSXConverter: JSXConverters<SerializedUploadNode> = {
const pictureJSX: React.ReactNode[] = []

// Iterate through each size in the data.sizes object
for (const size in uploadDocument.value.sizes) {
const imageSize = uploadDocument.value.sizes[size] as FileSizeImproved
for (const size in value.sizes) {
const imageSize = value.sizes[size] as FileSizeImproved

// Skip if any property of the size object is null
if (
Expand Down Expand Up @@ -74,11 +69,11 @@ export const UploadJSXConverter: JSXConverters<SerializedUploadNode> = {
// Add the default img tag
pictureJSX.push(
<img
alt={uploadDocument.value?.filename}
height={uploadDocument.value?.height}
alt={value?.filename}
height={value?.height}
key={'image'}
src={url}
width={uploadDocument.value?.width}
width={value?.width}
/>,
)
return <picture>{pictureJSX}</picture>
Expand Down
45 changes: 26 additions & 19 deletions packages/richtext-lexical/src/features/upload/server/nodes/UploadNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import type {
NodeKey,
Spread,
} from 'lexical'
import type { FileData, JsonObject, TypedCollection, TypeWithID } from 'payload'
import type {
CollectionSlug,
DataFromCollectionSlug,
JsonObject,
TypedUploadCollection,
UploadCollectionSlug,
} from 'payload'
import type { JSX } from 'react'

import { DecoratorBlockNode } from '@lexical/react/LexicalDecoratorBlockNode.js'
Expand All @@ -17,28 +23,29 @@ import { $applyNodeReplacement } from 'lexical'
import * as React from 'react'

export type UploadData<TUploadExtraFieldsData extends JsonObject = JsonObject> = {
fields: TUploadExtraFieldsData
/** Every lexical node that has sub-fields needs to have a unique ID. This is the ID of this upload node, not the ID of the linked upload document */
id: string
relationTo: string
/** Value can be just the document ID, or the full, populated document */
value: number | string | TypedCollection
}
[TCollectionSlug in CollectionSlug]: {
fields: TUploadExtraFieldsData
// Every lexical node that has sub-fields needs to have a unique ID. This is the ID of this upload node, not the ID of the linked upload document
id: string
relationTo: TCollectionSlug
// Value can be just the document ID, or the full, populated document
value: DataFromCollectionSlug<TCollectionSlug> | number | string
}
}[CollectionSlug]

// TODO: deprecate in Payload v4.
/**
* UploadDataImproved is a more precise type, and will replace UploadData in Payload v4.
* This type is for internal use only as it will be deprecated in the future.
* @internal
* @todo Replace UploadData with UploadDataImproved
*/
export type UploadDataImproved<TUploadExtraFieldsData extends JsonObject = JsonObject> = {
fields: TUploadExtraFieldsData
// Every lexical node that has sub-fields needs to have a unique ID. This is the ID of this upload node, not the ID of the linked upload document
id: string
relationTo: string
// Value can be just the document ID, or the full, populated document
value: (FileData & TypeWithID) | number | string
}
[TCollectionSlug in UploadCollectionSlug]: {
fields: TUploadExtraFieldsData
// Every lexical node that has sub-fields needs to have a unique ID. This is the ID of this upload node, not the ID of the linked upload document
id: string
relationTo: TCollectionSlug
// Value can be just the document ID, or the full, populated document
value: number | string | TypedUploadCollection[TCollectionSlug]
}
Comment on lines -40 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think TypedUploadCollection[TCollectionSlug] is significantly less precise than (FileData & TypeWithID).

Before this PR:
image

After this PR:
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only be the case in our monorepo though - it will be more precise in a real project

}[UploadCollectionSlug]

export function isGoogleDocCheckboxImg(img: HTMLImageElement): boolean {
return (
Expand Down
Loading