Multiple reenders on Select with Nextjs or Remix useSearchParams

[juliomuhlbauer]

🐛 Bug report

When updating a context on a machine with a search param from hooks from Nextjs and Remix, causes multiple reenders.

Using other solutions as Ariakit and Radix works fine with Nextjs.
I also tested Ark with Nextjs and Remix. It causes reenders too.

💥 Steps to reproduce

1. Clone the repo: https://github.com/juliomuhlbauer/zag/tree/example-search-param
2. Go to the example: https://github.com/juliomuhlbauer/zag/tree/example-search-param/examples/next-ts/app/search-param-zag
3. Try to change the search param on development
4. You will see the example causes multiple reenders

💻 Link to reproduction

It seems that in production works better. But in Dev is unusable. Also, Ark on production causes reenders too.

Default Zag (reenders on dev): https://zag-search-param-git-example-search-param-juliomuhlbauer.vercel.app/search-param-zag
Default Ark (reenders on prod): https://zag-search-param-git-example-search-param-juliomuhlbauer.vercel.app/search-param-ark
Repo Branch: https://github.com/juliomuhlbauer/zag/tree/example-search-param

Fix Zag (works): https://zag-search-param-git-fix-reenders-search-param-juliomuhlbauer.vercel.app/search-param-zag
Fix Ark (doesn't work yet): https://zag-search-param-git-fix-reenders-search-param-juliomuhlbauer.vercel.app/search-param-ark
Repo Branch: https://github.com/juliomuhlbauer/zag/tree/fix-reenders-search-param

🧐 Expected behavior

Only reender when changing the searchParam

🧭 Possible Solution

Passing the context directly to the state works fine:

useMachine function:

export function useMachinePassingContextDirectly<
  TContext extends Record<string, any>,
  TState extends S.StateSchema,
  TEvent extends S.EventObject = S.AnyEventObject,
>(machine: MachineSrc<TContext, TState, TEvent>, options?: S.HookOptions<TContext, TState, TEvent>) {
  const service = useService(machine, options)

  const stateSnapshot = useSnapshot(service.state)

  const state = {
    ...stateSnapshot,
    context: service.state.context,
  }
  const typedState = state as unknown as S.State<TContext, TState, TEvent>
  return [typedState, service.send, service] as const
}

And filtering the context prop in the proxy:

const addPropListener = (prop: string | symbol, propProxyState: ProxyState) => {
      if (isDev && propProxyStates.has(prop)) {
        throw new Error("prop listener already exists")
      }
      if (listeners.size) {
        const remove = propProxyState[3](createPropListener(prop))
        propProxyStates.set(prop, [propProxyState, remove])
      } else {
        if (prop === "context") {
          // console.log("context being added multiple times")
        } else {
          propProxyStates.set(prop, [propProxyState])
        }
      }
    }

🌍 System information

Software	Version(s)
Zag Version	0.31.1
Browser	Chrome
Operating System	Windows

📝 Additional information

Other Zag issue: #1073
Ark issue: chakra-ui/ark#1817

Nextjs + Ariakit works fine:
Repo: https://github.com/juliomuhlbauer/use-search-param-ark/blob/main/src/app/search-param-ariakit/page.tsx
Demo: https://use-search-param-ark.vercel.app/search-param-ariakit?city=Jambeiro

2023-12-08.14-31-51.mp4

[juliomuhlbauer]

I updated the issue because the examples were sharing the same zag logic.
Now I created 2 branches: one with the default behavior and the other with a temporary fix.

[juliomuhlbauer]

As I had put here: #1073 (comment), the problem appears when there are more items in the context. Sometimes with fewer items happens too. But I can always reproduce this bug with more items in the collection.

So I opened a PR to illustrate this in the Next example that you created: #1093

If you can test this in your machine I would appreciate. Also, if we could deploy this version to prod to see the effects there too.

Thanks very much in advance!

Repro:

2023-12-14.13-17-06.mp4

[segunadebayo]

It's related to inconsistent handling of state in useSyncExternalStore:

remix-run/react-router#10377
https://codesandbox.io/p/sandbox/use-sync-external-store-loop-9g7b81?file=%2Fsrc%2Findex.js%3A37%2C21

[segunadebayo]

Switching to good'ol useState+useEffect solves the issue

[juliomuhlbauer]

You say that this is a framework level problem of both Next and Remix?

Because it seems that the PR was merged, but Remix also has the same problem.

Nextjs + Ariakit works fine, so I don't think is a Next problem too:
Repo: https://github.com/juliomuhlbauer/use-search-param-ark/blob/main/src/app/search-param-ariakit/page.tsx
Demo: https://use-search-param-ark.vercel.app/search-param-ariakit?city=Jambeiro

[segunadebayo]

Lol. I meant this might be a race condition in Zag.js due to the use of the sync external store hook in the code.

Feel free to use Ariakit if you find meets your needs. I’ll look into this a bit more