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

Schema extract always assumes inline referenced documents are References #6913

Closed
obrassard opened this issue Jun 12, 2024 · 18 comments · Fixed by #7366
Closed

Schema extract always assumes inline referenced documents are References #6913

obrassard opened this issue Jun 12, 2024 · 18 comments · Fixed by #7366
Assignees
Labels
bug typegen Issues related to TypeScript types generation

Comments

@obrassard
Copy link

I have a document containing an array field, mixing different types of reference and objects :

defineField({
      name: 'modules',
      title: 'Modules',
      type: 'array',
      group: 'editorial',
      of: [
        { type: 'reference', name: 'ref.module.fullWidthTextBlock', to: [{ type: 'module.fullWidthTextBlock' }] },
        { type: 'reference', name: 'ref.module.textWithImageBlock', to: [{ type: 'module.textWithImageBlock' }] },
        { type: 'reference', name: 'ref.module.fullWidthImageBlock', to: [{ type: 'module.fullWidthImageBlock' }] },
        // ... Other reference types here
        { type: 'module.instagramBlock' }, // Object
      ],
    }),

We fetch this data using a query like this :

export const SanityProductModulesQuery = groq`
*[_type == 'product' && store.slug.current == $handle][0].modules[] {
    _type == 'module.instagramBlock' => {
        _key,
        _type,
        "title": title[$lang],
        category,
    },
    _type == 'ref.module.textWithImageBlock' => @ -> {
        _type,
        "_key": ^._key,
        "subTitle": subTitle[$lang],
        "title": title[$lang],
        "body": body[$lang],
        color,
        orientation
    },
    _type == 'ref.module.fullWidthTextBlock' => @ -> {
        _type,
        "_key": ^._key,
        "title": title[$lang],
        "body": body[$lang],
    }
}
`;

The Generated Types for this query is incorrect :

export type SanityProductModulesQueryResult = Array<
  | {
      _key: string;
      _type: 'module.instagramBlock';
      title: Array<{
        _type: 'localizedString';
        fr: string;
        en: string;
      }>;
      category: string;
    }
  | {} //We should have other types here, for referenced items
> | null;

The query itself work properly when tested in the vision plugin.


Which versions of Sanity are you using?

@sanity/cli (global)          3.45.0 (up to date)
@sanity/asset-utils            1.3.0 (up to date)
@sanity/color-input            3.1.1 (up to date)
@sanity/eslint-config-studio   3.0.1 (latest: 4.0.0)
@sanity/icons                  3.2.0 (up to date)
@sanity/language-filter        4.0.2 (up to date)
@sanity/ui                     2.3.1 (latest: 2.3.3)
@sanity/vision                3.45.0 (up to date)
sanity                        3.45.0 (up to date)

What operating system are you using?
Mac OS 14.5 / Chrome

Which versions of Node.js / npm are you running?

NPM : 9.8.1
NODE : v18.18.2

@obrassard
Copy link
Author

I thought this issue might be related to #6555 so I removed the => @ to test like this :

export const SanityProductModulesQuery = groq`
*[_type == 'product' && store.slug.current == $handle][0].modules[] {
    _type == 'module.instagramBlock' => {
        _key,
        _type,
        "title": title[$lang],
        category,
    },
    _type == 'ref.module.textWithImageBlock' => {
        _type,
        _key,
        _ref,
    },
    _type == 'ref.module.fullWidthTextBlock' => {
        _type,
        _key,
        _ref,
    }
}
`;

However, the result stays the same, and the generated Array type only contains the correct type for module.instagramBlock (which is the only possible object in our schema).

I have the impression that the problem has more to do with the fact that we have several references with different names, and the parser doesn't resolve properly those custom _type (eg _type == 'ref.module.textWithImageBlock')

@rexxars rexxars added the typegen Issues related to TypeScript types generation label Jun 18, 2024
@sgulseth sgulseth added the bug label Jun 24, 2024 — with Linear
@sgulseth
Copy link
Member

sgulseth commented Jul 2, 2024

Hi! Can you try with the latest version we just released? v3.49.0

@obrassard
Copy link
Author

Hi @sgulseth, unfortunately the issue persist event after upgrading to v3.49.0 :

⚡npx sanity versions
@sanity/cli (global)          3.49.0 (up to date)
@sanity/asset-utils            1.3.0 (up to date)
@sanity/color-input            3.1.1 (up to date)
@sanity/eslint-config-studio   4.0.0 (up to date)
@sanity/icons                  3.2.0 (up to date)
@sanity/language-filter        4.0.2 (up to date)
@sanity/ui                     2.6.1 (up to date)
@sanity/vision                3.49.0 (up to date)
sanity                        3.49.0 (up to date)

@obrassard
Copy link
Author

@sgulseth We noticed another issue that might be related to this one.

For context, we have different document types that represent pages sections (or modules) of our website. These sections are referenced in different other content type.

However, in one particular case we wanted to use some of those section schema without them being referenced. For this case, we've been using our document types as if they were objects, in an array like this :

