fix: don't overwrite searchParam values if they are provided in url with what is provided in validateSearch #2855
Conversation
…s (overwriting values) when passing keys explicitly
…t already exists don't overwrite it back to default
|
@schiller-manuel sure! As soon as I get back home |
|
@schiller-manuel about #2830, yes its the same issue, this pr fixes that (and mine) but introduced another bug :D <Navigate ... search={{x:y}} /> wont add the sp. Gonna see what I did wrong :( as for #2799, I don't beleive its related, since the passed object to search param is being overwritten (presumably deleting keys from it directly), if we change their example to this instead: ...
search={{...DefaultIndexSearch}} // create a new object here
...it works as expected. |
…not empty string)
…rwriting it to result, so current stuff in result isn't removed
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 58eacb9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
| @@ -9,13 +9,14 @@ export function retainSearchParams<TSearchSchema extends object>( | |||
| return ({ search, next }) => { | |||
| const result = next(search) | |||
| if (keys === true) { | |||
| return { ...search, ...result } | |||
| return { ...result, ...search } | |||
There was a problem hiding this comment.
I don't get this. retainSearchParams should only carry over search params that are NOT part of e.g. a <Link search={...}>. but this change here would mean that if the search params already contained foo: 'hello', then the value from <Link search={{foo:'bar'}}> would be ignored
There was a problem hiding this comment.
Shouldn't it? The value is retained, whatever it's initialized to. Imagine you go to route x with param=p1, then goto x/y with search param=p2, this would mean the param is p2 when ur on x/y and reset back to p1 when go on x. This means the value is not retained on x/* route right? If this is not the intention (which can be achieved with validateSearth alone), I might have misunderstood the point.
There was a problem hiding this comment.
no, the intention of this search middleware is to keep search params around so they continue to live on with their previous value unless a Link overrides them
There was a problem hiding this comment.
Ooh, ok then. Feel free to close this PR, cuz I'm not home for next 2 or 3 days.
- next(search) returns the validated result (including what's in the links search prop) so it's not gonna be as easy as this fix.
There was a problem hiding this comment.
no rush, just convert it to draft for the time being
| } | ||
| // add missing keys from search to result | ||
| // just like above, but only overwrite explicitly mentioned keys |
There was a problem hiding this comment.
same here, result should have precedence
|
@schiller-manuel Currenly, search middlewares call next which calls searchValidator, meaning, const postsRoute = createRoute({
getParentRoute: () => rootRoute,
path: 'posts',
validateSearch: z.object({
foo: z.string().default('default'),
}),
search: {
middlewares: [
// @ts-expect-error we cannot use zodValidator here to due to circular dependency
// this means we cannot get the correct input type for this schema
stripSearchParams({ foo: 'default' }),
retainSearchParams(true),
],
},
component: () => {
const { foo } = postsRoute.useSearch()
return (
<>
<h1>Posts</h1>
<div data-testid="posts-search">{foo}</div>
<Link data-testid="posts-link-new" to="/posts/new">
new
</Link>
</>
)
},
})here we are explicitly saying that this route should have |
|
|
Hi, thanks for your reply. |
Title is actually good enough :D
Fixes #2845
Fixes #2830