fix: consecutive updates accumulate #233

packages/use-query-params-adapter-react-router-5/src/index.ts

@@ -11,7 +11,12 @@ import {
 export const ReactRouter5Adapter: QueryParamAdapterComponent = ({
   children,
 }) => {
+  // note we need to useLocation() to get re-renders when location changes
+  // but we prefer to read location directly from history to fix #233
+  // @ts-ignore-line
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const location = useLocation();
+
   const history = useHistory();
 
   const adapter: QueryParamAdapter = {
@@ -22,7 +27,7 @@ export const ReactRouter5Adapter: QueryParamAdapterComponent = ({
       history.push(location.search || '?', location.state);
     },
     get location() {
-      return location;
+      return history.location;
     },
   };
 

packages/use-query-params-adapter-react-router-6/src/index.ts

@@ -1,5 +1,6 @@
-// @ts-ignore
+import { useContext } from 'react';
 import { useNavigate, useLocation } from 'react-router';
+import { UNSAFE_NavigationContext } from 'react-router-dom';
 import {
   QueryParamAdapter,
   QueryParamAdapterComponent,
@@ -11,6 +12,12 @@ import {
 export const ReactRouter6Adapter: QueryParamAdapterComponent = ({
   children,
 }) => {
+  // we need the navigator directly so we can access the current version
+  // of location in case of multiple updates within a render (e.g. #233)
+  // but we will limit our usage of it and have a backup to just use
+  // useLocation() output in case of some kind of breaking change we miss.
+  // see: https://github.com/remix-run/react-router/blob/f3d87dcc91fbd6fd646064b88b4be52c15114603/packages/react-router-dom/index.tsx#L113-L131
+  const { navigator } = useContext(UNSAFE_NavigationContext);
   const navigate = useNavigate();
   const location = useLocation();
 
@@ -28,7 +35,8 @@ export const ReactRouter6Adapter: QueryParamAdapterComponent = ({
       });
     },
     get location() {
-      return location;
+      // be a bit defensive here in case of an unexpected breaking change in React Router
+      return (navigator as any)?.location ?? location;
     },
   };
 

packages/use-query-params/src/__tests__/routers/mocked.test.tsx

@@ -66,9 +66,8 @@ const TestRouter = ({
  */
 const TestAdapter: QueryParamAdapterComponent = ({ children }) => {
   const history = React.useContext(TestRouterContext);
-  const [location, setLocation] = React.useState<PartialLocation>(
-    history.location
-  );
+  // need a use state here to force a re-render
+  const [, setLocation] = React.useState<PartialLocation>(history.location);
   React.useLayoutEffect(() => {
     history.onChange = setLocation;
   }, [history]);
@@ -80,8 +79,10 @@ const TestAdapter: QueryParamAdapterComponent = ({ children }) => {
     push(newLocation) {
       history.push(newLocation);
     },
+
+    // note this always reads the latest in history to fix #233
     get location() {
-      return location;
+      return history.location;
     },
   };
   return children(adapter);

packages/use-query-params/src/__tests__/routers/shared.tsx

@@ -327,15 +327,12 @@ export function testSpec(renderWithRouter: any) {
       expect(queryByText(/{"f":2,"g":"b","h":7}/)).toBeTruthy();
     });
 
-    it('multiple updates in same callback work (batching) with useQueryParam', async () => {
-      let numRenders = 0;
+    it('multiple updates in same callback work (no batching) with useQueryParam', async () => {
       const TestComponent = () => {
         const [foo, setFoo] = useQueryParam('foo');
         const [bar, setBar] = useQueryParam('bar');
         const [baz, setBaz] = useQueryParam('baz');
 
-        numRenders += 1;
-
         return (
           <div>
             {JSON.stringify({ foo, bar, baz })}
@@ -354,10 +351,9 @@ export function testSpec(renderWithRouter: any) {
       const { queryByText, getByText } = renderWithRouter(
         <TestComponent />,
         '?foo=foo1&bar=bar1&baz=baz1',
-        { enableBatching: true }
+        { enableBatching: false }
       );
 
-      expect(numRenders).toBe(1);
       expect(
         queryByText(/{"foo":"foo1","bar":"bar1","baz":"baz1"}/)
       ).toBeTruthy();
@@ -367,25 +363,25 @@ export function testSpec(renderWithRouter: any) {
           queryByText(/{"foo":"foo2","bar":"bar2","baz":"baz2"}/)
         ).toBeTruthy()
       );
-      expect(numRenders).toBe(2);
     });
 
-    it('multiple updates in same callback work (batching) with useQueryParams', async () => {
+    it('multiple updates in same callback work (batching) with useQueryParam', async () => {
       let numRenders = 0;
       const TestComponent = () => {
-        const [{ foo }, setFoo] = useQueryParams({ foo: StringParam });
-        const [{ bar }, setBar] = useQueryParams({ bar: StringParam });
-        const [{ baz }, setBaz] = useQueryParams({ baz: StringParam });
+        const [foo, setFoo] = useQueryParam('foo');
+        const [bar, setBar] = useQueryParam('bar');
+        const [baz, setBaz] = useQueryParam('baz');
 
         numRenders += 1;
+
         return (
           <div>
             {JSON.stringify({ foo, bar, baz })}
             <button
               onClick={() => {
-                setFoo({ foo: 'foo2' });
-                setBar({ bar: 'bar2' });
-                setBaz({ baz: 'baz2' });
+                setFoo('foo2');
+                setBar('bar2');
+                setBaz('baz2');
               }}
             >
               Change
@@ -399,6 +395,7 @@ export function testSpec(renderWithRouter: any) {
         { enableBatching: true }
       );
 
+      expect(numRenders).toBe(1);
       expect(
         queryByText(/{"foo":"foo1","bar":"bar1","baz":"baz1"}/)
       ).toBeTruthy();
@@ -411,23 +408,22 @@ export function testSpec(renderWithRouter: any) {
       expect(numRenders).toBe(2);
     });
 
-    it('disabling batching works', async () => {
+    it('multiple updates in same callback work (batching) with useQueryParams', async () => {
       let numRenders = 0;
       const TestComponent = () => {
-        const [foo, setFoo] = useQueryParam('foo');
-        const [bar, setBar] = useQueryParam('bar');
-        const [baz, setBaz] = useQueryParam('baz');
+        const [{ foo }, setFoo] = useQueryParams({ foo: StringParam });
+        const [{ bar }, setBar] = useQueryParams({ bar: StringParam });
+        const [{ baz }, setBaz] = useQueryParams({ baz: StringParam });
 
         numRenders += 1;
-
         return (
           <div>
             {JSON.stringify({ foo, bar, baz })}
             <button
               onClick={() => {
-                setFoo('foo2');
-                setBar('bar2');
-                setBaz('baz2'); // the above get clobbered if batching is off
+                setFoo({ foo: 'foo2' });
+                setBar({ bar: 'bar2' });
+                setBaz({ baz: 'baz2' });
               }}
             >
               Change
@@ -438,19 +434,19 @@ export function testSpec(renderWithRouter: any) {
       const { queryByText, getByText } = renderWithRouter(
         <TestComponent />,
         '?foo=foo1&bar=bar1&baz=baz1',
-        { enableBatching: false }
+        { enableBatching: true }
       );
 
-      expect(numRenders).toBe(1);
       expect(
         queryByText(/{"foo":"foo1","bar":"bar1","baz":"baz1"}/)
       ).toBeTruthy();
       getByText(/Change/).click();
       await waitFor(() =>
         expect(
-          queryByText(/{"foo":"foo1","bar":"bar1","baz":"baz2"}/)
+          queryByText(/{"foo":"foo2","bar":"bar2","baz":"baz2"}/)
         ).toBeTruthy()
       );
+      expect(numRenders).toBe(2);
     });
   });
 }

packages/use-query-params/src/urlName.ts

@@ -42,8 +42,8 @@ export function applyUrlNames(
 ) {
   let newEncodedValues: Partial<EncodedValueMap<any>> = {};
   for (const paramName in encodedValues) {
-    if (paramConfigMap[paramName]?.urlName) {
-      newEncodedValues[paramConfigMap[paramName].urlName] =
+    if (paramConfigMap[paramName]?.urlName != null) {
+      newEncodedValues[paramConfigMap[paramName].urlName!] =
         encodedValues[paramName];
     } else {
       newEncodedValues[paramName] = encodedValues[paramName];