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.

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

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

Closed
mhretab opened this issue Mar 5, 2021 · 10 comments
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mhretab
Copy link

mhretab commented Mar 5, 2021

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 cometkim self-assigned this Mar 5, 2021
@cometkim cometkim added the bug Something isn't working label Mar 5, 2021
@cometkim
Copy link
Owner

cometkim commented Mar 5, 2021

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
Copy link

kije commented Mar 7, 2021

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
Copy link

dgattey commented Apr 28, 2021

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
Copy link

dgattey commented Apr 28, 2021

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

dgattey added a commit to dgattey/dg that referenced this issue Apr 28, 2021
1. Renames RichTextParagraph to just RichText
2. Creates an Image component that uses GatsbyImage to render gatsbyImageData
3. Does some fun with typing so that gatsbyImageData isn't undefined (see the bug at
cometkim/gatsby-plugin-typegen#146)
4. Loads images for projects & sections' paragraphs & displays them
@cometkim
Copy link
Owner

cometkim commented Apr 29, 2021

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
Copy link
Owner

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.

@cometkim cometkim added this to the v3 milestone Apr 29, 2021
@dgattey
Copy link

dgattey commented Apr 29, 2021

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

@kije
Copy link

kije commented Apr 29, 2021

@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 cometkim mentioned this issue May 10, 2021
Closed
30 tasks
@cometkim
Copy link
Owner

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

@cometkim
Copy link
Owner

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.

cometkim added a commit that referenced this issue Jun 12, 2022
rc.0
- implemente XState based scheduler
- stabilize schema output
- bump GCG
- upgrade yarn to v3
- rewritten core logics
- upgrade eslint to v8
- upgrade plugin's dependencies
- pin graphql version
- rewrite emitSchema service
- rewriten emitPluginDocument service
- rewriten codegen service
- add TS & Flow examples

rc.1
- skip running on cloud build
- fix config validation
- more contexture reporting
- turn `flattenGeneratedTypesIncludeFragments` true
- stabilize documents
- change maybe type to use `null` instead of `undefined`
- update README

rc.2
- add mdx example
- MDX example to validate regression of [#117]
- update dependencies
- added support for `GatsbyImageData` scalar

Resolves #40
Resolves #103
Resolves #104
Resolves #109
Fixes #113
Fixes #117
Fixes #120
Resolves #122
Fixes #124
Resolves #131
Fixes #139, #99
Resolves #146
Fixes #157
Fixes #159
Fixes #160
Fixes #161
Fixes #162
Resolves #168
Resolves #169
Fixes #171

[#117]: #117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants