fix: address prop name collision regression

measuredco · Jan 29, 2025 · 3a69ad4 · 3a69ad4
1 parent b9ce5b7
commit 3a69ad4
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 7 deletions.
diff --git a/packages/core/lib/__tests__/use-resolved-fields.spec.tsx b/packages/core/lib/__tests__/use-resolved-fields.spec.tsx
@@ -28,7 +28,7 @@ const blankContext = {
     },
     components: {
       Flex: {
-        fields: {},
+        fields: { title: { type: "number" } },
       },
       Heading: {
         fields: { title: { type: "text" } },
@@ -37,6 +37,33 @@ const blankContext = {
   },
 };
 
+const blankContextWithData = {
+  ...blankContext,
+  state: {
+    ...blankContext.state,
+    data: {
+      ...blankContext.state.data,
+      content: [
+        ...blankContext.state.data.content,
+        {
+          type: "Heading",
+          props: {
+            id: "heading-1",
+            title: "Hello, world",
+          },
+        },
+        {
+          type: "Flex",
+          props: {
+            id: "flex-1",
+            title: 1,
+          },
+        },
+      ],
+    },
+  },
+};
+
 const contextWithRootResolver = (resolveFields: any) => {
   return {
     ...blankContext,
@@ -52,7 +79,7 @@ const contextWithRootResolver = (resolveFields: any) => {
 
 const contextWithItemResolver = (resolveFields: any) => {
   return {
-    ...blankContext,
+    ...blankContextWithData,
     state: {
       ...blankContext.state,
       data: {
@@ -159,6 +186,78 @@ describe("use-resolved-fields", () => {
     expect(result.current[1]).toBe(false);
   });
 
+  describe("when changing the selected item", () => {
+    const contextWithSelectedItem = {
+      ...blankContextWithData,
+      state: {
+        ...blankContextWithData.state,
+        ui: {
+          ...blankContextWithData.state.ui,
+          itemSelector: { zone: "default-zone", index: 0 },
+        },
+      },
+      selectedItem: blankContextWithData.state.data.content[0],
+    };
+
+    const contextWithSelectedItemAlt = {
+      ...blankContextWithData,
+      state: {
+        ...blankContextWithData.state,
+        ui: {
+          ...blankContextWithData.state.ui,
+          itemSelector: { zone: "default-zone", index: 1 },
+        },
+      },
+      selectedItem: blankContextWithData.state.data.content[1],
+    };
+
+    it("(failure case) returns the old fields, if NOT checking the id, with value check disabled", async () => {
+      useAppContextMock.mockReturnValue(contextWithSelectedItem);
+      useParentMock.mockReturnValue(null);
+
+      await act(() => {
+        params = renderHook(() =>
+          useResolvedFields({ _skipValueCheck: true, _skipIdCheck: true })
+        ); // we need to skip the value check, as this will cause a re-render and make test pass in any scenario
+      });
+
+      const { result, rerender } = params;
+
+      expect(result.current[0]).toEqual({ title: { type: "text" } });
+      expect(result.current[1]).toBe(false);
+
+      useAppContextMock.mockReturnValue(contextWithSelectedItemAlt);
+
+      await act(() => {
+        rerender();
+      });
+
+      expect(result.current[0]).toEqual({ title: { type: "text" } }); // We're asserting the failure. If behaving correctly, this would be `number` (see case below).
+    });
+
+    it("returns the default fields, if checking the id, with value check disabled", async () => {
+      useAppContextMock.mockReturnValue(contextWithSelectedItem);
+      useParentMock.mockReturnValue(null);
+
+      await act(() => {
+        params = renderHook(() => useResolvedFields({ _skipValueCheck: true })); // we need to skip the value check, as this will cause a re-render and make test pass in any scenario
+      });
+
+      const { result, rerender } = params;
+
+      expect(result.current[0]).toEqual({ title: { type: "text" } });
+      expect(result.current[1]).toBe(false);
+
+      useAppContextMock.mockReturnValue(contextWithSelectedItemAlt);
+
+      await act(() => {
+        rerender();
+      });
+
+      expect(result.current[0]).toEqual({ title: { type: "number" } }); // We're asserting the success.
+    });
+  });
+
   describe("with resolveFields on the root", () => {
     it("calls it immediately", async () => {
       const mockResolveFields = jest.fn().mockResolvedValue({

diff --git a/packages/core/lib/use-resolved-fields.ts b/packages/core/lib/use-resolved-fields.ts
@@ -15,7 +15,13 @@ const defaultPageFields: Record<string, Field> = {
 
 type ComponentOrRootData = Omit<ComponentData<any>, "type">;
 
-export const useResolvedFields = (): [FieldsType, boolean] => {
+export const useResolvedFields = ({
+  _skipValueCheck,
+  _skipIdCheck,
+}: {
+  _skipValueCheck?: boolean;
+  _skipIdCheck?: boolean;
+} = {}): [FieldsType, boolean] => {
   const { selectedItem, state, config } = useAppContext();
   const parent = useParent();
 
@@ -41,7 +47,10 @@ export const useResolvedFields = (): [FieldsType, boolean] => {
   const [lastSelectedData, setLastSelectedData] = useState<
     Partial<ComponentOrRootData>
   >({});
-  const [resolvedFields, setResolvedFields] = useState(defaultFields);
+  const [resolvedFields, setResolvedFields] = useState({
+    fields: defaultFields,
+    id: selectedItem?.props.id,
+  });
   const [fieldsLoading, setFieldsLoading] = useState(false);
   const lastFields = useRef<FieldsType>(defaultFields);
 
@@ -124,7 +133,10 @@ export const useResolvedFields = (): [FieldsType, boolean] => {
         setFieldsLoading(true);
 
         resolveFields(defaultFields).then((fields) => {
-          setResolvedFields(fields || {});
+          setResolvedFields({
+            fields: fields || {},
+            id: selectedItem?.props.id,
+          });
 
           lastFields.current = fields;
 
@@ -134,10 +146,11 @@ export const useResolvedFields = (): [FieldsType, boolean] => {
         return;
       }
     }
-    setResolvedFields(defaultFields);
+    setResolvedFields({ fields: defaultFields, id: selectedItem?.props.id });
   }, [
     defaultFields,
     state.ui.itemSelector,
+    selectedItem,
     hasResolver,
     parent,
     resolveFields,
@@ -154,6 +167,8 @@ export const useResolvedFields = (): [FieldsType, boolean] => {
   useOnValueChange(
     { data, parent, itemSelector: state.ui.itemSelector },
     () => {
+      if (_skipValueCheck) return;
+
       triggerResolver();
     },
     (a, b) => JSON.stringify(a) === JSON.stringify(b)
@@ -163,5 +178,9 @@ export const useResolvedFields = (): [FieldsType, boolean] => {
     triggerResolver();
   }, []);
 
-  return [resolvedFields, fieldsLoading];
+  if (resolvedFields.id !== selectedItem?.props.id && !_skipIdCheck) {
+    return [defaultFields, fieldsLoading];
+  }
+
+  return [resolvedFields.fields, fieldsLoading];
 };