47ng / nuqs

Type-safe search params state manager for Next.js - Like React.useState, but stored in the URL query string.
https://nuqs.47ng.com
MIT License
3.42k stars 69 forks source link

Twice render on update `useQueryStates's` field #516

Closed Kavan72 closed 2 weeks ago

Kavan72 commented 7 months ago

Context

What's your version of nuqs?

"nuqs": "^1.17.1"

Next.js information (obtained by running next info):

Operating System:
  Platform: linux
  Arch: x64
  Version: #107~20.04.1-Ubuntu SMP Fri Feb 9 14:20:11 UTC 2024
Binaries:
  Node: 18.19.1
  npm: 10.2.4
  Yarn: 1.22.19
  pnpm: 8.9.2
Relevant Packages:
  next: 14.1.1
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.3.3
Next.js Config:
  output: N/A

Are you using:

Description

I'm using useQueryStates with multiple states, when i update any state using this

setFilter(previousState => {
    return {
        query: query
    }
}),

then it's render twice, i've added one trace hook on home page to debug how many times render and which prevent the multiple render.

Reproduction

you can find the minimal code here:- https://github.com/Kavan72/nuqs-test

phipag commented 6 months ago

I can confirm that. It re-renders twice on Next.js 14.1.0 and only once on Next.js 14.0.4. Is this possibly related to Next.js starting to support native window.history.pushState methods? https://nextjs.org/blog/next-14-1#windowhistorypushstate-and-windowhistoryreplacestate

franky47 commented 6 months ago

Yes, this is due to useSearchParams becoming reactive to shallow URL changes since 14.1.0 (or previous versions with the windowHistorySupport experimental flag).

In order to keep the UI responsive, the internal state is updated immediately as a setState from React.useState would, which triggers the first render. Then the URL is updated, useSearchParams picks it up, and triggers the second render.

In order to have only a single render, we'd have to disable the internal state and only derive values from useSearchParams, which would work fine for some "low frequency" UIs, but not at all for "high frequency" state updates (eg: typing text in a search input, or moving a slider, when using controlled components).

phipag commented 6 months ago

I understand. Is useSearchParams used internally in nuqs? I don't use it anywhere in my code.

franky47 commented 6 months ago

Yes, it's used internally to obtain the search params during SSR for dynamic pages, and at mount time when rendering client-side.

phipag commented 6 months ago

Got it. I just tested using useSearchParams as single "source of truth" for the state and the UI feels less "snappy" because it re-loads the full page before applying the state change in the query params.

Do you have a plan/idea for nuqs to support this new behavior of Next.js with a single re-render like for Next.js <=14.04?

franky47 commented 6 months ago

it re-loads the full page before applying the state change in the query params.

That sounds like a bug, you should try using the latest next@14.1.2 which was published yesterday, it includes a fix for full-page reloads when combined with fragment anchor tags (see #498).

The whole "mental model" of how data flows through nuqs as @tordans mentioned in #408 needs to be laid out in the docs, with possible escape hatches to accommodate for various rendering patterns. Having an option to control what state is updated when would be a nice feature.

The issue becomes retro-compatibility with older versions of Next.js and the app/pages router behaviour differences, there are already a lot of hacks in place to make sure it all works fine. I've been considering dropping support for older Next.js versions in nuqs@^2, to remove those hacks and focus on making it future proof. Compatibility for the version range where shallow routing was in flux (14.0.2 to 14.1.1 included) will definitely be dropped, as there are contradictory behaviours which nuqs only supports through version detection. It's a matter of knowing what to do for 13.1.x to 14.0.1.

phipag commented 6 months ago

That sounds like a bug, you should try using the latest next@14.1.2 which was published yesterday

I mis-explained. It is not doing a hard reload but rather re-renders the page on the client-side (shows in XHR network requests in browser). Not a bug but expected for router.push from next/navigation, right?

Thanks for the explanation. A clarification of this would indeed be nice because it might confuse users (like me) why multiple re-renders occur when updating query state.

Kavan72 commented 6 months ago

@franky47 @phipag i just pushed the updated version of next.js (14.1.2), seems like issue is still there, also update on my repo https://github.com/Kavan72/nuqs-test

phipag commented 6 months ago

@Kavan72 I don't think this behavior is changed in Next.js 14.1.0 vs. 14.1.2. As @franky47 explained, two re-renders are expected for all Next.js versions which support the native window.history.push operation. The first re-render will update the internal state of nuqs and the second re-render is caused by the Next.js useSearchParams hook picking up the change of the browser URL.

As I understand it, updating the internal state first will make the UI feel quicker because it does not block until the browser URL is updated.

Kavan72 commented 6 months ago

@phipag okay thanks for clarification, btw i've found one more interesting bug, suppose i've 3 states

export const FilterType = {
    query: parseAsString.withDefault(''),
    leadStatus: customParseAsArrayOf(parseAsString).withDefault(['CONFIRMED', 'QUALIFIED', 'PENDING']),
    withRecognizedTerms: parseAsBoolean.withDefault(true)
}

export default function Home() {

    const [filters, setFilter] = useQueryStates(FilterType, {
        clearOnDefault: true,
    })

    useEffect(() => {
        console.log(filters)
    }, [filters.query, filters.leadStatus]);

    return (
        <Test filters={filters} setFilter={setFilter}/>
    );
}

if i update query state then useEffect will call once but if i update leadStatus which is array state then useEffect will call twice

franky47 commented 6 months ago

@Kavan72 that might be due to the double render from useSearchParams as well. After setting the state, the URL change will cause the parser to run and reset the state to that new value. It's likely the same value, but results in a new JS object instance.

For a string state, recreating a new instance with the same content won't trigger the dependency change detection in useEffect, but creating a new array will give a different reference, even if the contents are the same.

This is similar to what happened when setting a state with identical contents to the default value.

Unfortunately, since we can't tell React to use the parsers' eq function, you'd have to either make your app robust to multiple renders (data fetching in useEffect can break in many more ways), or change the dependency array to check for content equality (eg: with JSON.stringify).

Kavan72 commented 6 months ago

@franky47 can we create some kind of unique hash to useEffect and have some parser for state and url 🤔

franky47 commented 6 months ago

You don't need a hash:

useEffect(() => {}, [JSON.stringify(state)])
Kavan72 commented 6 months ago

@franky47, I understand your point, but my question is: can't we use the same reference to update non-primitive data types to avoid this problem? I'm fine with using JSON.stringify for non-primitive data types; I just want to improve this library as much as we can 😊.

franky47 commented 6 months ago

I did some experiments with a client-side cache of state references (keyed by the serialized search param string), which works fine to ensure referential stability for updates, but it breaks upon reloading the page when that cache gets rebuilt, so it's not super reliable.

franky47 commented 2 weeks ago

PSA: nuqs@1.19.1 was released with a fix for this issue. Non-primitive states (arrays, objects etc) are now referentially stable when their values don't change. This will help reducing the amount of unnecessary useEffect re-runs.

Note that this does not prevent re-renders, unfortunately one side-effect of depending on Next.js' useSearchParams is that it re-renders anytime any search param changes.

phipag commented 2 weeks ago

Thanks, I will test this soon.