defineField({
      title: 'Page modules',
      name: 'modules',
      description: 'Editorial sections displayed on the page',
      type: 'array',
      of: [
        { type: 'module.textWithImage' },
        { type: 'module.carousel' },
        { type: 'module.accordion' },
      ],
    }),

Where all "modules" aredocument types. This work's fine in the editor and the API, however the TypeGen doesn't seem to understand some query properly. For instance, when using conditional and projections like the following, the resulting type is Array<{}> | null.

    _type == "module.textWithImage" => {
        _type,
        name,
    }

It is noteworthy that once the type of blogArticle is changed from document to object, the type is generated properly.

@sgulseth
Copy link
Member

sgulseth commented Jul 25, 2024

Thanks for letting us know, @obrassard! I suspect this is related to sanity-io/groq-js#250 as we weren't handling splatting over optional fields very well.

A workaround, and to confirm the hypothesis, would be to mark the Pages modules field as required and export the schema with --enforce-required-fields. If it's still an issue it would help a lot to get a small production setup

@sgulseth sgulseth self-assigned this Jul 25, 2024
@obrassard
Copy link
Author

obrassard commented Jul 25, 2024

@sgulseth Unfortunately adding a required validation on the modules field did not solve the issue.


To help you diagnose the issue, I created a sample repository here : https://github.com/obrassard/sanity-typegen-issues containing an example of the two problems mentioned here.

In the example you'll find 4 types of documents (two types of page modules), a product document and an article document.

  • In product, modules documents are included as "named" references.
  • In article, modules documents are included as if they where objects (no references)

There is two different queries in the groq directory that were used to fetch an article or a product based on their slug. In both case the generated types are incorrect for the modules field :

Capture d’écran, le 2024-07-25 à 09 57 30

Note that you can use npm run codegen to regenerate the types in this project.

Let me know if you need more info ! 😄

@obrassard
Copy link
Author

On other branches of the example repo, you may also notice how

  1. Changing the type of both modules documents to object "fixes" the generated type for the article document : obrassard/sanity-typegen-issues@main...article-objects

  2. Changing how we define the references in the product document, fixes generation of the product document : obrassard/sanity-typegen-issues@main...product-reference

@sgulseth
Copy link
Member

sgulseth commented Jul 25, 2024

Ah, they are referencing another document. Then you need to dereference the referenced document. So instead of doing:

modules[] {
...
}

do:

modules[]-> {
...
}

@obrassard
Copy link
Author

@sgulseth

In the case of the article document, the modules are not referenced, as the field is defined as such :

defineField({
    name: 'modules',
    type: 'array',
    validation: Rule => Rule.required(),
    // No reference here
    of: [
      { type: 'module.textBlock' },
      { type: 'module.textImageBlock' },
    ],
  })

For the Product document, they are indeed referenced, however the query we wrote already include the dereferencing operator for each ref :

Capture d’écran, le 2024-07-25 à 15 19 00

@sgulseth
Copy link
Member

sgulseth commented Jul 25, 2024

I see. The problem is still similar: Inside of modules[] the object is a reference object, and look something like:

{
  _key: 'someKey',
  _type: 'reference',
  _ref: '...'
  _weak: true/false
}

So you can't compare _type to the referenced document type. What you want is something like:

 modules[] {
      _key,
      ...(@-> {
		_type,
        _type == 'module.textBlock' => {
          "title": ${localized('title')},
          "body": ${localized('body')},
        },
        _type == 'module.textImageBlock' => {
          "subTitle": ${localized('subTitle')},
          "title": ${localized('title')},
          "body": ${localized('body')},
          image
        }
    })
  }

here we dereference @ before comparing _type.

Running this locally gives me:

export type SanityProductModulesQueryResult = {
  title: string
  slug: string
  modules: Array<
    | {
        _key: string
        _type: 'module.textBlock'
        title: string
        body: string
      }
    | {
        _key: string
        _type: 'module.textImageBlock'
        subTitle: string | null
        title: string
        body: string
        image: {
          asset?: {
            _ref: string
            _type: 'reference'
            _weak?: boolean
            [internalGroqTypeReferenceTo]?: 'sanity.imageAsset'
          }
          hotspot?: SanityImageHotspot
          crop?: SanityImageCrop
          _type: 'image'
        }
      }
  >
} | null

@obrassard
Copy link
Author

obrassard commented Jul 25, 2024

Thanks @sgulseth I hadn't thought of writing the product query this way, but indeed by modifying the query as you proposed, it works as expected. Note, however, that the initial query (with _type == 'ref.module.textBlock' => @ -> {) did work as expected in the API and the Vision plugin (therefore I would have expected the TypeGen plugin to be able to parse it).

On the other hand, it doesn't solve the second case (BlogArticleQuery) where there is no reference.

@sgulseth
Copy link
Member

np!

did work as expected in the API and the Vision plugin

That is weird! That might indicate that the schema model and your content model isn't 1-1


Is the reproduction repo you pushed up to date with what you have that doens't work? SanityBlogArticleQueryResult is correct when I do: modules[]-> {

@obrassard
Copy link
Author

