Fix bug with search params removal (#9969)

remix-run · Jan 24, 2023 · e29876c · e29876c
1 parent 03da9d6
commit e29876c
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 12 deletions.
diff --git a/.changeset/afraid-ducks-know.md b/.changeset/afraid-ducks-know.md
@@ -0,0 +1,6 @@
+---
+"react-router-dom": patch
+"react-router-native": patch
+---
+
+Fix bug with search params removal
diff --git a/packages/react-router-dom/__tests__/search-params-test.tsx b/packages/react-router-dom/__tests__/search-params-test.tsx
@@ -125,4 +125,40 @@ describe("useSearchParams", () => {
     );
     expect(node.innerHTML).toMatch(/The new query is "Ryan Florence"/);
   });
+
+  it("allows removal of search params when a default is provided", () => {
+    function SearchPage() {
+      let [searchParams, setSearchParams] = useSearchParams({
+        value: "initial",
+      });
+
+      return (
+        <div>
+          <p>The current value is "{searchParams.get("value")}".</p>
+          <button onClick={() => setSearchParams({})}>Click</button>
+        </div>
+      );
+    }
+
+    act(() => {
+      ReactDOM.createRoot(node).render(
+        <MemoryRouter initialEntries={["/search?value=initial"]}>
+          <Routes>
+            <Route path="search" element={<SearchPage />} />
+          </Routes>
+        </MemoryRouter>
+      );
+    });
+
+    let button = node.querySelector<HTMLInputElement>("button")!;
+    expect(button).toBeDefined();
+
+    expect(node.innerHTML).toMatch(/The current value is "initial"/);
+
+    act(() => {
+      button.dispatchEvent(new Event("click", { bubbles: true }));
+    });
+
+    expect(node.innerHTML).toMatch(/The current value is ""/);
+  });
 });
diff --git a/packages/react-router-dom/dom.ts b/packages/react-router-dom/dom.ts
@@ -88,15 +88,17 @@ export function createSearchParams(
 
 export function getSearchParamsForLocation(
   locationSearch: string,
-  defaultSearchParams: URLSearchParams
+  defaultSearchParams: URLSearchParams | null
 ) {
   let searchParams = createSearchParams(locationSearch);
 
-  for (let key of defaultSearchParams.keys()) {
-    if (!searchParams.has(key)) {
-      defaultSearchParams.getAll(key).forEach((value) => {
-        searchParams.append(key, value);
-      });
+  if (defaultSearchParams) {
+    for (let key of defaultSearchParams.keys()) {
+      if (!searchParams.has(key)) {
+        defaultSearchParams.getAll(key).forEach((value) => {
+          searchParams.append(key, value);
+        });
+      }
     }
   }
 

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
@@ -853,13 +853,17 @@ export function useSearchParams(
   );
 
   let defaultSearchParamsRef = React.useRef(createSearchParams(defaultInit));
+  let hasSetSearchParamsRef = React.useRef(false);
 
   let location = useLocation();
   let searchParams = React.useMemo(
     () =>
+      // Only merge in the defaults if we haven't yet called setSearchParams.
+      // Once we call that we want those to take precedence, otherwise you can't
+      // remove a param with setSearchParams({}) if it has an initial value
       getSearchParamsForLocation(
         location.search,
-        defaultSearchParamsRef.current
+        hasSetSearchParamsRef.current ? null : defaultSearchParamsRef.current
       ),
     [location.search]
   );
@@ -870,6 +874,7 @@ export function useSearchParams(
       const newSearchParams = createSearchParams(
         typeof nextInit === "function" ? nextInit(searchParams) : nextInit
       );
+      hasSetSearchParamsRef.current = true;
       navigate("?" + newSearchParams, navigateOptions);
     },
     [navigate, searchParams]

diff --git a/packages/react-router-native/__tests__/__snapshots__/search-params-test.tsx.snap b/packages/react-router-native/__tests__/__snapshots__/search-params-test.tsx.snap
@@ -1,5 +1,30 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`useSearchParams allows removal of search params when a default is provided 1`] = `
+<View>
+  <Text>
+    The current query is "
+    initial
+    ".
+  </Text>
+  <View>
+    Click
+  </View>
+</View>
+`;
+
+exports[`useSearchParams allows removal of search params when a default is provided 2`] = `
+<View>
+  <Text>
+    The current query is "
+    ".
+  </Text>
+  <View>
+    Click
+  </View>
+</View>
+`;
+
 exports[`useSearchParams reads and writes the search string (functional update) 1`] = `
 <View>
   <Text>

diff --git a/packages/react-router-native/__tests__/search-params-test.tsx b/packages/react-router-native/__tests__/search-params-test.tsx
@@ -18,6 +18,10 @@ describe("useSearchParams", () => {
     return <View>{children}</View>;
   }
 
+  function Button({ children }: { children: React.ReactNode; onClick?: any }) {
+    return <View>{children}</View>;
+  }
+
   it("reads and writes the search string", () => {
     function SearchPage() {
       let [searchParams, setSearchParams] = useSearchParams({ q: "" });
@@ -112,4 +116,40 @@ describe("useSearchParams", () => {
 
     expect(renderer.toJSON()).toMatchSnapshot();
   });
+
+  it("allows removal of search params when a default is provided", () => {
+    function SearchPage() {
+      let [searchParams, setSearchParams] = useSearchParams({
+        value: "initial",
+      });
+
+      return (
+        <View>
+          <Text>The current query is "{searchParams.get("value")}".</Text>
+          <Button onClick={() => setSearchParams({})}>Click</Button>
+        </View>
+      );
+    }
+
+    let renderer: TestRenderer.ReactTestRenderer;
+    TestRenderer.act(() => {
+      renderer = TestRenderer.create(
+        <NativeRouter initialEntries={["/search?value=initial"]}>
+          <Routes>
+            <Route path="search" element={<SearchPage />} />
+          </Routes>
+        </NativeRouter>
+      );
+    });
+
+    expect(renderer.toJSON()).toMatchSnapshot();
+
+    let button = renderer.root.findByType(Button);
+
+    TestRenderer.act(() => {
+      button.props.onClick();
+    });
+
+    expect(renderer.toJSON()).toMatchSnapshot();
+  });
 });
diff --git a/packages/react-router-native/index.tsx b/packages/react-router-native/index.tsx
@@ -288,16 +288,19 @@ export function useSearchParams(
   defaultInit?: URLSearchParamsInit
 ): [URLSearchParams, SetURLSearchParams] {
   let defaultSearchParamsRef = React.useRef(createSearchParams(defaultInit));
+  let hasSetSearchParamsRef = React.useRef(false);
 
   let location = useLocation();
   let searchParams = React.useMemo(() => {
     let searchParams = createSearchParams(location.search);
 
-    for (let key of defaultSearchParamsRef.current.keys()) {
-      if (!searchParams.has(key)) {
-        defaultSearchParamsRef.current.getAll(key).forEach((value) => {
-          searchParams.append(key, value);
-        });
+    if (!hasSetSearchParamsRef.current) {
+      for (let key of defaultSearchParamsRef.current.keys()) {
+        if (!searchParams.has(key)) {
+          defaultSearchParamsRef.current.getAll(key).forEach((value) => {
+            searchParams.append(key, value);
+          });
+        }
       }
     }
 
@@ -310,6 +313,7 @@ export function useSearchParams(
       const newSearchParams = createSearchParams(
         typeof nextInit === "function" ? nextInit(searchParams) : nextInit
       );
+      hasSetSearchParamsRef.current = true;
       navigate("?" + newSearchParams, navigateOpts);
     },
     [navigate, searchParams]