fix(core): pass propertyKey to the of template of an array property

packages/firecms_core/src/types/properties.ts

@@ -562,6 +562,22 @@ export type PropertyBuilderProps<M extends Record<string, any> = any> =
          * Current value of this property
          */
         propertyValue?: any;
+        /**
+         * Path of this property within the entity, in dot notation, e.g.
+         * `my_map.my_field` or `my_array.0.my_field`.
+         * Useful to look up sibling values in `values` with `getValueInPath`.
+         *
+         * When a property builder is used as the `of` prop of an array property,
+         * it is resolved twice: once per existing element, with an index
+         * qualified key (`my_array.0`), and once as the template for the element
+         * shape, with the array's own key (`my_array`). In the latter case
+         * `propertyValue` is the whole array, since the template is not bound to
+         * any element.
+         *
+         * It may be undefined if the property is resolved outside the context of
+         * an entity.
+         */
+        propertyKey?: string;
         /**
          * Index of this property (only for arrays)
          */
@@ -592,6 +608,7 @@ export type PropertyBuilder<T extends CMSType = any, M extends Record<string, an
         values,
         previousValues,
         propertyValue,
+        propertyKey,
         index,
         path,
         entityId,

packages/firecms_core/src/util/resolutions.ts

@@ -305,7 +305,15 @@ export function resolveArrayProperty<T extends any[], M>({
                 ignoreMissingFields,
                 ...props
             });
