FormidableLabs / groqd

A schema-unaware, runtime and type-safe query builder for GROQ.
https://commerce.nearform.com/open-source/groqd
MIT License
221 stars 17 forks source link

Fields disappearing in makeSafeQueryRunner in select #245

Closed dennispassway closed 7 months ago

dennispassway commented 7 months ago

I have a website with multiple modules. The modules are an array of various types and these types have different content. Some of my fields are disappearing when I run them within makeSafeQueryRunner and I cannot figure out what I'm doing wrong. In the example below the 'title' and 'text' fields dissapear on the 'imageTextModule' _type after adding the image. If I remove the image field, the title and text do turn up, but I do not have the image.

I also miss the "items" fields in the other types, where items are arrays of objects. This field is not returned at all.

I have a runGroqDQuery as shown in the docs that looks like this:

export const runGroqDQuery = makeSafeQueryRunner(async (query, parameters) => {
  const response = await client.fetch(query, parameters, { next: { revalidate: 0 } });
  // console.log({ response: JSON.stringify(response, null, 2) });
  return response;
});

Logging response inside the function returns all the fields as I expect, and the query that gets passed is correct. The result of the runGroqDQuery function does miss the fields, and I cannot figure out why. What is happening inside the makeSafeQueryRunner that strips fields?

So in this example:

export async function getHomepage() {
  const response2 = await runGroqDQuery(getHomepageQuery, { slug: '' });
  const response3 = await client.fetch(getHomepageQuery.query, { slug: '' }, { next: { revalidate: 0 } });

  console.log({ response2: JSON.stringify(response2, null, 2) });

  return response3;
}

response2 misses fields, and response3 is complete (but is not typed since it does not use the query runner!)

The query that I use:

export const getHomepageQuery = q('*')
  .filter(`_type == '${CONTENT_TYPE_HOME}' && _id == '${CONTENT_TYPE_HOME}'`)
  // eslint-disable-next-line unicorn/prefer-spread
  .slice(0)
  .grab$({
    title: q.string().optional(),
    intro: q.string().optional(),
    modules: q('modules')
    .filter()
    .select({
      '_type == "content"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        text: q.array(q.contentBlock()),
      },
      '_type == "imageModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        display: q.string().optional(),
        image: sanityImage('image', { withAsset: ['base'] }).nullable(),
      },
      '_type == "stickyContent"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        items: q.array(
          q.object({
            title: q.string().optional(),
            content: q.array(q.contentBlock()),
          })
        ),
      },
      '_type == "imageTextModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        reversed: q.boolean().optional(),
        text: q.string().optional(),
        title: q.string().optional(),
        image: sanityImage('image', { withAsset: ['base'] }).nullable(),
      },
      '_type == "leftRightTextModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        content: q.array(q.contentBlock()),
        title: q.string().optional(),
      },
      '_type == "peopleModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        text: q.string().optional(),
        title: q.string().optional(),
      },
      '_type == "iconGridModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        text: q.string().optional(),
        title: q.string().optional(),
        items: q
          .array(
            q.object({
              itemTitle: q.string().optional(),
              itemText: q.string().optional(),
              icon: q.string().optional(),
            })
          )
          .nullable(),
      },
      '_type == "statisticsModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        items: q
          .array(
            q.object({
              statistic: q.string().optional(),
              title: q.string().optional(),
            })
          )
          .nullable(),
      },
      '_type == "imageCarouselModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        // items: q("items").filter().grab$({}),
      },
      '_type == "videoModule"': {
        _key: q.string().optional(),
        _type: q.string().optional(),
        theme: q.string().optional().nullable(),
        type: q.string().optional(),
        url: q.string().optional(),
      },
    }),
  });

Maybe I'm just writing something wrong in the select query but I cannot seem to figure this out. What I don't understand is why adding or removing some fields (for example the image), removes other fields...

Hope this is enough information for you to help me, thanks!

dennispassway commented 7 months ago

In the meantime i found that the fields do dissapear in the schema.parse(res) step in the makeSafeQueryRunner function. I found this issue and think this is related: https://stackoverflow.com/questions/75814140/why-is-zod-removing-a-valid-property-when-parseing

scottrippey commented 7 months ago

Thank you for the details! I do think it's related to that Zod issue, but it seems like something that can be solved. As I investigate, I wanted to share that I found a minimal reproduction, for reference:

import { runQuery } from "playground";
import { q } from "groqd";

runQuery(
  q("*", { isArray: true })
   .filterByType('pokemon')
   .select({
    'base.HP > 50': {
      name: q.string(), 
      HP: q("base.HP"),
      HIGH: q('true')
    },
    default: {
      name: q.string(),
      HP: q("base.HP"),
      LOW:  q('"THIS VALUE NEVER APPEARS"'),
    },
  })
);

Update: a solution

The problem (as stated below) is that at runtime, the schema checker runs each schema, one-by-one, until there's a match. In the first condition, the schema checks for name, HP but DOESN'T check for HIGH, because technically q(...) doesn't have any conditions. So, when data comes in from the default condition, it does contain valid name, HP, so the first matcher is used, and therefore omits the LOW property.

There are various solutions here; I need to make the schema more specific, by ensuring that HIGH gets validated. So, this works:

      HIGH: ['true', q.literal(true)],

Playground

scottrippey commented 7 months ago

Ok, I think I found some more information, and I might have a workaround.

The problem

At runtime, select runs the results through each Zod schema until it finds a "match". So what's happening here, is your first matcher is type == 'content' => { theme, text }, but that also matches the data that's coming from imageTextModule (because it has theme, text fields too). It looks like this would also happen for peopleModule, iconGridModule and so on. You only get the theme, text fields in the results, because Zod it thinks it’s a match, and it only copies known fields. This is all part of the behavior of Zod, so unfortunately, it's not something that could be easily fixed.

A workaround

To solve this, we need to make sure the matcher doesn't prematurely match, and an easy way to do this is by validating the type at runtime, like _type: q.literal("content"). For example:

/// ...
   .select({
      '_type == "content"': {
        _key: q.string().optional(),
        _type: q.literal("content"), // 👈 This makes sure we don't prematurely match during runtime
        theme: q.string().optional().nullable(),
        text: q.array(q.contentBlock()),
      },
/// ...

You will likely need to repeat this for all your types, since you never know when the fields will overlap.

dennispassway commented 7 months ago

Wow, great find. Thanks for figuring this out so quickly! It makes sense that this is how it works now you explain it, but it is not obvious when you follow the docs from groqd. I think it would be a good addition to the docs if somewhere it is explained how the matching within the zod parse works, and reference that from the select field.

My issue is solved, so thanks for that! If I can do anything to help update the docs let me know. I feel like you can explain it better like you did to me tho! :)

scottrippey commented 7 months ago

I'll add some of this explanation to the docs, thanks for the suggestion.

I'm also considering new API methods that would help this situation. For example, selectByType and grabByType, like:

.selectByType({ 
  content: { 
    theme: q.string(), 
    text: q.contentBlocks(),
  },
  imageTextModule: { 
    theme: q.string(), 
    text: q.contentBlocks(), 
    image: sanityImage(...),
  },
})