feat: bump react to version 18 and run all tests/stories in strict mode (#101)

* bump react

* Change files

* use strict mode in stories and tests

* bump node version

* make strict mode hack a little clearer

---------

Co-authored-by: Stanislaw Wilczynski <[email protected]>

Author: sjwilczynski

.github/workflows/pr.yml

@@ -10,7 +10,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [16.x]
+        node-version: [18.x]
 
     steps:
       - uses: actions/checkout@v2

change/@nova-examples-9c2d5916-30ef-4e59-88d2-9f5e53ecddd4.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "bump react",
+  "packageName": "@nova/examples",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@nova-react-f3fa87cf-0982-490f-85ea-23e3eda2fcdf.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "bump react",
+  "packageName": "@nova/react",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@nova-react-test-utils-aeed978b-31e7-4ce5-b299-b0bdb3209648.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "bump react",
+  "packageName": "@nova/react-test-utils",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/examples/.storybook/main.ts

@@ -8,7 +8,9 @@ const config: StorybookConfig = {
   ],
   framework: {
     name: "@storybook/react-webpack5",
-    options: {},
+    options: {
+      strictMode: true,
+    },
   },
   webpackFinal: (config) => {
     return {

packages/examples/package.json

@@ -20,8 +20,8 @@
     "@nova/react-test-utils": "4.3.1",
     "@nova/types": "1.4.0",
     "graphql": "^15.5.0",
-    "react": "^17.0.2",
-    "react-dom": "^17.0.2"
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1"
   },
   "devDependencies": {
     "@babel/core": "^7.20.2",
@@ -42,11 +42,12 @@
     "@storybook/react": "^7.6.19",
     "@storybook/react-webpack5": "^7.6.19",
     "@storybook/test": "^7.6.19",
-    "@testing-library/jest-dom": "^5.14.1",
-    "@testing-library/react": "^12.0.0",
+    "@storybook/types": "^7.6.19",
+    "@testing-library/jest-dom": "^6.1.3",
+    "@testing-library/react": "^15.0.0",
     "@types/jest": "^29.2.0",
-    "@types/react": "^17.0.2",
-    "@types/react-dom": "^17.0.11",
+    "@types/react": "^18.3.1",
+    "@types/react-dom": "^18.3.0",
     "esbuild-loader": "^3.0.1",
     "monorepo-scripts": "*",
     "prop-types": "15.8.1",

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,

packages/examples/src/Feedback/FeedbackContainer.test.tsx

@@ -1,9 +1,10 @@
 import { composeStories } from "@storybook/react";
 import * as stories from "./FeedbackContainer.stories";
-import { act, render, screen } from "@testing-library/react";
+import { render, screen, waitFor } from "@testing-library/react";
 import * as React from "react";
 import "@testing-library/jest-dom";
 import { prepareStoryContextForTest } from "@nova/react-test-utils";
+import { executePlayFunction } from "../testing-utils/executePlayFunction";
 
 const { AutoGeneratedDataOnly, Primary, Liked, Like, LikeFailure, Loading } =
   composeStories(stories);
@@ -29,17 +30,16 @@ describe("FeedbackContainer", () => {
 
   it("should show unlike button after clicking like button", async () => {
     const { container } = render(<Like />);
-    await act(async () => Like.play({ canvasElement: container }));
+    await executePlayFunction(Like, { canvasElement: container });
     const button = await screen.findByRole("button", { name: "Unlike" });
     expect(button).toBeInTheDocument();
   });
 
   it("should show an error if the like button fails", async () => {
     const { container } = render(<LikeFailure />);
-    // This needs to be wrapped in act as play function for this story
-    // relies on mock client to resolve queries which updates component state
-    await act(async () =>
-      LikeFailure.play(prepareStoryContextForTest(LikeFailure, container)),
+    await executePlayFunction(
+      LikeFailure,
+      prepareStoryContextForTest(LikeFailure, container),
     );
     const error = await screen.findByText("Something went wrong");
     expect(error).toBeInTheDocument();
@@ -55,8 +55,14 @@ describe("FeedbackContainer", () => {
     render(<Primary />);
     render(<Liked />);
     render(<Primary />);
-    const texts = await screen.findAllByText("Feedback: Feedback title");
-    expect(texts).toHaveLength(3);
-    expect(texts[2]).toBeInTheDocument();
+    const text = "Feedback: Feedback title";
+    await waitFor(
+      () => {
+        const texts = screen.getAllByText(text);
+        expect(texts).toHaveLength(3);
+      },
+      { timeout: 3000 },
+    );
+    expect(screen.getAllByText(text)[2]).toBeInTheDocument();
   });
 });

packages/examples/src/testing-utils/executePlayFunction.ts

@@ -0,0 +1,39 @@
+import type { ReactRenderer } from "@storybook/react";
+import type {
+  ComposedStoryPlayContext,
+  ComposedStoryFn,
+} from "@storybook/types";
+import { act, waitFor } from "@testing-library/react";
+
+type PlayFunctionThatReturnsPromise = (
+  options: ComposedStoryPlayContext<ReactRenderer>,
+) => Promise<void>;
+
+// This function is a workaround for the issues with React 18 that if someone
+// tries to call `play` function from act like:
+// await act(async () => {
+//   await Story.play({ canvasElement: container });
+// });
+// it will fail by never rerendering the component causing any assertions in play
+// function to fail. To mitigate that we mimic awaiting by adding a flag that will
+// be set to true once the play function is completed and we wait for that flag to
+// be true.
+export const executePlayFunction = async (
+  Story: ComposedStoryFn<ReactRenderer>,
+  options: ComposedStoryPlayContext<ReactRenderer>,
+) => {
+  let playFunctionCompleted = false;
+  const playFunction = Story.play as PlayFunctionThatReturnsPromise;
+  act(() => {
+    playFunction(options)
+      .then(() => {
+        playFunctionCompleted = true;
+      })
+      .catch((error) => {
+        throw error;
+      });
+  });
+  await waitFor(() => {
+    expect(playFunctionCompleted).toBe(true);
+  });
+};

packages/nova-react-test-utils/package.json

@@ -12,7 +12,7 @@
   },
   "peerDependencies": {
     "@storybook/react": "^7.6",
-    "react": "^17.0.2"
+    "react": "^17.0.2 || ^18"
   },
   "dependencies": {
     "@apollo/client": "^3.4.15",
@@ -34,13 +34,13 @@
     "@graphitation/apollo-react-relay-duct-tape-compiler": "^1.4.0",
     "@storybook/react": "^7.6.19",
     "@storybook/types": "^7.6.19",
-    "@testing-library/react": "^12.0.0",
+    "@testing-library/react": "^15.0.0",
     "@types/jest": "^29.2.0",
-    "@types/react": "^17.0.2",
-    "@types/react-test-renderer": "^17.0.1",
+    "@types/react": "^18.3.1",
+    "@types/react-test-renderer": "^18.3.0",
     "monorepo-scripts": "*",
-    "react": "^17.0.2",
-    "react-test-renderer": "^17.0.2"
+    "react": "^18.3.1",
+    "react-test-renderer": "^18.3.1"
   },
   "repository": {
     "type": "git",

packages/nova-react-test-utils/src/nova-mock-environment.tsx

@@ -1,5 +1,5 @@
 import React from "react";
-import type { ComponentType } from "react";
+import type { ComponentType, PropsWithChildren } from "react";
 import type {
   NovaCentralizedCommanding,
   NovaGraphQL,
@@ -33,7 +33,7 @@ export interface NovaMockEnvironment<T extends Environment = "test"> {
    * A React component that will be used to wrap the NovaFacadeProvider children. This is used by the test-utils to
    * inject a ApolloProvider.
    */
-  providerWrapper: ComponentType;
+  providerWrapper: ComponentType<PropsWithChildren>;
 }
 
 interface NovaMockEnvironmentProviderProps<T extends Environment> {

packages/nova-react-test-utils/src/storybook-nova-decorator.tsx

@@ -109,7 +109,7 @@ export const getNovaEnvironmentDecorator: (
       context.parameters[NAME_OF_ASSIGNED_PARAMETER_IN_DECORATOR] = environment;
       return (
         <NovaMockEnvironmentProvider environment={environment}>
-          {getStory(context)}
+          {getStory(context) as React.ReactNode}
         </NovaMockEnvironmentProvider>
       );
     },

packages/nova-react/package.json

@@ -22,15 +22,14 @@
   },
   "devDependencies": {
     "@graphitation/graphql-js-tag": "^0.9.0",
-    "@testing-library/react": "^12.0.0",
-    "@testing-library/react-hooks": "^8.0.1",
+    "@testing-library/react": "^15.0.0",
     "@types/invariant": "^2.2.35",
     "@types/jest": "^29.2.0",
-    "@types/react": "^17.0.2",
-    "@types/react-dom": "^17.0.11",
+    "@types/react": "^18.3.1",
+    "@types/react-dom": "^18.3.0",
     "monorepo-scripts": "*",
-    "react": "^17.0.2",
-    "react-dom": "^17.0.2"
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1"
   },
   "repository": {
     "type": "git",

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);
   });
 });