From 61b6474900b3d5c722758b652f94fe6e57e5f4a1 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Mon, 3 Feb 2025 18:39:35 +0530 Subject: [PATCH 1/7] 1014-memoize-conditional-branch-widgets --- .changeset/funny-peas-compare.md | 5 + packages/runtime/src/widgets/Conditional.tsx | 21 ++-- .../widgets/__tests__/Conditional.test.tsx | 107 +++++++++++++++++- 3 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 .changeset/funny-peas-compare.md diff --git a/.changeset/funny-peas-compare.md b/.changeset/funny-peas-compare.md new file mode 100644 index 000000000..8075a2b54 --- /dev/null +++ b/.changeset/funny-peas-compare.md @@ -0,0 +1,5 @@ +--- +"@ensembleui/react-runtime": patch +--- + +Fix: memoize conditional branch widgets diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index c98416351..6760514b1 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -1,7 +1,7 @@ -import type { Expression } from "@ensembleui/react-framework"; +import type { EnsembleWidget, Expression } from "@ensembleui/react-framework"; import { unwrapWidget, useRegisterBindings } from "@ensembleui/react-framework"; import { cloneDeep, head, isEmpty, last } from "lodash-es"; -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { WidgetRegistry } from "../registry"; import { EnsembleRuntime } from "../runtime"; import type { EnsembleWidgetProps } from "../shared/types"; @@ -26,6 +26,7 @@ export const Conditional: React.FC = ({ conditions, ...props }) => { + const [matched, setMatched] = useState<{ [key: string]: unknown }>({}); const [isValid, errorMessage] = hasProperStructure(conditions); if (!isValid) throw Error(errorMessage); @@ -55,11 +56,17 @@ export const Conditional: React.FC = ({ if (trueIndex === undefined || trueIndex < 0) { return null; } - const extractedWidget = extractWidget(conditions[trueIndex]); - return { - ...extractedWidget, - key: conditionStatements[trueIndex]?.toString(), - }; + const key = conditionStatements[trueIndex]?.toString(); + + const extractedWidget = + key && matched[key] + ? (matched[key] as EnsembleWidget) + : extractWidget(conditions[trueIndex]); + + if (key && !matched[key]) { + setMatched((prev) => ({ ...prev, [key]: extractedWidget })); + } + return { ...extractedWidget, key }; }, [conditionStatements, conditions, trueIndex]); if (!widget) { diff --git a/packages/runtime/src/widgets/__tests__/Conditional.test.tsx b/packages/runtime/src/widgets/__tests__/Conditional.test.tsx index a28df0f80..4a8623650 100644 --- a/packages/runtime/src/widgets/__tests__/Conditional.test.tsx +++ b/packages/runtime/src/widgets/__tests__/Conditional.test.tsx @@ -1,4 +1,10 @@ -import { render, screen } from "@testing-library/react"; +/* eslint import/first: 0 */ +// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment +const framework = jest.requireActual("@ensembleui/react-framework"); +// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access +const unwrapWidgetSpy = jest.fn().mockImplementation(framework.unwrapWidget); +import { fireEvent, render, screen } from "@testing-library/react"; +import { BrowserRouter } from "react-router-dom"; import type { ConditionalProps, ConditionalElement } from "../Conditional"; import { Conditional, @@ -7,9 +13,20 @@ import { extractCondition, } from "../Conditional"; import "../index"; +import { EnsembleScreen } from "../../runtime/screen"; jest.mock("react-markdown", jest.fn()); +// eslint-disable-next-line @typescript-eslint/no-unsafe-return +jest.mock("@ensembleui/react-framework", () => ({ + ...framework, + unwrapWidget: unwrapWidgetSpy, +})); + +afterEach(() => { + jest.clearAllMocks(); +}); + describe("Conditional Component", () => { test('renders the widget when "if" condition is met', () => { const conditionalProps: ConditionalProps = { @@ -233,3 +250,91 @@ describe("extractCondition Function", () => { expect(extractedCondition).toBe("1 === 1"); }); }); + +describe("conditional widget memoization", () => { + it("should memoize branch widgets and prevent unnecessary re-renders", () => { + render( + 0}`, + Text: { + text: "Greater than 0", + }, + }, + ], + }, + }, + { + name: "Button", + properties: { + label: "Increase", + onTap: { + executeCode: + "ensemble.storage.set('number', ensemble.storage.get('number') + 1)", + }, + }, + }, + { + name: "Button", + properties: { + label: "Decrease", + onTap: { + executeCode: + "ensemble.storage.set('number', ensemble.storage.get('number') - 1)", + }, + }, + }, + ], + }, + }, + onLoad: { executeCode: 'ensemble.storage.set("number", -1)' }, + }} + />, + { + wrapper: BrowserRouter, + }, + ); + + expect(unwrapWidgetSpy).toHaveBeenCalledTimes(1); + expect(screen.getByText("Less than 0")).not.toBeNull(); + + fireEvent.click(screen.getByText("Increase")); + expect(unwrapWidgetSpy).toHaveBeenCalledTimes(2); + expect(screen.getByText("Equals to 0")).not.toBeNull(); + + fireEvent.click(screen.getByText("Increase")); + expect(unwrapWidgetSpy).toHaveBeenCalledTimes(3); + expect(screen.getByText("Greater than 0")).not.toBeNull(); + + fireEvent.click(screen.getByText("Decrease")); + expect(unwrapWidgetSpy).toHaveBeenCalledTimes(3); + expect(screen.getByText("Equals to 0")).not.toBeNull(); + + fireEvent.click(screen.getByText("Decrease")); + expect(unwrapWidgetSpy).toHaveBeenCalledTimes(3); + expect(screen.getByText("Less than 0")).not.toBeNull(); + }); +}); From 279128dd13eec21d6ce752f81605d617cb7310b4 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Tue, 4 Feb 2025 18:17:15 +0530 Subject: [PATCH 2/7] fix: memoize the react node of conditional widget --- packages/runtime/src/widgets/Conditional.tsx | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 6760514b1..96b1f7e3b 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -1,6 +1,7 @@ -import type { EnsembleWidget, Expression } from "@ensembleui/react-framework"; +import type { Expression } from "@ensembleui/react-framework"; import { unwrapWidget, useRegisterBindings } from "@ensembleui/react-framework"; import { cloneDeep, head, isEmpty, last } from "lodash-es"; +import type { ReactNode } from "react"; import { useMemo, useState } from "react"; import { WidgetRegistry } from "../registry"; import { EnsembleRuntime } from "../runtime"; @@ -58,22 +59,25 @@ export const Conditional: React.FC = ({ } const key = conditionStatements[trueIndex]?.toString(); - const extractedWidget = - key && matched[key] - ? (matched[key] as EnsembleWidget) - : extractWidget(conditions[trueIndex]); + if (key && matched[key]) { + return matched[key] as ReactNode[]; + } + + const extractedWidget = extractWidget(conditions[trueIndex]); + + const renderWidget = EnsembleRuntime.render([{ ...extractedWidget, key }]); if (key && !matched[key]) { - setMatched((prev) => ({ ...prev, [key]: extractedWidget })); + setMatched((prev) => ({ ...prev, [key]: renderWidget })); } - return { ...extractedWidget, key }; + return renderWidget; }, [conditionStatements, conditions, trueIndex]); if (!widget) { return null; } - return <>{EnsembleRuntime.render([widget])}; + return <>{widget}; }; WidgetRegistry.register(widgetName, Conditional); From cf64c88f2dfccfe808377bdff17120910dee7fd2 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Wed, 5 Feb 2025 19:53:12 +0530 Subject: [PATCH 3/7] fix: update the types --- packages/runtime/src/widgets/Conditional.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 96b1f7e3b..99f14e482 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -27,7 +27,7 @@ export const Conditional: React.FC = ({ conditions, ...props }) => { - const [matched, setMatched] = useState<{ [key: string]: unknown }>({}); + const [matched, setMatched] = useState<{ [key: string]: ReactNode[] }>({}); const [isValid, errorMessage] = hasProperStructure(conditions); if (!isValid) throw Error(errorMessage); @@ -60,7 +60,7 @@ export const Conditional: React.FC = ({ const key = conditionStatements[trueIndex]?.toString(); if (key && matched[key]) { - return matched[key] as ReactNode[]; + return matched[key]; } const extractedWidget = extractWidget(conditions[trueIndex]); @@ -129,3 +129,4 @@ export const extractCondition = (condition: ConditionalElement) => { throw Error("Improper structure, make sure every condition has a condition"); }; + From c348a0172640bd0c077f185c8193c4813219129c Mon Sep 17 00:00:00 2001 From: sagar davara Date: Wed, 5 Feb 2025 19:53:12 +0530 Subject: [PATCH 4/7] fix: update the types --- packages/runtime/src/widgets/Conditional.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 96b1f7e3b..5dc081243 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -27,7 +27,7 @@ export const Conditional: React.FC = ({ conditions, ...props }) => { - const [matched, setMatched] = useState<{ [key: string]: unknown }>({}); + const [matched, setMatched] = useState<{ [key: string]: ReactNode[] }>({}); const [isValid, errorMessage] = hasProperStructure(conditions); if (!isValid) throw Error(errorMessage); @@ -60,7 +60,7 @@ export const Conditional: React.FC = ({ const key = conditionStatements[trueIndex]?.toString(); if (key && matched[key]) { - return matched[key] as ReactNode[]; + return matched[key]; } const extractedWidget = extractWidget(conditions[trueIndex]); From 77fca62f723274d5e2dce71cd5fe1581218c2817 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Wed, 5 Feb 2025 20:20:25 +0530 Subject: [PATCH 5/7] fix: lint issue --- packages/runtime/src/widgets/Conditional.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 99f14e482..5dc081243 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -129,4 +129,3 @@ export const extractCondition = (condition: ConditionalElement) => { throw Error("Improper structure, make sure every condition has a condition"); }; - From 49520c68b5bdbf7f6dffdf39ec1c7d59f2f4d8f1 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Wed, 5 Feb 2025 20:28:44 +0530 Subject: [PATCH 6/7] fix: added ref --- packages/runtime/src/widgets/Conditional.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 5dc081243..1c9e297e3 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -2,7 +2,7 @@ import type { Expression } from "@ensembleui/react-framework"; import { unwrapWidget, useRegisterBindings } from "@ensembleui/react-framework"; import { cloneDeep, head, isEmpty, last } from "lodash-es"; import type { ReactNode } from "react"; -import { useMemo, useState } from "react"; +import { useMemo, useRef } from "react"; import { WidgetRegistry } from "../registry"; import { EnsembleRuntime } from "../runtime"; import type { EnsembleWidgetProps } from "../shared/types"; @@ -27,7 +27,7 @@ export const Conditional: React.FC = ({ conditions, ...props }) => { - const [matched, setMatched] = useState<{ [key: string]: ReactNode[] }>({}); + const matched = useRef<{ [key: string]: ReactNode[] }>({}); const [isValid, errorMessage] = hasProperStructure(conditions); if (!isValid) throw Error(errorMessage); @@ -59,16 +59,16 @@ export const Conditional: React.FC = ({ } const key = conditionStatements[trueIndex]?.toString(); - if (key && matched[key]) { - return matched[key]; + if (key && matched.current[key]) { + return matched.current[key]; } const extractedWidget = extractWidget(conditions[trueIndex]); const renderWidget = EnsembleRuntime.render([{ ...extractedWidget, key }]); - if (key && !matched[key]) { - setMatched((prev) => ({ ...prev, [key]: renderWidget })); + if (key && !matched.current[key]) { + matched.current = { ...matched.current, [key]: renderWidget }; } return renderWidget; }, [conditionStatements, conditions, trueIndex]); From 078c704bdd66407b634b7afeaddd7c146b077fd1 Mon Sep 17 00:00:00 2001 From: sagar davara Date: Thu, 6 Feb 2025 11:51:13 +0530 Subject: [PATCH 7/7] fix: update values through key in object --- packages/runtime/src/widgets/Conditional.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime/src/widgets/Conditional.tsx b/packages/runtime/src/widgets/Conditional.tsx index 1c9e297e3..03c57c475 100644 --- a/packages/runtime/src/widgets/Conditional.tsx +++ b/packages/runtime/src/widgets/Conditional.tsx @@ -68,7 +68,7 @@ export const Conditional: React.FC = ({ const renderWidget = EnsembleRuntime.render([{ ...extractedWidget, key }]); if (key && !matched.current[key]) { - matched.current = { ...matched.current, [key]: renderWidget }; + matched.current[key] = renderWidget; } return renderWidget; }, [conditionStatements, conditions, trueIndex]);