From 40dff5ee0f71e4c3868094089eab8810ca1ec686 Mon Sep 17 00:00:00 2001
From: Rajat Agrawal <rajat@appsmith.com>
Date: Thu, 3 Aug 2023 11:28:37 +0530
Subject: [PATCH] fix: Skip echarts rerendering if props are same (#25880)

Fixes #25783
---
 .../component/ChartErrorComponent.tsx         | 15 ++----
 .../widgets/ChartWidget/component/index.tsx   | 49 ++++++++-----------
 2 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/app/client/src/widgets/ChartWidget/component/ChartErrorComponent.tsx b/app/client/src/widgets/ChartWidget/component/ChartErrorComponent.tsx
index d8d19db781..64352d25c1 100644
--- a/app/client/src/widgets/ChartWidget/component/ChartErrorComponent.tsx
+++ b/app/client/src/widgets/ChartWidget/component/ChartErrorComponent.tsx
@@ -30,12 +30,8 @@ const ErrorBox = styled.div`
   align-items: center;
   flex-flow: column;
   gap: 8px;
-}`;
-
-const ErrorStack = styled.div`
   overflow-y: scroll;
-  text-align: center;
-`;
+}`;
 
 const MoreDetailsButton = styled(Button)`
   flex-shrink: 2;
@@ -44,7 +40,6 @@ const MoreDetailsButton = styled(Button)`
 `;
 
 const Title = styled(Text)`
-  overflow: scroll;
   font-weight: var(--ads-font-weight-bold-xl);
 `;
 
@@ -93,11 +88,9 @@ export function ChartErrorComponent(props: ChartErrorProps) {
           text={messages.MoreDetails}
           width="200px"
         />
-        <ErrorStack>
-          <Collapse isOpen={!bodyCollapsed}>
-            <Text type={TextType.P1}>{errorMessage().body}</Text>
-          </Collapse>
-        </ErrorStack>
+        <Collapse isOpen={!bodyCollapsed}>
+          <Text type={TextType.P1}>{errorMessage().body}</Text>
+        </Collapse>
       </ErrorBox>
     </ChartErrorContainer>
   );
diff --git a/app/client/src/widgets/ChartWidget/component/index.tsx b/app/client/src/widgets/ChartWidget/component/index.tsx
index c79bcf742d..3b98b9599f 100644
--- a/app/client/src/widgets/ChartWidget/component/index.tsx
+++ b/app/client/src/widgets/ChartWidget/component/index.tsx
@@ -177,19 +177,12 @@ class ChartComponent extends React.Component<
     }
   };
 
-  resizeEchartsIfNeeded = () => {
-    if (this.echartsInstance) {
-      if (
-        this.echartsInstance.getHeight() !=
-          this.props.dimensions.componentHeight ||
-        this.echartsInstance.getWidth() != this.props.dimensions.componentWidth
-      ) {
-        this.echartsInstance.resize({
-          width: this.props.dimensions.componentWidth,
-          height: this.props.dimensions.componentHeight,
-        });
-      }
-    }
+  shouldResizeECharts = () => {
+    return (
+      this.echartsInstance?.getHeight() !=
+        this.props.dimensions.componentHeight ||
+      this.echartsInstance?.getWidth() != this.props.dimensions.componentWidth
+    );
   };
 
   renderECharts = () => {
@@ -200,33 +193,31 @@ class ChartComponent extends React.Component<
     }
 
     const newConfiguration = this.getEChartsOptions();
-    let needsNewConfig = true;
+    const needsNewConfig = !equal(newConfiguration, this.echartConfiguration);
+    const resizedNeeded = this.shouldResizeECharts();
 
-    if (
-      this.state.eChartsError &&
-      equal(newConfiguration, this.echartConfiguration)
-    ) {
-      // this check is required if chartError is present and the code shouldn't calculate the same error again
-      needsNewConfig = false;
-    } else {
+    if (needsNewConfig) {
       this.echartConfiguration = newConfiguration;
-    }
-
-    try {
       this.echartsInstance.off("click");
       this.echartsInstance.on("click", this.dataClickCallback);
 
-      if (needsNewConfig) {
+      try {
         this.echartsInstance.setOption(this.echartConfiguration, true);
+
         if (this.state.eChartsError) {
           this.setState({ eChartsError: undefined });
         }
+      } catch (error) {
+        this.disposeECharts();
+        this.setState({ eChartsError: error as Error });
       }
+    }
 
-      this.resizeEchartsIfNeeded();
-    } catch (error) {
-      this.disposeECharts();
-      this.setState({ eChartsError: error as Error });
+    if (resizedNeeded) {
+      this.echartsInstance.resize({
+        width: this.props.dimensions.componentWidth,
+        height: this.props.dimensions.componentHeight,
+      });
     }
   };