fix(core): Keep order of nested relations during hydration (#2864) (#2865)

packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts

@@ -1,6 +1,5 @@
 import { Injectable } from '@nestjs/common';
 import { Type } from '@vendure/common/lib/shared-types';
-import { isObject } from '@vendure/common/lib/shared-utils';
 import { unique } from '@vendure/common/lib/unique';
 import { SelectQueryBuilder } from 'typeorm';
 
@@ -14,6 +13,7 @@ import { TranslatorService } from '../translator/translator.service';
 import { joinTreeRelationsDynamically } from '../utils/tree-relations-qb-joiner';
 
 import { HydrateOptions } from './entity-hydrator-types';
+import { mergeDeep } from './merge-deep';
 
 /**
  * @description
@@ -134,7 +134,7 @@ export class EntityHydrator {
                 const hydrated = await hydratedQb.getOne();
                 const propertiesToAdd = unique(missingRelations.map(relation => relation.split('.')[0]));
                 for (const prop of propertiesToAdd) {
-                    (target as any)[prop] = this.mergeDeep((target as any)[prop], hydrated[prop]);
+                    (target as any)[prop] = mergeDeep((target as any)[prop], hydrated[prop]);
                 }
 
                 const relationsWithEntities = missingRelations.map(relation => ({
@@ -306,51 +306,4 @@ export class EntityHydrator {
             ? input[0]?.hasOwnProperty('translations') ?? false
             : input?.hasOwnProperty('translations') ?? false;
     }
-
-    /**
-     * Merges properties into a target entity. This is needed for the cases in which a
-     * property already exists on the target, but the hydrated version also contains that
-     * property with a different set of properties. This prevents the original target
-     * entity from having data overwritten.
-     */
-    private mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b: T): T {
-        if (!a) {
-            return b;
-        }
-        if (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.length > 1) {
-            if (a[0].hasOwnProperty('id')) {
-                // If the array contains entities, we can use the id to match them up
-                // so that we ensure that we don't merge properties from different entities
-                // with the same index.
-                const aIds = a.map(e => e.id);
-                const bIds = b.map(e => e.id);
-                if (JSON.stringify(aIds) !== JSON.stringify(bIds)) {
-                    // The entities in the arrays are not in the same order, so we can't
-                    // safely merge them. We need to sort the `b` array so that the entities
-                    // are in the same order as the `a` array.
-                    const idToIndexMap = new Map();
-                    a.forEach((item, index) => {
-                        idToIndexMap.set(item.id, index);
-                    });
-                    b.sort((_a, _b) => {
-                        return idToIndexMap.get(_a.id) - idToIndexMap.get(_b.id);
-                    });
-                }
-            }
-        }
-        for (const [key, value] of Object.entries(b)) {
-            if (Object.getOwnPropertyDescriptor(b, key)?.writable) {
-                if (Array.isArray(value)) {
-                    (a as any)[key] = value.map((v, index) =>
-                        this.mergeDeep(a?.[key]?.[index], b[key][index]),
-                    );
-                } else if (isObject(value)) {
-                    (a as any)[key] = this.mergeDeep(a?.[key], b[key]);
-                } else {
-                    (a as any)[key] = b[key];
-                }
-            }
-        }
-        return a ?? b;
-    }
 }

packages/core/src/service/helpers/entity-hydrator/merge-deep.spec.ts

@@ -0,0 +1,45 @@
+import { describe, expect, it } from 'vitest';
+
+import { Order, Sale } from '../../../entity';
+
+import { mergeDeep } from './merge-deep';
+
+describe('mergeDeep()', () => {
+    // https://github.com/vendure-ecommerce/vendure/issues/2864
+    it('should sync the order of sub relations', () => {
+        const prefetched = new Order({
+            lines: [
+                {
+                    id: 'line1',
+                    sales: [new Sale({ id: 'sale-of-line-1' })],
+                },
+                {
+                    id: 'line2',
+                    sales: [new Sale({ id: 'sale-of-line-2' })],
+                },
+            ],
+        });
+
+        const hydrationFetched = new Order({
+            lines: [
+                {
+                    id: 'line2',
+                    productVariant: { id: 'variant-of-line-2' },
+                },
+                {
+                    id: 'line1',
+                    productVariant: { id: 'variant-of-line-1' },
+                },
+            ],
+        });
+
+        const merged = mergeDeep(prefetched, hydrationFetched);
+        const line1 = merged.lines.find(l => l.id === 'line1');
+        const line2 = merged.lines.find(l => l.id === 'line2');
+
+        expect(line1?.sales[0].id).toBe('sale-of-line-1');
+        expect(line1?.productVariant?.id).toBe('variant-of-line-1');
+        expect(line2?.sales[0].id).toBe('sale-of-line-2');
+        expect(line2?.productVariant?.id).toBe('variant-of-line-2');
+    });
+});

packages/core/src/service/helpers/entity-hydrator/merge-deep.ts

@@ -0,0 +1,44 @@
+import { isObject } from '@vendure/common/lib/shared-utils';
+
+/**
+ * Merges properties into a target entity. This is needed for the cases in which a
+ * property already exists on the target, but the hydrated version also contains that
+ * property with a different set of properties. This prevents the original target
+ * entity from having data overwritten.
+ */
+export function mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b: T): T {
+    if (!a) {
+        return b;
+    }
+    if (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.length > 1) {
+        if (a[0].hasOwnProperty('id')) {
+            // If the array contains entities, we can use the id to match them up
+            // so that we ensure that we don't merge properties from different entities
+            // with the same index.
+            const aIds = a.map(e => e.id);
+            const bIds = b.map(e => e.id);
+            if (JSON.stringify(aIds) !== JSON.stringify(bIds)) {
+                // The entities in the arrays are not in the same order, so we can't
+                // safely merge them. We need to sort the `b` array so that the entities
+                // are in the same order as the `a` array.
+                const idToIndexMap = new Map();
+                a.forEach((item, index) => {
+                    idToIndexMap.set(item.id, index);
+                });
+                b.sort((_a, _b) => {
+                    return idToIndexMap.get(_a.id) - idToIndexMap.get(_b.id);
+                });
+            }
+        }
+    }
+    for (const [key, value] of Object.entries(b)) {
+        if (Object.getOwnPropertyDescriptor(b, key)?.writable) {
+            if (Array.isArray(value) || isObject(value)) {
+                (a as any)[key] = mergeDeep(a?.[key], b[key]);
+            } else {
+                (a as any)[key] = b[key];
+            }
+        }
+    }
+    return a ?? b;
+}