Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cursor jumps to end of search params bound input #3162

Open
Nick-Lucas opened this issue Jan 14, 2025 · 5 comments
Open

Cursor jumps to end of search params bound input #3162

Nick-Lucas opened this issue Jan 14, 2025 · 5 comments

Comments

@Nick-Lucas
Copy link

Which project does this relate to?

Router

Describe the bug

It's common to bind a Filter input to search params so that URLs can be shared, and a user should be able to type anywhere in the input and it behave predictably.

Current behaviour of this isn't predictable though, if you type into the end of an input it behaves fine, but if you type into the middle, the cursor jumps to the end.

I suspect this is because startTransition is called by default under the hood now, but there doesn't appear to be a way to disable this behaviour or any past github issue which describes a workaround or fix

Your Example Website or App

Example project: search-validator-adapters

Steps to Reproduce the Bug or Issue

  1. Boot up the search-validator-adapters example project
  2. Navigate to a tab (I like the zod one 😉)
  3. Type some text into the input
  4. Change cursor position to middle of the text
  5. type 1 character
  6. the cursor jumps to the end 😱

Expected behavior

Typing anywhere in the input types where your cursor is and retains cursor position

Screenshots or Videos

Image

Platform

  • OS: Mac and Windows
  • Browser: Chrome (tested)

Additional context

No response

@Nick-Lucas
Copy link
Author

Nick-Lucas commented Jan 15, 2025

I've added a failing test here: #3174

I'm probably not the right person to fix it simply because I understand why startTransition is now the default, but I can see this being a tricky one to balance the need for startTransition with the need for binding inputs successfully. Also startTransition is probably not the whole story here, in theory that shouldn't affect binding but I've seen it do so before

@schiller-manuel
Copy link
Contributor

schiller-manuel commented Jan 24, 2025

as the router state is outside of react, React seems to not be able to track the cursor position as it would with a local useState.

This works around that:

const Zod = () => {
  const search = Route.useSearch({
    select: (search) => search.search ?? '',
  })
  const navigate = useNavigate({ from: Route.fullPath })

  const [state, setState] = React.useState(search)
  return (
    <>
      <Header title="Zod" />
      <Content>
        <Search
          search={state}
          onChange={(v) => {setState(v); navigate({search: { search: v } })}}
        />
        <React.Suspense>
          <Users search={search} />
        </React.Suspense>
      </Content>
    </>
  )
}

A different solution is to use an uncontrolled input via defaultValue:

export const Search = ({ search, onChange }: SearchProps) => {
  return (
    <div className="flex gap-2 align-center items-center">
      <label>Search</label>
      <input
        type="search"
        className="border p-1 px-2 rounded"
        defaultValue={search}
        onChange={(e) => onChange(e.target.value)}
      ></input>
    </div>
  )
}

@Nick-Lucas
Copy link
Author

So maybe the bugfix is the react-router package needs to have its own in-react cache which synchronises out to the router-core? Possibly with useOptimistic instead of useState?

Would you accept a PR for that?

@Nick-Lucas
Copy link
Author

Nick-Lucas commented Jan 25, 2025

Done some investigation today. useSearch and likely useMatch (I only checked useSearch though) have had this bug at least as far back as v1.26.8

useLocation though does not have the bug at all, and the fact it also includes a selector makes it useful as a workaround for now

Image

@Nick-Lucas
Copy link
Author

Nick-Lucas commented Jan 25, 2025

Okay PR updated with test cases against all 3 hooks. 1 test case passes

Image

These hooks have the following dependencies:

useLocation->useRouterState
useMatch->useRouterState
useSearch->useMatch->useRouterState

While all lean on useRouterState, useMatch/useSearch takes its location/search state from state.matches[?].search and useLocation takes its search state from state.location. state.matches seems to update asynchronously whereas state.location updates synchronously. That's the cause of this bug

It can be hackily fixed for this narrow use case with this change:

export function useMatch<
  TRouter extends AnyRouter = RegisteredRouter,
  const TFrom extends string | undefined = undefined,
  TStrict extends boolean = true,
  TThrow extends boolean = true,
  TSelected = unknown,
  TStructuralSharing extends boolean = boolean,
>(
  opts: UseMatchOptions<
    TRouter,
    TFrom,
    TStrict,
    TSelected,
    ThrowConstraint<TStrict, TThrow>,
    TStructuralSharing
  >,
): ThrowOrOptional<UseMatchResult<TRouter, TFrom, TStrict, TSelected>, TThrow> {
  // const nearestMatchId = React.useContext(
  //   opts.from ? dummyMatchContext : matchContext,
  // )

  const matchSelection = useRouterState({
    select: (state: any) => {
      // const match = state.matches[1].find((d: any) =>
      //   opts.from ? opts.from === d.routeId : d.id === nearestMatchId,
      // )
      // invariant(
      //   !((opts.shouldThrow ?? true) && !match),
      //   `Could not find ${opts.from ? `an active match from "${opts.from}"` : 'a nearest match!'}`,
      // )

      // if (match === undefined) {
      //   return undefined
      // }

      return opts.select ? opts.select({search: state.location.search} as any) : {search: state.location.search}
    },
    structuralSharing: opts.structuralSharing,
  } as any)

  return matchSelection as any
}

I see 3 possible solutions here:

  1. don't return match from useMatch because it's async (not really any option)
  2. don't use useMatch inside useSearch, lean on useLocation instead (but this gives up the route matching / validation)
  3. Change the behaviour of the matches list to update synchronously (this sounds a lot like it's getting into the innards of the router and would be a fundamental change to behaviour)
  4. Patch search on useMatch (I'll commit this for now to fix this case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants