Reset Select and MultiSelect value when external value changes to undefined (#606)

* fix: actually run effects in useUpdateEffect
* test: decorate rerenders with ClickUIProvider
* test(SingleSelect): add cases with external value change
* test(MultiSelect): add cases with external value change
* test(useUpdateEffect): add tests

Author: ariser

src/components/Select/MultiSelect.test.tsx

@@ -64,7 +64,7 @@ describe("MultiSelect", () => {
     expect(queryByText("Content0")).not.toBeNull();
   });
 
-  it("should always respect given value in select", () => {
+  it("should prioritize external value over internal state", () => {
     const onSelect = vi.fn();
     const { queryByText, getByTestId, getByText } = renderSelect({
       value: ["content0", "content1"],
@@ -87,6 +87,51 @@ describe("MultiSelect", () => {
     expect(queryByTestingText(selectTrigger, "Content3")).toBeNull();
   });
 
+  it("should react to external value change", () => {
+    const { getByTestId, rerender } = renderCUI(
+      <MultiSelect
+        value={["content0", "content1"]}
+        options={selectOptions}
+      />
+    );
+    const selectTrigger = getByTestId("select-trigger");
+    expect(selectTrigger).not.toBeNull();
+    expect(queryByTestingText(selectTrigger, "Content0")).not.toBeNull();
+    expect(
+      queryByTestingText(selectTrigger, "Content1 long text content")
+    ).not.toBeNull();
+    rerender(
+      <MultiSelect
+        value={["content0", "content3"]}
+        options={selectOptions}
+      />
+    );
+    expect(queryByTestingText(selectTrigger, "Content0")).not.toBeNull();
+    expect(queryByTestingText(selectTrigger, "Content1 long text content")).toBeNull();
+    expect(queryByTestingText(selectTrigger, "Content3")).not.toBeNull();
+  });
+
+  it("shuold fall back to placeholder if value changes to undefined", () => {
+    const { getByTestId, rerender } = renderCUI(
+      <MultiSelect
+        value={["content0", "content1"]}
+        placeholder="noop"
+        options={selectOptions}
+      />
+    );
+    const selectTrigger = getByTestId("select-trigger");
+    expect(selectTrigger).not.toBeNull();
+    expect(queryByTestingText(selectTrigger, "noop")).toBeNull();
+    rerender(
+      <MultiSelect
+        value={undefined}
+        placeholder="noop"
+        options={selectOptions}
+      />
+    );
+    expect(queryByTestingText(selectTrigger, "noop")).not.toBeNull();
+  });
+
   it("should respect given defaultValue in select", () => {
     const { getByTestId } = renderSelect({
       defaultValue: ["content0", "content2"],

src/components/Select/SingleSelect.test.tsx

@@ -93,7 +93,7 @@ describe("Select", () => {
     expect(queryByText("Content0")).toBeNull();
   });
 
-  it("should always respect given value in select", () => {
+  it("should prioritize external value over internal state", () => {
     const onSelect = vi.fn();
     const { queryByText, getByTestId, getByText } = renderSelect({
       value: "content0",
@@ -115,6 +115,46 @@ describe("Select", () => {
     expect(queryByTestingText(selectTrigger, "Content0")).not.toBeNull();
   });
 
+  it("should react to external value change", () => {
+    const { getByTestId, rerender } = renderCUI(
+      <Select
+        value="content0"
+        options={selectOptions}
+      />
+    );
+    const selectTrigger = getByTestId("select-trigger");
+    expect(selectTrigger).not.toBeNull();
+    expect(queryByTestingText(selectTrigger, "Content0")).not.toBeNull();
+    rerender(
+      <Select
+        value="content3"
+        options={selectOptions}
+      />
+    );
+    expect(queryByTestingText(selectTrigger, "Content3")).not.toBeNull();
+  });
+
+  it("shuold fall back to placeholder if value changes to undefined", () => {
+    const { getByTestId, rerender } = renderCUI(
+      <Select
+        value="content0"
+        placeholder="noop"
+        options={selectOptions}
+      />
+    );
+    const selectTrigger = getByTestId("select-trigger");
+    expect(selectTrigger).not.toBeNull();
+    expect(queryByTestingText(selectTrigger, "noop")).toBeNull();
+    rerender(
+      <Select
+        value={undefined}
+        placeholder="noop"
+        options={selectOptions}
+      />
+    );
+    expect(queryByTestingText(selectTrigger, "noop")).not.toBeNull();
+  });
+
   it("should respect given defaultValue in select", () => {
     const { getByTestId } = renderSelect({
       defaultValue: "content0",

src/hooks/useUpdateEffect.test.ts

@@ -0,0 +1,52 @@
+import { renderHook } from "@testing-library/react";
+import { useUpdateEffect } from "./useUpdateEffect";
+
+describe("useUpdateEffect", () => {
+  it("should not run the effect on the first render", () => {
+    const effect = vi.fn();
+
+    renderHook(() => useUpdateEffect(effect));
+
+    // The effect should not have been called yet
+    expect(effect).not.toHaveBeenCalled();
+  });
+
+  it("should run the effect on subsequent renders", () => {
+    const effect = vi.fn();
+
+    const { rerender } = renderHook(() => {
+      useUpdateEffect(effect);
+    });
+
+    rerender();
+
+    // The effect should have been called once
+    expect(effect).toHaveBeenCalledTimes(1);
+  });
+
+  it("should run the effect when dependencies change", () => {
+    const effect = vi.fn();
+
+    const { rerender } = renderHook((dependency: string = "initial_dependency") =>
+      useUpdateEffect(effect, [dependency])
+    );
+
+    rerender("updated_dependency");
+
+    // The effect should have been called once
+    expect(effect).toHaveBeenCalledTimes(1);
+  });
+
+  it("should not run the effect if dependencies do not change", () => {
+    const effect = vi.fn();
+
+    const { rerender } = renderHook((dependency: string = "same_dependency") =>
+      useUpdateEffect(effect, [dependency])
+    );
+
+    rerender("same_dependency");
+
+    // The effect should not have been called again
+    expect(effect).toHaveBeenCalledTimes(0);
+  });
+});

src/hooks/useUpdateEffect.ts

@@ -4,7 +4,7 @@ export const useUpdateEffect: typeof useEffect = (effect, deps) => {
   const isFirstMount = useRef(true);
 
   useEffect(() => {
-    if (isFirstMount) {
+    if (isFirstMount.current) {
       isFirstMount.current = false;
       return;
     }

src/utils/test-utils.tsx

@@ -2,15 +2,30 @@ import { type ThemeName } from "@/theme";
 import { render as renderTL } from "@testing-library/react";
 import { ClickUIProvider } from "..";
 
+// eslint-disable-next-line react-refresh/only-export-components
+const Wrapper = ({
+  theme,
+  children,
+}: {
+  theme: ThemeName;
+  children: React.ReactNode;
+}) => (
+  <ClickUIProvider
+    theme={theme}
+    config={{ tooltip: { delayDuration: 0 } }}
+  >
+    {children}
+  </ClickUIProvider>
+);
+
 const renderCUI = (children: React.ReactNode, theme: ThemeName = "dark") => {
-  return renderTL(
-    <ClickUIProvider
-      theme={theme}
-      config={{ tooltip: { delayDuration: 0 } }}
-    >
-      {children}
-    </ClickUIProvider>
-  );
+  const rtlRenderResult = renderTL(<Wrapper theme={theme}>{children}</Wrapper>);
+
+  return {
+    ...rtlRenderResult,
+    rerender: (rerenderChildren: React.ReactNode) =>
+      rtlRenderResult.rerender(<Wrapper theme={theme}>{rerenderChildren}</Wrapper>),
+  };
 };
 
 export { renderCUI };