solid-router icon indicating copy to clipboard operation
solid-router copied to clipboard

`useSearchParams` suggestion

Open ghost opened this issue 2 years ago • 14 comments

It would be very needed that setSearchParams from solid-start works like setting state, meaning it holds previous value and overwrites existing state when omitting prev. That way, it both works when you want to overwrite existing or append.

setSearchParams({ hello: "world" }) //overwrites existing
setSearchParams(prev => ({...prev, hello: "world" })) //appends

ghost avatar May 28 '23 08:05 ghost

Undefined for delete ?

setSearchParams(prev => ({...prev, hello: undefined })) // delete

mtt-artis avatar May 28 '23 08:05 mtt-artis

If searchParams returned an Object instead of a Proxy, one could do:

setSearchParams(prev => {
  delete prev["hello"]
  return prev
})

It should return an object imo:

export function useSearchParams() {
  let initial = new URLSearchParams(isServer ? {} : window.location.search);

  const params = () => {
    const obj = {};
    for (const v of initial.keys()) obj[v] = initial.get(v);
    return obj;
  };

  const add = obj => {
    for (const key in obj) initial.append(key, obj[key]);
    const search = initial.length ? `?${initial.toString()}` : "";
    window.history.replaceState(null, "", window.location.pathname + search);
  };

  const setParams = arg => {
    if (typeof arg === "object") {
      initial = new URLSearchParams(); //reset
      add(arg);
    } else if (arg instanceof Function) {
      const old = params();
      initial = new URLSearchParams(); //reset
      add(arg(old));
    }
  };

  return [params, setParams];
}

rattleSSnake avatar May 28 '23 12:05 rattleSSnake

i think proxy is needed for reactivity the current useSearchParams function could be updated didn t try tbh

https://github.com/solidjs/solid-router/blob/HEAD/src/routing.ts#L100-L115

export const useSearchParams = <T extends Params>(): [
  T,
  (newParams: SetParams | ((current: Readonly<T>) => SetParams), options?: Partial<NavigateOptions>) => void
] => {
  const location = useLocation();
  const navigate = useNavigate();
  const setSearchParams = (params: SetParams | ((params: Readonly<T>) => SetParams), options?: Partial<NavigateOptions>) => {
    const searchString = typeof params === "function"
      ? untrack(() => mergeSearchString("", params(location.query as T)))
      : untrack(() => mergeSearchString(location.search, params));
    navigate(location.pathname + searchString + location.hash, {
      scroll: false,
      resolve: false,
      ...options
    });
  };
  return [location.query as T, setSearchParams];
};

mtt-artis avatar May 28 '23 14:05 mtt-artis

But since it is a proxy, how would you deal this situation ?

setSearchParams(prev => {
  delete prev["something"];
  console.log(prev)
  return prev
});

Also on this line it should be:

: untrack(() => mergeSearchString("", params));

to make overwrite work

rattleSSnake avatar May 28 '23 14:05 rattleSSnake

you might be able to use produce https://www.solidjs.com/docs/latest/api#produce again, i didn t try


: untrack(() => mergeSearchString("", params));

would be a breaking change

mtt-artis avatar May 28 '23 15:05 mtt-artis

You think it's ok to design it like that ? The user should be able to delete key in this way imo:

setSearchParams(prev => {
  delete prev["something"];
  console.log(prev)
  return prev
});

rattleSSnake avatar May 28 '23 15:05 rattleSSnake

I don't really like the delete keyword in general so this doesn't bother me but that's a personal preference and I'm not a core member ;)

I think Ryan spoke about this pattern for store in one of his streams. I'll add the link if I find it.

mtt-artis avatar May 28 '23 15:05 mtt-artis

Thank you for your contribution @mtt-artis. I believe you didn't include this feature that could be a breaking change.

: untrack(() => mergeSearchString("", params));

would be a breaking change

setSearchParams should behave like state, where a plain object would overwrite existing params.

setSearchParams({ lonelykey: "i-overwrite" });

Imo if we don't want to change this there is no point to add callback

rattleSSnake avatar May 30 '23 20:05 rattleSSnake

Hey @rattleSSnake I share the logic here but I don't think the breaking change is a good idea. Noneless I'd be happy to make the change and refactor mergeSearchString if the reviewer asks me to do so.

mtt-artis avatar May 31 '23 01:05 mtt-artis

With the update, can we somehow reset search params (by deleting every previous key I guess) ?

What happens if I do this: setSearchParams(prev => ({ omitprev: "what" }));

rattleSSnake avatar May 31 '23 06:05 rattleSSnake

This will reset search params

setSearchParams(prev => ({ }));

mtt-artis avatar May 31 '23 06:05 mtt-artis