fix(core): dont crash when image url string is passed to preview

[sjelfull]

Description

We currently have a situation where you can use a asset URL as a media preview value, and this will crash in the context of a block preview inside the PTE.

Here is a example schema:

defineField({
    type: 'image',
    icon: ImageIcon,
    name: 'imageWithAssetUrlPreview',
    title: 'Image w/ asset url preview',
    options: {
      hotspot: true,
    },
    preview: {
      select: {
        media: 'asset.url',
      },
      prepare({media}) {
        return {media, title: 'Image w/ asset url preview'}
      },
    },
  }),

Before this change the value would be passed on to the default preview component, where we had these conditionals:

 if (isValidElementType(mediaProp)) {
    return mediaProp
  }

  if (isValidElement(mediaProp)) {
    return mediaProp
  }

  if (isImageSource(mediaProp)) {
    return renderMedia
  }

  // Handle image urls
  if (isString(imageUrl)) {
    return (
      <img
        src={imageUrl}
        alt={isString(title) ? title : undefined}
        referrerPolicy="strict-origin-when-cross-origin"
      />
    )
  }

isValidElementType(mediaProp) would actually be true for any string here, meaning it would be passed on to createElement, and in turn it would crash.

After the change it is correctly handled, and the passed Sanity image URL will be transformed to the same dimensions as any other preview image.

What to review

• Does the logic change make sense? Was there a reason we ordered the conditional checking, or was it simply a oversight that isValidElementType will return true for any string?

Notes for release

• Fixes a situation where setting the media preview value of a image to asset.url would crash inside the Portable Text Editor

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
page-building-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2024 0:44am
performance-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2024 0:44am
test-compiled-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2024 0:44am
test-next-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2024 0:44am
test-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2024 0:44am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
studio-workshop	⬜️ Ignored (Inspect)	Visit Preview		Jun 5, 2024 0:44am

[github-actions[bot]]

No changes to documentation

[github-actions[bot]]

Component Testing Report Updated Jun 5, 2024 12:47 PM (UTC)

File	Status	Duration	Passed	Skipped	Failed
comments/CommentInput.spec.tsx	✅ Passed (Inspect)	35s	15	0	0
formBuilder/ArrayInput.spec.tsx	✅ Passed (Inspect)	6s	3	0	0
formBuilder/inputs/PortableText/Annotations.spec.tsx	✅ Passed (Inspect)	26s	6	0	0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx	✅ Passed (Inspect)	31s	11	7	0
formBuilder/inputs/PortableText/Decorators.spec.tsx	✅ Passed (Inspect)	14s	6	0	0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx	✅ Passed (Inspect)	8s	3	0	0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx	✅ Passed (Inspect)	36s	15	0	0
formBuilder/inputs/PortableText/Input.spec.tsx	✅ Passed (Inspect)	1m 17s	21	0	0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx	✅ Passed (Inspect)	1m 1s	18	0	0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx	✅ Passed (Inspect)	7s	3	9	0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx	✅ Passed (Inspect)	21s	9	0	0
formBuilder/inputs/PortableText/Styles.spec.tsx	✅ Passed (Inspect)	14s	6	0	0
formBuilder/inputs/PortableText/Toolbar.spec.tsx	✅ Passed (Inspect)	29s	12	0	0

[RitaDias]

I now realise that I likely shouldn't have approved it without some tests but my question about them still remains on the other comment 😂

[sjelfull]

@RitaDias Thanks for the review! I'll see if I can add some quick tests for this, thanks for the reminder.

[bjoerge]

@sjelfull what's the status here? Are these changes still relevant or can we close this?

[sjelfull]

@bjoerge I think it might be relevant still, easy to check by using the schema above to reproduce. Missing tests though, and need to be rebased.