47ng/nuqs

Interaction between `useQueryState`, `useEffect`, and a React server action

Closed this issue · 10 comments

zack commented

Context

What's your version of nuqs?

^1.17.5

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

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP Fri Mar 29 23:14:13 UTC 2024
  Available memory (MB): 32052
  Available CPU cores: 24
Binaries:
  Node: 18.20.4
  npm: 10.7.0
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.2.5 // Latest available version is detected (14.2.5).
  eslint-config-next: 14.2.5
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.2
Next.js Config:
  output: N/A

Are you using:

Description

I am certainly open to the idea that I'm not understanding something here, but I'm seeing normal behavior with React.useState and buggy behavior with a clean replacement using nuqs, so I'm not entirely sure where to go from here. I appreciate your hard work and am hoping you'll have a moment to take a look at this and let me know your thoughts. Thank you. :)

BoroughData.tsx is using nuqs to keep track of what borough is currently selected. I am using a custom parser in order to keep Typescript happy, but otherwise it's just a very basic use of useQueryState.

When the borough is changed in the UI and setBorough is called, resulting in a change in the value of borough, a useEffect hook is triggered, which calls getTimeframeData, a server action defined here, which is the behavior that is suggested by the next.js docs.

This works perfectly the first time a user selects a borough. The second time a user selects a borough, the application UI is set to loading, and line 68 is executed, but this console.log, which is the first line of the server action, never runs. On the third usage, everything works fine. It seems to bounce back and forth between working and not working.

This logic could instead be run in the change event handler, but it needs to be (potentially) executed on load anyway, which would be in a useEffect hook, so might as well just do it there every time. Regardless, moving the functionality out of a useEffect hook and into the event handler does not resolve the issue.

Everything works as expected when using React.useState.

There are no warnings or errors in the console at any time.

The relevant bits of BoroughData.tsx, console.logs removed for brevity:

const parseAsBorough = createParser({
  parse(queryValue) {
    return isBorough(queryValue) ? queryValue : '';
  },
  serialize(value) {
    return value;
  },
});

export default memo(function BoroughData() {
  const [borough, setBorough] = useQueryState(
    'borough',
    parseAsBorough.withDefault(''),
  );
  
  // ...

  React.useEffect(() => {
    let ignore = false;

    if (borough) {
      setIsLoading(true);
      setTimeframe(undefined);
      getTimeframeData({ station: { borough } })
        .then((newData) => {
          if (!ignore) {
            setTimeframe(newData);
            setIsLoading(false);
          }
        });
    }

    return () => {
      ignore = true;
    };
  }, [borough]);

  // ...

Here's some more interesting information. On the second selection, the POST request to the server has the wrong querystring. On the third selection, it posts the old querystring (the second one, that is now old), and then the new one. That's weird.

2024-07-26.22-34-25-2.mp4

Thanks again for building and maintaining such a lovely library. :)

Reproduction

The branch, deployed: https://citibike-git-querystring-everything-ravenouswombats-projects.vercel.app/

To reproduce:

  1. Open the app to the borough view
  2. Select a borough from the dropdown
  3. Notice the page load, then populate with data
  4. Select a different borough from the dropdown
  5. Notice the page load forever
    a. Expectation: the page would load, then populate
  6. Select a different borough from the dropdown
  7. Notice the page load, then populate with data

Thanks for the detailed report!

The main difference with useState is that useQueryState will re-render a couple more times during an update, due to the internals of nuqs (setting an internal state immediately, then defering the URL update to satisfy rate limits on the History API imposed by browsers) and that of Next.js (re-rendering every component that uses useSearchParams after the URL changes).

Could it be that multiple calls to the getTimeframeData server action with the same arguments are cached, and this messes up the ignore mechanism?

zack commented

Funny enough, I hadn't implemented the ignore mechanism anywhere on the site until I was running into this issue. It was one of my attempts to resolve it.

Just for fun, I pushed all of the Nuqs logic for this component down into a new component. This new logic is not something I would want to use, and it would not correctly set the state on load, but I was curious if it solve the issue. It reduced the number of renders being experienced by BoroughData.tsx by 2-3 per borough select, but interestingly it did not solve the problem.

BoroughData.tsx:

  // ..

  const [borough, setBorough] = React.useState<Borough | ''>('');

  // ...

  return (
    <>
      <Nuqs borough={borough} />
      // ..
    </>
  );

// ..

The new Nuqs.tsx:

const parseAsBorough = createParser({
  parse(queryValue) {
    return isBorough(queryValue) ? queryValue : '';
  },
  serialize(value) {
    return value;
  },
});

export default function Nuqs({ borough }) {
  const [_, setBorough] = useQueryState(
    'borough',
    parseAsBorough.withDefault(''),
  );

  React.useEffect(() => {
    setBorough(borough);
  }, [borough, setBorough]);

  return null;
}

Is this still relevant or can this issue be closed?

zack commented

Hi @franky47. Unfortunately I was not able to resolve it. With the release of v2 I was hoping to get back into it over the winter break and try it again. :) I'll update soon once I'm able to try it out. Thanks!

zack commented

Unfortunately I'm seeing the same behavior with 2.2.3. :( The line just before getTimeframeData executes but a console.log in the first line of getTimeframeData is not executed. It's weird because it seems like nuqs is doing exactly what it's supposed to do. The state updates, the new state is what it's supposed to be, the effect executes because the dep changed, etc. But also the server action breaks only when nuqs is used. It's a real head scratcher.

Would you be able to try and replicate the issue in a minimal setup (create-next-app)?

I haven't had much chance to dig into how server actions change the rendering process in Next.js, only know that a good part of the app router core changes in the past year or so were about handling those.

Have you tried updating Next.js itself to the latest version (if possible)?

zack commented

I will try both and get back to you soon. :)

zack commented

(tl;dr: fixed, somehow)

  • I directly upgraded the app to React 19 and Next.js 15. This did not solve the problem.
  • I tried to build a minimal setup, but no matter how closely I tried to get it to match the failing code, it worked and I was unable to reproduce the issue.
  • Back to the real app, when I commented out setIsLoading(true); and setTimeframe(undefined); which were right above the failing server action call, the code worked.
  • I swapped out the isLoading for useTranslation and I removed the setTimeframe(undefined) (solving this elsewhere) and now it works.
  • I ran into the problem again, and it seemed to be caused by a child component making a call to a server action while some of the data was stale? Maybe? But there were no exceptions being thrown anywhere, and no linting errors indicating bad types, so I'm not sure what was going on there. And at this point I've upgraded React and Next, swapped in useTranslation in a bunch of places, swapped in the new React compiler, and upgraded MUI, so ... there have been a lot of changes.

Anyway, I'm sorry I wasn't able to figure out exactly what the situation was that was causing the problem, but either it was a problem with my code or I was able to work around it without too much trouble. I think being able to solve it by removing the two state setters is the biggest clue, but to what? I'm not sure.

useTranslation

Did you mean useTransition? That would indeed be the correct way to know when an action is in flight, or there is also useActionState that has a pending property.

Glad you managed to make it work!

zack commented

Just deployed the updates using nuqs at https://citibikedata.nyc and it's working splendidly. Thanks for the library, it really simplified the process of getting this feature out!