+            // `of` describes the shape of any element of the array, not one
+            // specific element, so there is no index to qualify the key with.
+            // We pass the array's own key, mirroring what the `oneOf` branch
+            // below does for `oneOf.properties`. Without this, a property
+            // builder used as `of` (or nested inside it) would be invoked with
+            // `propertyKey: undefined`, while the per-index resolutions above
+            // receive `${propertyKey}.${index}`.
             const ofProperty = resolveProperty({
+                propertyKey,
                 propertyOrBuilder: of,
                 ignoreMissingFields,
                 ...props

packages/firecms_core/test/resolutions.test.ts

@@ -1,5 +1,6 @@
 import { describe, test } from "@jest/globals";
 import { buildCollection, buildProperty, resolveCollection } from "../src/util";
+import { PropertyBuilderProps } from "../src/types";
 import * as util from "util";
 
 const testCollection = buildCollection({
@@ -162,3 +163,311 @@ describe("resolutions", () => {
         expect(resolved.defaultSize).toBe("s");
     });
 });
+
+/**
+ * Regression tests for https://github.com/firecmsco/firecms/issues/659
+ *
+ * `propertyKey` must be handed to every property builder that gets resolved,
+ * for every array resolution path (`of` as a tuple, `of` as a single property
+ * or builder, and `oneOf`). Builders rely on it to locate sibling values with
+ * dot notation (e.g. `example.0.parent` from `example.0.child`).
+ */
+describe("resolutions - propertyKey passed to property builders", () => {
+
+    /**
+     * Records the props every builder invocation receives, so we can assert on
+     * the exact `propertyKey`/`propertyValue` pairs and their order.
+     */
+    function recordingBuilder(calls: { propertyKey?: string, propertyValue?: any, index?: number }[],
+                              result: any = {
+                                  name: "Child",
+                                  dataType: "string"
+                              }) {
+        // Destructuring `propertyKey` out of the typed `PropertyBuilderProps`
+        // also asserts, at compile time, that it is part of the public contract.
+        return buildProperty(({
+                                 propertyKey,
+                                 propertyValue,
+                                 index
+                             }: PropertyBuilderProps) => {
+            calls.push({
+                propertyKey,
+                propertyValue,
+                index
+            });
+            return result;
+        });
+    }
+
+    function resolve(properties: any, values: any) {
+        return resolveCollection({
+            collection: buildCollection({
+                id: "test_659",
+                path: "test_659",
+                name: "Test 659",
+                properties
+            }),
+            path: "test_659",
+            values,
+            authController: {} as any
+        });
+    }
+
+    test("a builder nested in the map of an array `of` always receives a defined propertyKey", () => {
+
+        // This is the exact shape reported in #659: a builder inside a map,
+        // inside an array, that needs to find its sibling `parent` value.
+        const calls: { propertyKey?: string, propertyValue?: any }[] = [];
+
+        const resolved = resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                of: {
+                    name: "Entry",
+                    dataType: "map",
+                    properties: {
+                        parent: {
+                            name: "Parent",
+                            dataType: "string"
+                        },
+                        child: recordingBuilder(calls)
+                    }
+                }
+            }
+        }, {
+            example: [
+                { parent: "first" },
+                { parent: "second" }
+            ]
+        });
+
+        // The bug: the `of` template resolution used to invoke the builder with
+        // `propertyKey: undefined`.
+        expect(calls.map(c => c.propertyKey)).not.toContain(undefined);
+
+        // Two per-index resolutions (one per array element), then the single
+        // `of` template resolution.
+        expect(calls.map(c => c.propertyKey)).toEqual([
+            "example.0.child",
+            "example.1.child",
+            "example.child"
+        ]);
+
+        // A builder can therefore always derive its siblings' paths.
+        const siblingPaths = calls.map(c => [...c.propertyKey!.split(".").slice(0, -1), "parent"].join("."));
+        expect(siblingPaths).toEqual(["example.0.parent", "example.1.parent", "example.parent"]);
+
+        // The resolved collection still exposes both the `of` template and the
+        // per-index resolved properties.
+        const exampleProperty = resolved.properties.example as any;
+        expect(exampleProperty.of.properties.child.dataType).toEqual("string");
+        expect(exampleProperty.resolvedProperties).toHaveLength(2);
+        expect(exampleProperty.resolvedProperties[0].properties.child.dataType).toEqual("string");
+    });
+
+    test("a builder used directly as an array `of` receives a defined propertyKey", () => {
+
+        const calls: { propertyKey?: string, propertyValue?: any, index?: number }[] = [];
+
+        resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                of: recordingBuilder(calls)
+            }
+        }, {
+            example: ["first", "second"]
+        });
+
+        expect(calls.map(c => c.propertyKey)).not.toContain(undefined);
+        expect(calls).toEqual([
+            {
+                propertyKey: "example.0",
+                propertyValue: "first",
+                index: 0
+            },
+            {
+                propertyKey: "example.1",
+                propertyValue: "second",
+                index: 1
+            },
+            // The `of` template is not tied to any element, so it resolves
+            // against the array property's own key. `propertyValue` is
+            // consequently the whole array.
+            {
+                propertyKey: "example",
+                propertyValue: ["first", "second"],
+                index: undefined
+            }
+        ]);
+    });
+
+    test("the `of` template builder is still resolved when the array has no value", () => {
+
+        const calls: { propertyKey?: string, propertyValue?: any }[] = [];
+
+        resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                of: recordingBuilder(calls)
+            }
+        }, {});
+
+        // No elements, so only the `of` template resolution happens.
+        expect(calls).toEqual([
+            {
+                propertyKey: "example",
+                propertyValue: undefined,
+                index: undefined
+            }
+        ]);
+    });
+
+    test("a tuple `of` passes index qualified property keys", () => {
+
+        const calls: { propertyKey?: string, propertyValue?: any, index?: number }[] = [];
+
+        resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                of: [
+                    {
+                        name: "Plain",
+                        dataType: "string"
+                    },
+                    recordingBuilder(calls)
+                ]
+            }
+        }, {
+            example: ["plain value", "builder value"]
+        });
+
+        expect(calls).toEqual([
+            {
+                propertyKey: "example.1",
+                propertyValue: "builder value",
+                index: 1
+            }
+        ]);
+    });
+
+    test("`oneOf` passes index qualified property keys per element and the array key for the templates", () => {
+
+        const calls: { propertyKey?: string, propertyValue?: any }[] = [];
+
+        resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                oneOf: {
+                    properties: {
+                        alpha: recordingBuilder(calls),
+                        beta: {
+                            name: "Beta",
+                            dataType: "number"
+                        }
+                    }
+                }
+            }
+        }, {
+            example: [
+                {
+                    type: "alpha",
+                    value: "first"
+                },
+                {
+                    type: "beta",
+                    value: 2
+                }
+            ]
+        });
+
+        expect(calls.map(c => c.propertyKey)).toEqual([
+            // Per-element resolution of the `alpha` branch.
+            "example.0",
+            // Template resolution of `oneOf.properties`, based on the array key.
+            "example.alpha"
+        ]);
+    });
+
+    test("resolving non builder properties is unaffected", () => {
+
+        const resolved = resolve({
+            example: {
+                name: "Example",
+                dataType: "array",
+                of: {
+                    name: "Entry",
+                    dataType: "map",
+                    properties: {
+                        parent: {
+                            name: "Parent",
+                            dataType: "string"
+                        }
+                    }
+                }
+            }
+        }, {
+            example: [{ parent: "first" }]
+        });
+
+        const expectedOf = {
+            name: "Entry",
+            dataType: "map",
+            resolved: true,
+            fromBuilder: false,
+            properties: {
+                parent: {
+                    name: "Parent",
+                    dataType: "string",
+                    resolved: true,
+                    fromBuilder: false
+                }
+            }
+        };
+
+        expect(resolved.properties.example).toEqual({
+            name: "Example",
+            dataType: "array",
+            resolved: true,
+            fromBuilder: false,
+            of: expectedOf,
+            resolvedProperties: [expectedOf]
+        });
+    });
+
+    test("a builder resolved as a plain (non array) property still receives its propertyKey", () => {
+
+        const calls: { propertyKey?: string, propertyValue?: any }[] = [];
+
+        resolve({
+            plain: recordingBuilder(calls),
+            nested: {
+                name: "Nested",
+                dataType: "map",
+                properties: {
+                    child: recordingBuilder(calls)
+                }
+            }
+        }, {
+            plain: "plain value",
+            nested: { child: "nested value" }
+        });
+
+        expect(calls).toEqual([
+            {
+                propertyKey: "plain",
+                propertyValue: "plain value",
+                index: undefined
+            },
+            {
+                propertyKey: "nested.child",
+                propertyValue: "nested value",
+                index: undefined
+            }
+        ]);
+    });
+});