use strict mode in stories and tests

microsoft · Jul 23, 2024 · 4183269 · 4183269
1 parent f798c7f
commit 4183269
Show file tree

Hide file tree

Showing 12 changed files with 95 additions and 96 deletions.
diff --git a/packages/examples/.storybook/main.ts b/packages/examples/.storybook/main.ts
@@ -8,7 +8,9 @@ const config: StorybookConfig = {
   ],
   framework: {
     name: "@storybook/react-webpack5",
-    options: {},
+    options: {
+      strictMode: true,
+    },
   },
   webpackFinal: (config) => {
     return {

diff --git a/packages/examples/package.json b/packages/examples/package.json
@@ -43,7 +43,7 @@
     "@storybook/react-webpack5": "^7.6.19",
     "@storybook/test": "^7.6.19",
     "@storybook/types": "^7.6.19",
-    "@testing-library/jest-dom": "^5.14.1",
+    "@testing-library/jest-dom": "^6.1.3",
     "@testing-library/react": "^15.0.0",
     "@types/jest": "^29.2.0",
     "@types/react": "^18.3.1",

diff --git a/packages/examples/src/Feedback/FeedbackContainer.stories.ts b/packages/examples/src/Feedback/FeedbackContainer.stories.ts
@@ -77,8 +77,10 @@ export const LikeFailure: Story = {
       graphql: { mock },
     } = getNovaEnvironmentForStory(context);
 
-    // wait for next tick for apollo client to update state
-    await new Promise((resolve) => setTimeout(resolve, 0));
+    await waitFor(async () => {
+      const operation = mock.getMostRecentOperation();
+      await expect(operation).toBeDefined();
+    });
     await mock.resolveMostRecentOperation((operation) =>
       MockPayloadGenerator.generate(operation, {
         Feedback: () => sampleFeedback,

diff --git a/packages/nova-react/src/commanding/nova-centralized-commanding-provider.test.tsx b/packages/nova-react/src/commanding/nova-centralized-commanding-provider.test.tsx
@@ -21,40 +21,41 @@ describe(useNovaCentralizedCommanding, () => {
     expect.assertions(1);
 
     const TestUndefinedContextComponent: React.FC = () => {
-      try {
-        useNovaCentralizedCommanding();
-      } catch (e) {
-        expect((e as Error).message).toMatch(
-          "Nova Centralized Commanding provider must be initialized prior to consumption!",
-        );
-      }
+      useNovaCentralizedCommanding();
       return null;
     };
 
-    render(<TestUndefinedContextComponent />);
+    expect(() => render(<TestUndefinedContextComponent />)).toThrow(
+      "Nova Centralized Commanding provider must be initialized prior to consumption!",
+    );
   });
 
   it("is able to access the commanding instance provided by the provider", () => {
-    expect.assertions(2);
+    expect.assertions(1);
 
     const commanding = {
       trigger: jest.fn(),
     } as unknown as NovaCentralizedCommanding;
 
     const TestPassedContextComponent: React.FC = () => {
       const facadeFromContext = useNovaCentralizedCommanding();
-      expect(facadeFromContext).toBe(commanding);
-      facadeFromContext.trigger({
-        entity: {
-          type: EntityType.teams_activity,
-          action: EntityAction.default,
-        },
-        command: {
-          stateTransition: EntityStateTransition.new,
-          visibilityState: EntityVisibilityState.show,
-        },
-      });
-      expect(commanding.trigger).toBeCalledTimes(1);
+      const didTrigger = React.useRef(false);
+      React.useEffect(() => {
+        if (didTrigger.current) {
+          return;
+        }
+        facadeFromContext.trigger({
+          entity: {
+            type: EntityType.teams_activity,
+            action: EntityAction.default,
+          },
+          command: {
+            stateTransition: EntityStateTransition.new,
+            visibilityState: EntityVisibilityState.show,
+          },
+        });
+        didTrigger.current = true;
+      }, []);
       return null;
     };
 
@@ -63,5 +64,7 @@ describe(useNovaCentralizedCommanding, () => {
         <TestPassedContextComponent />
       </NovaCentralizedCommandingProvider>,
     );
+
+    expect(commanding.trigger).toBeCalledTimes(1);
   });
 });
diff --git a/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx b/packages/nova-react/src/eventing/nova-eventing-provider.test.tsx
@@ -71,34 +71,26 @@ describe("useNovaEventing", () => {
     expect.assertions(1);
 
     const TestUndefinedContextComponent: React.FC = () => {
-      try {
-        useNovaEventing();
-      } catch (e) {
-        expect((e as Error).message).toMatch(
-          "Nova Eventing provider must be initialized prior to consumption of eventing!",
-        );
-      }
+      useNovaEventing();
       return null;
     };
 
-    render(<TestUndefinedContextComponent />);
+    expect(() => render(<TestUndefinedContextComponent />)).toThrow(
+      "Nova Eventing provider must be initialized prior to consumption of eventing!",
+    );
   });
 
   it("useNovaUnmountEventing throws without a provider", () => {
     expect.assertions(1);
 
     const TestUndefinedContextComponent: React.FC = () => {
-      try {
-        useNovaUnmountEventing();
-      } catch (e) {
-        expect((e as Error).message).toMatch(
-          "Nova Eventing provider must be initialized prior to consumption of unmountEventing!",
-        );
-      }
+      useNovaUnmountEventing();
       return null;
     };
 
-    render(<TestUndefinedContextComponent />);
+    expect(() => render(<TestUndefinedContextComponent />)).toThrow(
+      "Nova Eventing provider must be initialized prior to consumption of unmountEventing!",
+    );
   });
 
   test("Takes in children and eventing props, renders children, and updates children as expected.", () => {
@@ -113,7 +105,8 @@ describe("useNovaEventing", () => {
       initialChildren,
     );
 
-    expect(renderSpy).toHaveBeenCalledTimes(1);
+    // called twice on each render due to strict mode
+    expect(renderSpy).toHaveBeenCalledTimes(2);
 
     wrapper.rerender(
       <NovaEventingProvider
@@ -125,7 +118,8 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       updatedChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(2);
+    // called twice on each render due to strict mode
+    expect(renderSpy).toHaveBeenCalledTimes(4);
   });
 
   test("Takes in children and eventing props, creates a stable wrapped NovaReactEventing instance from eventing across re-renders when children do not change.", () => {
@@ -143,7 +137,8 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(1);
+    // called twice on each render due to strict mode
+    expect(renderSpy).toHaveBeenCalledTimes(2);
 
     wrapper.rerender(
       <NovaEventingProvider
@@ -155,7 +150,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(1);
+    expect(renderSpy).toHaveBeenCalledTimes(2);
 
     // 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.
@@ -172,7 +167,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(1);
+    expect(renderSpy).toHaveBeenCalledTimes(2);
 
     //Trigger a callback on the test child through eventing
     eventCallback();
@@ -195,7 +190,7 @@ describe("useNovaEventing", () => {
     expect(wrapper.queryAllByTestId("children")[0].innerHTML).toBe(
       initialChildren,
     );
-    expect(renderSpy).toHaveBeenCalledTimes(1);
+    expect(renderSpy).toHaveBeenCalledTimes(2);
 
     //Trigger a callback on the test child through eventing
     eventCallback();
@@ -230,17 +225,23 @@ describe("NovaReactEventing exposes 'generateEvent'", () => {
         event,
       };
     }
+    const didGenerate = React.useRef(false);
+    React.useEffect(() => {
+      if (didGenerate.current) {
+        return;
+      }
+      facadeFromContext.generateEvent(eventWrapper);
+      expect(eventing.bubble).toBeCalledWith({
+        event,
+        source: {
+          inputType: InputType.programmatic,
+          timeStamp: expectedTime,
+        },
+      });
+      expect(mapper).toBeCalledTimes(0);
+      didGenerate.current = true;
+    }, []);
 
-    facadeFromContext.generateEvent(eventWrapper);
-
-    expect(eventing.bubble).toBeCalledWith({
-      event,
-      source: {
-        inputType: InputType.programmatic,
-        timeStamp: expectedTime,
-      },
-    });
-    expect(mapper).toBeCalledTimes(0);
     return null;
   };
 

diff --git a/packages/nova-react/src/graphql/nova-graphql-provider.test.tsx b/packages/nova-react/src/graphql/nova-graphql-provider.test.tsx
@@ -12,36 +12,29 @@ describe(useNovaGraphQL, () => {
     expect.assertions(1);
 
     const TestUndefinedContextComponent: React.FC = () => {
-      try {
-        useNovaGraphQL();
-      } catch (e) {
-        expect((e as Error).message).toMatch(
-          "Nova GraphQL provider must be initialized prior to consumption!",
-        );
-      }
+      useNovaGraphQL();
       return null;
     };
 
-    render(<TestUndefinedContextComponent />);
+    expect(() => render(<TestUndefinedContextComponent />)).toThrow(
+      "Nova GraphQL provider must be initialized prior to consumption!",
+    );
   });
 
   it("is able to access the GraphQL instance provided by the provider", () => {
-    expect.assertions(3);
+    expect.assertions(1);
 
     const graphql = {
       useLazyLoadQuery: jest.fn(),
     } as unknown as NovaGraphQL;
 
     const TestPassedContextComponent: React.FC = () => {
       const graphqlFromContext = useNovaGraphQL();
-      expect(graphqlFromContext).toBe(graphql);
-      expect(graphqlFromContext.useLazyLoadQuery).toBeDefined();
       // TODO figure out if this is needed
       if (!graphqlFromContext.useLazyLoadQuery) {
         return null;
       }
       graphqlFromContext.useLazyLoadQuery("foo", {});
-      expect(graphql.useLazyLoadQuery).toBeCalledTimes(1);
       return null;
     };
 
@@ -50,5 +43,8 @@ describe(useNovaGraphQL, () => {
         <TestPassedContextComponent />
       </NovaGraphQLProvider>,
     );
+
+    // twice due to strict mode
+    expect(graphql.useLazyLoadQuery).toBeCalledTimes(2);
   });
 });
diff --git a/scripts/config/jest.config.js → scripts/config/jest.config.ts b/scripts/config/jest.config.js → scripts/config/jest.config.ts
@@ -1,4 +1,6 @@
-module.exports = {
+import path from "path";
+
+export default {
   preset: "ts-jest",
   rootDir: process.cwd(),
   roots: ["<rootDir>/src"],
@@ -7,4 +9,6 @@ module.exports = {
   transform: {
     "\\.(gql|graphql)$": "@graphql-tools/jest-transform",
   },
+  setupFiles: [path.join(__dirname, "jest.setup.ts")],
+  setupFilesAfterEnv: [path.join(__dirname, "jest.setupAfterEnv.ts")],
 };
diff --git a/scripts/config/jest.setup.ts b/scripts/config/jest.setup.ts
@@ -0,0 +1,6 @@
+import { configure } from "@testing-library/react";
+
+configure({
+  reactStrictMode: true,
+});
+
diff --git a/scripts/config/jest.setupAfterEnv.ts b/scripts/config/jest.setupAfterEnv.ts
@@ -0,0 +1,6 @@
+import { cleanup } from "@testing-library/react";
+
+afterEach(() => {
+  // For some reason needed with strict mode enabled to cleanup DOM after each test
+  cleanup();
+});
diff --git a/scripts/just.config.ts b/scripts/just.config.ts
@@ -67,7 +67,7 @@ export const build = () => {
 
 export const test = () => {
   return jestTask({
-    config: path.join(__dirname, "config", "jest.config.js"),
+    config: path.join(__dirname, "config", "jest.config.ts"),
     watch: argv().watch,
     _: argv()._,
   });

diff --git a/scripts/package.json b/scripts/package.json
@@ -9,6 +9,7 @@
   },
   "devDependencies": {
     "@graphql-tools/jest-transform": "^1.2.2",
+    "@testing-library/react": "^15.0.0",
     "@types/node": "^16.11.6",
     "@typescript-eslint/eslint-plugin": "^5.50.0",
     "@typescript-eslint/parser": "^5.50.0",

diff --git a/yarn.lock b/yarn.lock
@@ -2,7 +2,7 @@
 # yarn lockfile v1
 
 
-"@adobe/css-tools@^4.0.1", "@adobe/css-tools@^4.3.2":
+"@adobe/css-tools@^4.3.2":
   version "4.3.3"
   resolved "https://registry.yarnpkg.com/@adobe/css-tools/-/css-tools-4.3.3.tgz#90749bde8b89cd41764224f5aac29cd4138f75ff"
   integrity sha512-rE0Pygv0sEZ4vBWHlAgJLGDU7Pm8xoO6p3wsEceb7GYAjScrOHpEo8KK/eVkAcnSM+slAEtXjA2JpdjLp4fJQQ==
@@ -3488,21 +3488,6 @@
     lz-string "^1.5.0"
     pretty-format "^27.0.2"
 
-"@testing-library/jest-dom@^5.14.1":
-  version "5.16.5"
-  resolved "https://registry.yarnpkg.com/@testing-library/jest-dom/-/jest-dom-5.16.5.tgz#3912846af19a29b2dbf32a6ae9c31ef52580074e"
-  integrity sha512-N5ixQ2qKpi5OLYfwQmUb/5mSV9LneAcaUfp32pn4yCnpb8r/Yz0pXFPck21dIicKmi+ta5WRAknkZCfA8refMA==
-  dependencies:
-    "@adobe/css-tools" "^4.0.1"
-    "@babel/runtime" "^7.9.2"
-    "@types/testing-library__jest-dom" "^5.9.1"
-    aria-query "^5.0.0"
-    chalk "^3.0.0"
-    css.escape "^1.5.1"
-    dom-accessibility-api "^0.5.6"
-    lodash "^4.17.15"
-    redent "^3.0.0"
-
 "@testing-library/jest-dom@^6.1.3":
   version "6.4.5"
   resolved "https://registry.yarnpkg.com/@testing-library/jest-dom/-/jest-dom-6.4.5.tgz#badb40296477149136dabef32b572ddd3b56adf1"
@@ -3737,7 +3722,7 @@
   dependencies:
     "@types/istanbul-lib-report" "*"
 
-"@types/jest@*", "@types/jest@^29.2.0":
+"@types/jest@^29.2.0":
   version "29.5.0"
   resolved "https://registry.yarnpkg.com/@types/jest/-/jest-29.5.0.tgz#337b90bbcfe42158f39c2fb5619ad044bbb518ac"
   integrity sha512-3Emr5VOl/aoBwnWcH/EFQvlSAmjV+XtV9GGu5mwdYew5vhQh0IUZx/60x0TzHDu09Bi7HMx10t/namdJw5QIcg==
@@ -3894,13 +3879,6 @@
   resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-2.0.1.tgz#20f18294f797f2209b5f65c8e3b5c8e8261d127c"
   integrity sha512-Hl219/BT5fLAaz6NDkSuhzasy49dwQS/DSdu4MdggFB8zcXv7vflBI3xp7FEmkmdDkBUI2bPUNeMttp2knYdxw==
 
-"@types/testing-library__jest-dom@^5.9.1":
-  version "5.14.5"
-  resolved "https://registry.yarnpkg.com/@types/testing-library__jest-dom/-/testing-library__jest-dom-5.14.5.tgz#d113709c90b3c75fdb127ec338dad7d5f86c974f"
-  integrity sha512-SBwbxYoyPIvxHbeHxTZX2Pe/74F/tX2/D3mMvzabdeJ25bBojfW0TyB8BHrbq/9zaaKICJZjLP+8r6AeZMFCuQ==
-  dependencies:
-    "@types/jest" "*"
-
 "@types/tough-cookie@*":
   version "4.0.2"
   resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.2.tgz#6286b4c7228d58ab7866d19716f3696e03a09397"
@@ -5797,7 +5775,7 @@ doctrine@^3.0.0:
   dependencies:
     esutils "^2.0.2"
 
-dom-accessibility-api@^0.5.6, dom-accessibility-api@^0.5.9:
+dom-accessibility-api@^0.5.9:
   version "0.5.16"
   resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz#5a7429e6066eb3664d911e33fb0e45de8eb08453"
   integrity sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==