fix(richtext-lexical): incorrect UploadData types #11288
Conversation
packages/richtext-lexical/src/features/upload/server/nodes/UploadNode.tsx
Show resolved
Hide resolved
packages/richtext-lexical/src/features/upload/server/nodes/UploadNode.tsx
Show resolved
Hide resolved
| @@ -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') { | |||
There was a problem hiding this comment.
I guess ts was asking for optional chaining here? any reason why it is needed now? afaik value should be defined
| 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] | ||
| } |
There was a problem hiding this comment.
I think TypedUploadCollection[TCollectionSlug] is significantly less precise than (FileData & TypeWithID).
Before this PR:
After this PR:
There was a problem hiding this comment.
This will only be the case in our monorepo though - it will be more precise in a real project
| 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} /> | ||
| } |
There was a problem hiding this comment.
Now almost all things derived from value are "any"
There was a problem hiding this comment.
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.
This PR fixes the
UploadDatatype that was weakened in a previous PR, causing a breaking change. It also improves the newly addedUploadDataImprovedtype by bringing back its support for generated types and using theUploadCollectionSlugtype helper to restrict collection slugs to upload-enabled collections.