fix(apollo-forest-run): tolerate divergent null/object node chunks during update

change/@graphitation-apollo-forest-run-6b6eb87f-21ce-43f0-b729-efe2047dfa19.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix(apollo-forest-run): tolerate divergent null/object node chunks during update",
+  "packageName": "@graphitation/apollo-forest-run",
+  "email": "vrazuvaev@microsoft.com_msteamsmdb",
+  "dependentChangeType": "patch"
+}

packages/apollo-forest-run/src/__tests__/regression.test.ts

@@ -1559,6 +1559,152 @@ test("correctly handles optimistic fragment write for deeply nested node", () =>
   ]);
 });
 
+test("update must not crash when a node field is null in one chunk and an object in another", () => {
+  // Aliasing produces an Object chunk and a CompositeNull chunk for the same
+  // node; a later ObjectDifference is applied to both, asserting
+  // isObjectValue(base) on the null chunk.
+  const cache = new ForestRun();
+
+  const aliasedQuery = gql`
+    query Aliased {
+      x: foo {
+        __typename
+        id
+        details {
+          __typename
+          a
+          b
+        }
+      }
+      y: foo {
+        __typename
+        id
+        details {
+          __typename
+          a
+          b
+        }
+      }
+    }
+  `;
+  const singleQuery = gql`
+    query Single {
+      foo {
+        __typename
+        id
+        details {
+          __typename
+          a
+          b
+        }
+      }
+    }
+  `;
+
+  // Same node written twice in one operation: object via `x`, null via `y`.
+  cache.write({
+    query: aliasedQuery,
+    result: {
+      x: {
+        __typename: "Foo",
+        id: "1",
+        details: { __typename: "Detail", a: 1, b: 1 },
+      },
+      y: {
+        __typename: "Foo",
+        id: "1",
+        details: null,
+      },
+    },
+  });
+
+  // Changing details yields one ObjectDifference applied to every chunk,
+  // including the CompositeNull one.
+  const writeChanged = () =>
+    cache.write({
+      query: singleQuery,
+      result: {
+        foo: {
+          __typename: "Foo",
+          id: "1",
+          details: { __typename: "Detail", a: 2, b: 1 },
+        },
+      },
+    });
+
+  expect(writeChanged).not.toThrow();
+});
+
+test("update must not crash when the same node id appears twice with divergent details (no alias)", () => {
+  // Same regression via a normalized node-id collision rather than aliasing:
+  // the same node id appears twice in one list result with `details` an object
+  // in one occurrence and null in the other, yielding an Object chunk and a
+  // CompositeNull chunk without aliasing.
+  const cache = new ForestRun();
+
+  const listQuery = gql`
+    query List {
+      foos {
+        __typename
+        id
+        details {
+          __typename
+          a
+          b
+        }
+      }
+    }
+  `;
+  const singleQuery = gql`
+    query Single {
+      foo {
+        __typename
+        id
+        details {
+          __typename
+          a
+          b
+        }
+      }
+    }
+  `;
+
+  // Same node id twice in one list result: object then null.
+  cache.write({
+    query: listQuery,
+    result: {
+      foos: [
+        {
+          __typename: "Foo",
+          id: "1",
+          details: { __typename: "Detail", a: 1, b: 1 },
+        },
+        {
+          __typename: "Foo",
+          id: "1",
+          details: null,
+        },
+      ],
+    },
+  });
+
+  // Changing details yields one ObjectDifference applied to every chunk,
+  // including the CompositeNull one.
+  const writeChanged = () =>
+    cache.write({
+      query: singleQuery,
+      result: {
+        foo: {
+          __typename: "Foo",
+          id: "1",
+          details: { __typename: "Detail", a: 2, b: 1 },
+        },
+      },
+    });
+
+  expect(writeChanged).not.toThrow();
+});
+
 test("merge policy on embedded object must not crash when a prior operation wrote null (draftHelpers:72)", () => {
   // A keyless ("embedded") type with a field `merge` policy. If a previous
   // operation wrote that position as an explicit `null`, the merge machinery
@@ -1695,3 +1841,77 @@ test("merge policy on embedded object must not crash when a prior null operation
     },
   });
 });
+
+test("update must not crash when a list item is null in one chunk and an object in another", () => {
+  // Same divergent-chunk regression, but the null/object divergence is on a *list item*
+  // rather than a field. The list difference is applied to every chunk of the node, so the
+  // per-item ObjectDifference reaches the CompositeNull element of the divergent chunk
+  // (updateValue is called for list items without a null guard). It must be skipped, not crash.
+  const cache = new ForestRun();
+
+  const aliasedQuery = gql`
+    query Aliased {
+      x: foo {
+        __typename
+        id
+        items {
+          __typename
+          value
+        }
+      }
+      y: foo {
+        __typename
+        id
+        items {
+          __typename
+          value
+        }
+      }
+    }
+  `;
+  const singleQuery = gql`
+    query Single {
+      foo {
+        __typename
+        id
+        items {
+          __typename
+          value
+        }
+      }
+    }
+  `;
+
+  // Same node written twice in one operation: item object via `x`, null item via `y`.
+  cache.write({
+    query: aliasedQuery,
+    result: {
+      x: {
+        __typename: "Foo",
+        id: "1",
+        items: [{ __typename: "Item", value: 1 }],
+      },
+      y: {
+        __typename: "Foo",
+        id: "1",
+        items: [null],
+      },
+    },
+  });
+
+  // Changing the item yields one CompositeListDifference (with a per-item ObjectDifference)
+  // applied to every chunk, including the one whose item is CompositeNull.
+  const writeChanged = () =>
+    cache.write({
+      query: singleQuery,
+      result: {
+        foo: {
+          __typename: "Foo",
+          id: "1",
+          items: [{ __typename: "Item", value: 2 }],
+        },
+      },
+    });
+
+  expect(writeChanged).not.toThrow();
+});

packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts

@@ -35,6 +35,41 @@ describe("replaceValue invariant", () => {
       'Invariant violation: Failed to update "query ReplaceScalar": cannot replace Scalar with Object (of type Widget)',
     );
   });
+
+  it("still reports the invariant when path/node resolution throws", () => {
+    const operation = createTestOperation(gql`
+      query ReplaceObject {
+        node {
+          id
+        }
+      }
+    `);
+    // base is an addressable, keyless embedded object, so both path resolution and
+    // enclosing-node resolution must ascend the tree via findParent. A corrupt tree can
+    // make findParent throw while an invariant is being reported; the message builder must
+    // swallow that and still surface the invariant rather than the internal exception.
+    const objectBase = resolveFieldValue(
+      generateChunk(`query ObjectSource { embedded { field } }`).value,
+      "embedded",
+    ) as ObjectChunk;
+    const listReplacement = resolveFieldValue(
+      generateChunk(`query ListSource { items @mock(count: 2) { id } }`).value,
+      "items",
+    );
+
+    const context = {
+      operation,
+      findParent: () => {
+        throw new Error("corrupt tree: findParent exploded");
+      },
+    } as unknown as UpdateTreeContext;
+
+    expect(() =>
+      replaceValue(context, objectBase as any, listReplacement as any),
+    ).toThrow(
+      'Invariant violation: Failed to update "query ReplaceObject": cannot replace Object with CompositeList',
+    );
+  });
 });
 
 const nestedFeedListQuery = gql`
@@ -83,6 +118,9 @@ const NESTED_INCOMPATIBLE_OBJECT_DIFF_ERROR =
   'Invariant violation: Failed to update "query BaseFeedQuery" ' +
   "at path listContainer.items.0.value (in Entity): expected CompositeList, got ObjectDifference";
 
+const NESTED_DIVERGENT_NULL_WARNING_PATH =
+  "at path listContainer.items.0.value (in Entity)";
+
 describe("incompatible object difference invariant", () => {
   it("reports operation and path for a field two levels deep below a list", () => {
     const cache = new ForestRun();
@@ -131,4 +169,53 @@ describe("incompatible object difference invariant", () => {
       NESTED_INCOMPATIBLE_OBJECT_DIFF_ERROR,
     );
   });
+
+  it("reports the full path in the divergent-null warning without throwing", () => {
+    // A divergent null/object chunk no longer throws: the null chunk is skipped with a
+    // warning. That warning must still report the full path, even though a CompositeNull's
+    // source data is `null` and is not addressable by the path utils on its own - the path
+    // is derived from its parent chunk, yielding "listContainer.items.0.value".
+    const warnings: string[] = [];
+    const cache = new ForestRun({
+      logger: {
+        ...console,
+        warn: (...args: unknown[]) => warnings.push(args.join(" ")),
+      },
+    });
+
+    // The same Entity's "value" is a null (with a sub-selection, i.e. a CompositeNull)
+    // under "listContainer" and an object under "objectContainer".
+    cache.write({
+      query: nestedFeedListQuery,
+      result: {
+        objectContainer: {
+          __typename: "Container",
+          id: "object",
+          items: [{ __typename: "Entity", id: "1", value: { note: "old" } }],
+        },
+        listContainer: {
+          __typename: "Container",
+          id: "list",
+          items: [{ __typename: "Entity", id: "1", value: null }],
+        },
+      },
+    });
+
+    const run = () =>
+      cache.write({
+        query: nestedFeedObjectQuery,
+        result: {
+          objectContainer: {
+            __typename: "Container",
+            id: "object",
+            items: [{ __typename: "Entity", id: "1", value: { note: "new" } }],
+          },
+        },
+      });
+
+    expect(run).not.toThrow();
+    expect(
+      warnings.some((w) => w.includes(NESTED_DIVERGENT_NULL_WARNING_PATH)),
+    ).toBe(true);
+  });
 });