lorenzodejong / next-sanity-image

Utility for using responsive images hosted on the Sanity.io CDN with the Next.js image component.
MIT License
149 stars 20 forks source link

Typescript: Issue with StaticImport type #16

Closed RyKilleen closed 3 years ago

RyKilleen commented 3 years ago

Hi there!

It seems like something with Next/Image's complex types is causing some trouble with the next-sanity-image library (screenshot below.)

It seems like no matter what useNextSanityImage returns, next/image gets mad that src is a string instead of a StaticImport. Any typescript guidance would be appreciated!

Error screenshot: image

Relevant Code ```typescript import { ImageUrlBuilder } from '@sanity/image-url/lib/types/builder' import { SanityImageSource } from '@sanity/image-url/lib/types/types' import { useNextSanityImage, UseNextSanityImageBuilderOptions } from 'next-sanity-image' import Image from 'next/image' import React, { FunctionComponent } from 'react' import configuredSanityClient from './sanity' // This custom url builder insists on using the images' width as the max width instead of scaling the image up. // It's likely that we'll need to use different imgBuilders with different behaviors for different Next/Image layout types const customImageURLBuilder = ( imageUrlBuilder: ImageUrlBuilder, options: UseNextSanityImageBuilderOptions, ) => { return imageUrlBuilder .width( options.width && options.width != 0 ? Math.min(options.originalImageDimensions.width, options.width) : options.originalImageDimensions.width, ) .auto('format') } const NextSanityImageWithCustomBuilder = (image: SanityImageSource) => useNextSanityImage(configuredSanityClient, image, { imageBuilder: customImageURLBuilder as any, enableBlurUp: false }) type NextSanityImageProps = { alt?: string, image: SanityImageSource, layout?: 'fill' | 'fixed' | 'intrinsic' | 'responsive' | undefined } const NextSanityImage: FunctionComponent = ({ image, layout = 'intrinsic', alt, ...rest }) => { const imageProps = NextSanityImageWithCustomBuilder(image) if (layout === 'fill') { return ( {alt} ) } else { return ( {alt} ) } } export default NextSanityImage ```
lorenzodejong commented 3 years ago

Thanks for providing the extensive code example. I tried it out and noticed that the union defined on the ImageProps type from next/image switches to ObjectImageProps instead of StringImageProps because imageProps is potentially null. This is a change made in one of the recent releases, whenever you don't provide an image or the image can't be loaded the hook will return null as well.

Adding a null check for imageProps solves the issue in your code. Perhaps an additional function overload in the hook could solve this issue, because in your code the image is always defined (judging from the types of the component). I'll take a look at this to see if/how it's possible.

RyKilleen commented 3 years ago

Thanks for the insight! Meant to close this at the time, we ended up strongly typing it on our end to solve the issue, thanks for confirming it was some oddities upstream

gpoole commented 3 years ago

@lorenzodejong you mentioned an overload might be possible, would that still be worthwhile? As a TypeScript noob this caused me some confusion and it seems to happen with a simple use case:

const SanityImage: FC<{ image: SanityImageSource }> = ({ image }) => {
  const imageProps = useNextSanityImage(getSanityClient(), image)
  return <NextImage {...imageProps} layout='intrinsic' />
}

I've noticed this type has changed since your original discussion.

lorenzodejong commented 3 years ago

@gpoole i've just release a function overload which should solve this issue. Please check out release 3.1.6 to see if this has fixed your scenario!

gpoole commented 3 years ago

Thanks @lorenzodejong, much appreciated! The issue is fixed.