[AC3] Hard to migrate to v3 due to lots of queries with non-normalized data type

[chenesan]

Thank you for your work on AC3. The new declarative api like possibleTypes is great!
For the better api our team would like to migrate from AC2 to AC3, but found out it's hard -- because it's tedious to configure the merging behavior on AC3 when our application have lots of non-normalized data type.

We know that in AC3 the InMemoryCache will replace, rather than auto merging incoming data. Because, as doc and #5603 has said, the auto merging behavior in some cases may be incorrect. To solve the problem, either we have to setting keyFields for each non-normalized data type (#6663), or setting merge: true on each field (as conclusion in doc).

But we have lots of feature entry point and sub field is non-normalized, the data graph may be like:

{
  root {
    featureA { # It's an entry point for a feature, which has no identity and is non-normalized object.
       items # an array of featureA item, each item containing uuid
       fieldA
       fieldB
       #...lots of fields, may be non-normalized object,
       fieldN
    }
    featureB {
       # like feature A, may have an items and lots of fields.
    }
    featureC
    #... lots of feature
    featureN
}

The featureX for us is like an entry point with non normalized object, and often we want to query featureX with different fields:

query query1OnFeatureA {
  root {
    featureA {
      items
      fieldA
      fieldB
    }
  }
}

query query2OnFeatureA {
  root {
    featureA {
      items
      fieldsC
      fieldsD
    }
  }
}

To fix the warning for each query / type, as a result we will have to manually write hundreds of lines of code, just to handle the merging behavior. This is a main issue that our team cannot easily migrate from AC2 to AC3. I've also seen issue like #6794 complaining about this.

So, is there any better way to handle this in such situation? (I guess the merge: true is for this kind of simplification, but still not good enough).

Some suggestion

From my view, if it's possible, it might be good to bring the auto-merging behavior back as an option on InMemoryCache, maybe something like:

const cache = new InMemoryCache({
  autoMergingDenormalizedObject: true 
})

Yes we all know it's not theoretically safe. But in our team's experience with AC2, we didn't have any issue caused by the auto merging behavior. Also, if it indeed cause bug on some types in user code, we can let it respect the custom merge function to correct it.
To prevent misusing this, the default can be false and we can make the option remind user to use it carefully and consciously, like dangerouslyAutoMergingDenormalizedObject: 'YES_I_UNDERSTOOD'.

Thank you for reading this long issue :)

[pleunv]

I ran into the same difficulties here when trying to migrate a large codebase, and held off on the upgrade for now. Specifying and implementing merging across hundreds of fields sounds very cumbersome, and I feel like I still don't have a good grasp on the cache mechanics even after going through the docs numerous times. Especially when it comes to implementing various pagination mechanisms (sometimes on the same field) I feel quite lost.

I think a flag like the one above might be a bit too optimistic, but perhaps a way to specify a list of types that can be auto-merged? Or is this possible already today?

[benjamn]

We've been considering supporting an API for specifying type and field policies dynamically, as a fallback for types/fields that you haven't explicitly configured.

Here's what I've got so far:

new InMemoryCache({
  typePolicies: {
    Person: <explicit type policy for Person>,
    Book: <explicit type policy for Book>,
    ...

    policyForType(typename: string): TypePolicy | undefined {
      // Return a (partial) type policy for the given typename.
      // This function will be called only once per typename.
      // Note that the returned TypePolicy may have a policyForField method.
    },

    Query: {
      fields: {
        someField: <explicit field policy for Query.someField>,
        ...
      },

      // Like policyForType, this function is called at most once per Type.field,
      // and specifies fallback field policy properties (keyArgs, read, merge) for
      // the specified field. Any type policy can have a policyForField function.
      policyForField(fieldName: string): FieldPolicy | undefined {
        // How you implement isMergeableQueryField is up to you.
        if (isMergeableQueryField(fieldName)) {
          return { merge: true };
        }
      },
    },
  },
})

There are lots of details to work out regarding the precedence of explicit configuration over dynamic configuration, and how to merge multiple policies for the same type/field, but I believe all those questions have reasonable answers.

@chenesan @pleunv Does this kind of system sound like it might make things easier for you?

[benjamn]

One thing that TypeScript makes difficult: policyForType probably needs to be moved up a level (out of typePolicies), so it’s not in a position where policyForType looks like a type name.

[chenesan]

Looks good for our use case, Thanks for the great work @benjamn!

Just have some questions:

• So if we want to auto merge all the non-normalized fields, will we configure policyForType like this?

policyForType: () => ({
  policyForField: () => ({
    merge: true,
  })
})

• One thing I'm not sure about { merge: true } is (Maybe not related to this new api, but I'm confused when reading the doc), when doing recursively merge with mergeObject and reaching a normalized field, will InMemoryCache merge the incoming field or replace it? It seems that the doc only say that it will call custom merge function on the field:

  Fortunately, you can find a helper function called options.mergeObjects in the options passed to the merge function, which generally behaves the same as { ...existing, ...incoming }, except when the incoming fields have custom merge functions. When options.mergeObjects encounters custom merge functions for any of the fields in its second argument (incoming), those nested merge functions will be called before combining the fields of existing and incoming, as desired

  But not mentioning if it will do merging on normalized field that not define field policy.

[benjamn]

Configuring { merge: true } for everything probably won't work because there's no good auto-merging strategy for arrays. In practice, if you configure { merge: true } for an array field, the mergeObjects helper will throw at runtime, when incoming is an array. So you'd probably want to write out a custom generic merge(existing, incoming, options) {...} function that's more sensitive to the types involved. Since you're only writing that function once, it should be manageable to make it a little more sophisticated than { merge: true } or { merge: false }.

Fields that don't have a custom merge function are replaced rather than merged. In other words, the default merge strategy is like { merge: false }, but with some warnings around potentially risky (data-lossy) situations.

[chenesan]

Got it. So we also have to handle array case by ourselves, but at least we only need to write once with this new API 😄 Thanks for the work!
I'd keep this issue for people who might have the same problem.

[domkm]

We're running into the same issues. I posted a related question in Spectrum.

[Orodan]

Running into a similar issue with a lot of different objects, most of them have a "price" field (with a VAT, dutyFree and allTaxesIncluded value). They do not have an id and specifying a merge function for 50+ properties on big projects seems really error prone with AC3 at the moment :/

More details in here

[benjamn]

Two active PRs that I believe will help address the concerns raised here: #7065 and #7070

[freshollie]

In my case I am using Apollo as a way to build a large cache of data from a device as the user navigates through pages. None of this data is normalised, and I was looking for a solution which behaved the same as v2 - in that nested objects would automatically be merged for a given query, as long as the query variables matched the original.

My solution came in the form of using graphql-code-generator and @graphql-codegen/introspection to generate a json structure containing my schema types. I then used the following to automatically enable merging for all types in my schema, effectively similar to v2.

import introspection from "./__generated__/introspection.json";

const cache = new InMemoryCache({
  typePolicies: Object.fromEntries(
    introspection.__schema.types
      .filter(({ kind }) => kind === "OBJECT")
      .map(({ name }) => [name, { merge: true }])
  ),
});

Configuring { merge: true } for everything probably won't work because there's no good auto-merging strategy for arrays

Because this filters for only OBJECT types, this should not cause issues with arrays.

@chenesan could you try this in your codebase? Works great for me!

[chenesan]

Thank you @freshollie! Looks like a possible solution.
I might not have enough time to try this (and new AC v3.3!) recently. Will be back if I get it done!

[hwillson]

It doesn't sound like there is an outstanding issue here, so closing. Thanks!

[Orodan]

I find it quite disapointing to close this issue on the sole basis that's it's not "outstanding" and with no real official workaround yet. It's still blocking people from migrating to the last version of this awesome library 😞

[hwillson]

@Orodan can you be specific about what's outstanding here? As mentioned in #6808 (comment), we merged 2 PR's to help with this. Would you mind opening a new issue with the details that you feel are still outstanding, so it's easier for us to track (and ultimately resolve)? Thanks!

[wanchaiS]

I still think it would be nice if we have an option as @chenesan suggested from the first place, in large project we have graphtypes that do not have an id as the author shown in example, even we follow the possibleTypes approach still now we have to maintain the json file. Even if we have the auto genration for the json file still the logic to determine which graphtype to merge cannot be explicit