obrassard commented Jul 25, 2024

Is the reproduction repo you pushed up to date with what you have that doens't work? SanityBlogArticleQueryResult is correct when I do: modules[]-> {

Sorry for the confusion, actually adding the -> operator does generate the correct type for SanityBlogArticleQueryResult, however the query itself does not work as you can see below :

Before
Capture d’écran, le 2024-07-25 à 16 05 55

After
Capture d’écran, le 2024-07-25 à 16 05 45

It does not work, because unlike the other query the modules are not referenced here (even tho they are declared as documents they are used as if they were objects)

@obrassard
Copy link
Author

obrassard commented Jul 25, 2024

@sgulseth While testing the new query with ...(@-> { on our real website, I noticed another issue.

If you have an array mixing references and non-refeferenced objects, the result is not correct.

I updated the example repo, and as you can see here the module.objectBlock has been added to the product's modules[] field. This is not a reference.

When generating the type, we got :

modules: Array<
    | {
        _key: string
        _type: 'module.textBlock'
       ...
      }
    | {
        _key: string
        _type: 'module.textImageBlock'
        ...
      }
    | unknown
  >

But we should have :

modules: Array<
    | {
        _key: string
        _type: 'module.textBlock'
       ...
      }
    | {
        _key: string
        _type: 'module.textImageBlock'
        ...
      }
    | {
        _key: string
        _type: 'module.objectBlock'
       ...
      }
  >

@sgulseth
Copy link
Member

Sorry for the confusion, actually adding the -> operator does generate the correct type for SanityBlogArticleQueryResult, however the query itself does not work as you can see below :

From what I can tell based on your vision result is that your schema has diverged from the content. In your schema it's references, but in the content lake it's stored inline.

If you have an array mixing references and non-refeferenced objects, the result is not correct.

yes, then you also need to compare the types inside the modules container. Ie something like:

 modules[] {
        _key,
        _type == 'module.objectBlock' => {
            _type,
            "title": ${localized('title')},
        },
        _type == 'reference' => {
          ...(@ -> {
              _type == 'module.textBlock' => {
                  _type,
                  "title": ${localized('title')},
                  "body": ${localized('body')},
              },
              _type == 'module.textImageBlock' =>  {
                  _type,
                  "subTitle": ${localized('subTitle')},
                  "title": ${localized('title')},
                  "body": ${localized('body')},
                  image
              },
          })
        }
    }

generates:

modules: Array<
    | {
        _key: string
        _type: 'module.objectBlock'
        title: string
      }
    | {
        _key: string
        _type: 'module.textBlock'
        title: string
        body: string
      }
    | {
        _key: string
        _type: 'module.textImageBlock'
        subTitle: string | null
        title: string
        body: string
        image: {
          asset?: {
            _ref: string
            _type: 'reference'
            _weak?: boolean
            [internalGroqTypeReferenceTo]?: 'sanity.imageAsset'
          }
          hotspot?: SanityImageHotspot
          crop?: SanityImageCrop
          _type: 'image'
        }
      }
  >

@obrassard
Copy link
Author

From what I can tell based on your vision result is that your schema has diverged from the content. In your schema it's references, but in the content lake it's stored inline.

In the definition of the article document, the modules field is defined without using reference :

defineField({
    name: 'modules',
    type: 'array',
    validation: Rule => Rule.required(),
    // No reference here
    of: [
      { type: 'module.textBlock' },
      { type: 'module.textImageBlock' },
    ],
  })

E.g. If I add a text block in an article, the text block data is saved directly in the article document itself, not in a module.textBlock document referenced by the article. Therefore, it makes sense that using the dereferencing operator returns null.

It seems that the issue is with the sanity schema extract command, which incorrectly assumes that module.textBlock and module.textImageBlock will be referenced within articles document. However, this is incorrect because it is not an array of: [{ type: 'reference'

// Not this
of: [
     { type: 'reference', to: [
        { type: 'module.textBlock' },
        { type: 'module.textImageBlock' },
    ]}
]

This assumption is probably made because module.textBlock and module.textImageBlock are defined as document and not object. It would explain why changing the type of both modules to object "fixes" the type generation : obrassard/sanity-typegen-issues@main...article-objects.

However, in our real case scenario, we would not want to define those modules as object because we need them to be referenced at different place. The article document is a special case where we exceptionally want to use the same objects schemas for our modules, without referencing to an external document.

Of course, we could have created other object with the same fields definition for each of our modules used in the article, but since we have a lot of different modules it was not practical to duplicate their definition. And, as we can read in the Sanity documentation, "there's no semantic difference between a document and an object", so it would make sense to use them like we did (and it works fine with the content lake API)

@sgulseth
Copy link
Member

sgulseth commented Jul 26, 2024

Ah. I see, you are using a document as an inline reference. This looks to be a bug in the schema extract, yes :)

@sgulseth sgulseth changed the title TypeGen does not generate correct type when querying an array containing multiple references types Schema extract always assumes inline referenced documents are References Jul 26, 2024
Copy link
Contributor

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug typegen Issues related to TypeScript types generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants