cometkim / gatsby-plugin-typegen

Let's give developers using GatsbyJS better DX with extreme type-safety
https://www.gatsbyjs.org/packages/gatsby-plugin-typegen/
MIT License
204 stars 23 forks source link

Scalar JSON always undefined due to default config of JSON set to type never #146

Closed mhretab closed 2 years ago

mhretab commented 3 years ago

Hi,

in DEFAULT_SHARED_CONFIG scalar JSON is set to never and this is creating an issue for the generated type gatsbyImageData when using it with gatsby-source-contentful. Tried changing JSON type to any and that seems to fix it.

cometkim commented 3 years ago

Hi @mhretab Thanks for the reporting.

I quit my job 3 months ago and haven't had a chance to explore the new Gatsby APIs. However, I think I have had enough rest. I will try again soon and this will definitely be included in v3.

And please share your work if it is public. It will be very helpful for investigation.

kije commented 3 years ago

Also note that some gatsby source plugins set the type of some fields on the sourced node to type JSON. For example, with gatsby-source-craft, all date fields on the sourced nodes have type JSON.

Declaring the Scalar['JSON'] as type never in the generated typescript definition thus prevents the user of this plugin from using these fields without doing ugly and cumbersome typescript casts (e.g. (post.postDate as unknown) as string) all over the place

I think the type of Scalar['JSON'] should have type any as described above, or at least type unknown so if one wants to use the field only one cast (e.g. to string) is necessary, instead of two

dgattey commented 3 years ago

I had this exact problem today, where gatsbyImageData set on a Contentful type was always undefined and I couldn't figure out why. Temporarily changing Scalars['JSON'] in the generated file from never to any fixed it.

Got around it locally by not using Pick to pull out the gatsbyImageData field and instead manually typing it myself. Technically not typesafe but I know if that field is set it will have been generated by the image transformer and thus should be the correct type. Better than any!

import { GatsbyImage, IGatsbyImageData } from 'gatsby-plugin-image'
import * as React from 'react'

export type ImageData = GatsbyTypes.Maybe<
  Pick<GatsbyTypes.ContentfulAsset, 'id' | 'title' | 'description'> & {
    gatsbyImageData: IGatsbyImageData
  }
>

(for reference, this was the before)

import { GatsbyImage } from 'gatsby-plugin-image'
import * as React from 'react'

export type ImageData = GatsbyTypes.Maybe<
  Pick<GatsbyTypes.ContentfulAsset, 'id' | 'title' | 'description' | 'gatsbyImageData'>
>
dgattey commented 3 years ago

Out of curiosity why is JSON never to start with? It seems like that's just wrong.

cometkim commented 3 years ago

Because the JSON typename is actually reserved by Gatsby internal. https://github.com/gatsbyjs/gatsby/blob/6b4b7f81ec/packages/gatsby/src/schema/print.js#L33-L104

It was intentional to prevent trying to override it and I thought Gatsby itself doesn't expose JSON type which means unknown. Because basically the Gatsby query compiler can infer the shape from any data.

cometkim commented 3 years ago

typegen plugin provides types strict much as possible unless there is a specific reason, but if any is reported to be useful for realworld, it will do so.

dgattey commented 3 years ago

Thanks! any definitely works, but Record<string, any> may be even better given what JSON actually is

kije commented 3 years ago

@dgattey @cometkim beware Record<string, any> might not improve things/not work for the gatsbyImage use case, as helper functions from gatsby-plugin-image (like getImage) expect to get a ImageDataLike type passed as argument.

JSON: any would work without the need for a cast in this case, as all types can be casted to any (and vice versa). JSON: Record<string, any> on the other hand is not structurally equal to ImageDataLike and it cannot be casted to any type, thus Typescript will throw a type error since Record<string, any> is not guaranteed to have the required fields from ImageDataLike

Also, regardless of the gatsby image use case, something like [1,2,3] would also be valid JSON, so in this case Record<string, any> wouldn't be the correct type either.

Thus, IMHO JSON: any seems to be IMHO the better type to use

cometkim commented 3 years ago

Or I can manipulate the schema for field name gatsbyImageData to use ImageDataLike type directly

cometkim commented 2 years ago

This is fixed in v3 (currently RC)

As clear solution for most sites, the type of JSON is changed to any.

Support for more precise types for image queries is a major change and will be revisited in v4.