Improve `navigate()`

[lgklsv]

Description

navigate("") Expected behaviour - remove all search params (It was working like that in previous versions)

Error: [[email protected]][Bug] You stumbled upon a Vike bug. 
Go to https://github.com/vikejs/vike/issues/new?template=bug.yml and copy-paste this error. 
A maintainer will fix the bug (usually within 24 hours). 
Debug info (for Vike maintainers; you can ignore this): {"src":3,"urlResolved":""}
    at createErrorWithCleanStackTrace (createErrorWithClean….js?v=c7c30877:4:17)
    at assert (assert.js?v=c7c30877:55:27)
    at assertUrlResolved (getPageContextUrlCom…js?v=c7c30877:40:40)
    at getUrlParsed (getPageContextUrlCom….js?v=c7c30877:63:9)
    at Object.urlPathnameGetter [as urlPathname] (getPageContextUrlCom…js?v=c7c30877:80:26)
    at assertPageContextUrl (getPageContextUrlCom…s?v=c7c30877:133:31)
    at route (index.js?v=c7c30877:18:5)
    at renderPageNominal (renderPageClientSide…31&v=c7c30877:93:46)
    at async renderPageClientSide (renderPageClientSide…331&v=c7c30877:48:5)
    at async navigate (navigate.js?t=173996…331&v=c7c30877:21:5)
await in navigate	

Environment

• Vike version: 0.4.223
• Node.js version: v20.12.2
• Operating System: macOS

[brillout]

Does it work with 0.4.223-commit-6f064ad?

[brillout]

Let's re-open if it doesn't.

[lgklsv]

Let's re-open if it doesn't.

It partially works, logical url changes as expected, urlParsed.search does not have any params after navigate("") as expected, but visible url in url bar does not change

[brillout]

navigate("") Expected behaviour - remove all search params

I kinda like it, but it isn't explicitly part of the API contract (yet?). Anything that makes you feel like it should be?

[lgklsv]

navigate("") Expected behaviour - remove all search params

I kinda like it, but it isn't explicitly part of the API contract (yet?). Anything that makes you feel like it should be?

What do you mean ? Something like that

navigate('', {
     pageContext: {
         searchParams: new URLSearchParams(),
     },
});

But still this does not change url in url bar. I was using navigate("") because it was working in previous versions and kinda intuitive. Btw, I really like react-router useSearchParams API, I recreated it in my vike project. If you like this idea, it would be nice to add it as a part of vike-react

[brillout]

How about this?

navigate({
  search: {
    someNewQuery: 'param',
    removedQuery: null
  }
})

navigate({
  searchAll: {}, // remove all previous queries
  search: {
    someNewQuery: 'param',
  }
})

// Full API
navigate({
  pathname: '/some-path',
  hash: 'some-hash',
  searchAll: { filter: ['cars', 'computers']] }, // ?filter=cars&filter=computers
  search: {
    someExtraQuery: 'param',
  }
})
// Backwards compatible
navigate('/hello?world=1')

I'm hesitating about searchAll: {} for removing previous queries because it isn't explicit. But the pro is that it aligns with the pageContext.urlParsed API.

WDYT? Would you prefer this or still prefer react-router's useSearchParams()?

[brillout]

Or maybe:

// URL current: /products?filter=cars
// URL next: /products?someNewQuery=param
navigate({
  // overwrite
  searchAll: "",
  // mutate
  search: {
    someNewQuery: 'param',
  }
})

// URL current: /products?filter=cars
// URL next: /products?filter=cars&filter=computers
navigate({
  // overwrite
  searchAll: "filter=cars&filter=computers"
})

[brillout]

Also:

// URL current: /products?filter=cars#price
// URL next: /products#price
navigate('?')

// URL current: /products?filter=cars#price
// URL next: /products?filter=cars
navigate('#')

Feel free to ignore my designing thinking, but definitely let me know if you think there is a neat thing about useSearchParams() I don't have on the radar.

[lgklsv]

navigate('?') works, and it is how I fixed it right away

Here is how my useSearchParams looks like, it is just a simple hook that combines const { urlParsed } = usePageContext(); to get search params and navigate to set them. I mean I really like URLSearchParams API and the idea of using search params like useState hook

import { NavigateOptions, useNavigate } from './useNavigate';
import { useCallback } from 'react';
import { usePageContext } from 'vike-react/usePageContext';

export const useSearchParams = (): [
  URLSearchParams,
  (params: string[][] | Record<string, string> | string | URLSearchParams) => void,
] => {
  const { urlParsed } = usePageContext();
  const navigate = useNavigate();
  const searchParams = new URLSearchParams(urlParsed.search);

  const setSearchParams = useCallback(
    (
      params: string[][] | Record<string, string> | string | URLSearchParams,
      options?: NavigateOptions,
    ) => {
      const newParams = new URLSearchParams(params);
      navigate(`?${newParams.toString()}`, options);
    },
    [navigate],
  );

  return [searchParams, setSearchParams];
};

[brillout]

I see, I like it. It could be nice to ship it with vike-react 🤔

Note that your implementation erases the hash, which may or may not be what you want.

-     navigate(`?${newParams.toString()}`, options);
+     // Preserves hash
+     navigate(`?${newParams.toString()}${urlParsed.hashOriginal ?? ''}`, options);

Also, wouldn't the following be simpler? Isn't useNavigate() useless compared to just directly using navigate()?

- import { NavigateOptions, useNavigate } from './useNavigate';
+ import { navigate } from 'vike/client/router';
  import { useCallback } from 'react';
  import { usePageContext } from 'vike-react/usePageContext';

  export const useSearchParams = (): [
    URLSearchParams,
    (params: string[][] | Record<string, string> | string | URLSearchParams) => void,
  ] => {
    const { urlParsed } = usePageContext();
-   const navigate = useNavigate();
    const searchParams = new URLSearchParams(urlParsed.search);

    const setSearchParams = useCallback(
      (
        params: string[][] | Record<string, string> | string | URLSearchParams,
        options?: NavigateOptions,
      ) => {
        const newParams = new URLSearchParams(params);
        navigate(`?${newParams.toString()}`, options);
      },
-     [navigate],
+     [],
    );

    return [searchParams, setSearchParams];
  };

[lgklsv]

Agree, it is better to keep hash in general useSearchParams , I missed it because I don't use them in my project. useNavigate() is also specific to my project, I add locale to url there, but you are right in this case it doesn't matter.

[brillout]

Related:

• https://github.com/vikejs/vike/issues/2253
• https://github.com/vikejs/vike/pull/2240