From 57e85aa617f64ebd9e3db14ff2f49f450322f46d Mon Sep 17 00:00:00 2001
From: Stanislaw Wilczynski <Stanislaw.Wilczynski@microsoft.com>
Date: Fri, 26 Jul 2024 18:04:28 +0200
Subject: [PATCH] make strict mode hack a little clearer

---
 .../eventing/nova-eventing-provider.test.tsx  | 22 +++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx b/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx
index 29341eb..4788e29 100644
--- a/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx
+++ b/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx
@@ -33,6 +33,13 @@ describe("useNovaEventing", () => {
   const initialChildren = "initial children";
   const updatedChildren = "updated children";
 
+  // With React strict mode enabled, in development mode, the component will render twice, check https://react.dev/learn/synchronizing-with-effects#how-to-handle-the-effect-firing-twice-in-development
+  // for more details. This will cause the renderSpy to be called twice on each render, but in prod mode this will only be called once.
+  // We add this wrapper to encapsulate the logic for artificially increasing the render count to account for strict mode.
+  const expectComponentToRenderTimes = (count: number) => {
+    expect(renderSpy).toHaveBeenCalledTimes(count * 2);
+  };
+
   beforeEach(() => {
     jest.clearAllMocks();
 
@@ -105,8 +112,7 @@ describe("useNovaEventing", () => {
       initialChildren,
     );
 
-    // called twice on each render due to strict mode
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    expectComponentToRenderTimes(1);
 
     wrapper.rerender(
       <NovaEventingProvider
@@ -118,8 +124,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       updatedChildren,
     );
-    // called twice on each render due to strict mode
-    expect(renderSpy).toHaveBeenCalledTimes(4);
+    expectComponentToRenderTimes(2);
   });
 
   test("Takes in children and eventing props, creates a stable wrapped NovaReactEventing instance from eventing across re-renders when children do not change.", () => {
@@ -137,8 +142,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    // called twice on each render due to strict mode
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    expectComponentToRenderTimes(1);
 
     wrapper.rerender(
       <NovaEventingProvider
@@ -150,7 +154,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    expectComponentToRenderTimes(1);
 
     // Update eventing instance to test useRef pathway. This will ensure the wrapped eventing instance
     // returned from useEventing is stable from one render to the next.
@@ -167,7 +171,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    expectComponentToRenderTimes(1);
 
     //Trigger a callback on the test child through eventing
     eventCallback();
@@ -190,7 +194,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    expectComponentToRenderTimes(1);
 
     //Trigger a callback on the test child through eventing
     eventCallback();