fix(v2): use base url to navigate to search page

[tetunori]

Motivation

To resolve #2827.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

• Linted (yarn prettier && yarn lint).
• Tested (yarn test)
  • but target file did not have any tests.
• Replacing the changed file on my test site to ensure that the defect is fixed.

[docusaurus-bot]

Deploy preview for docusaurus-2 ready!

Built with commit 22985fb

https://deploy-preview-2838--docusaurus-2.netlify.app

[yangshun]

Thanks! This fix is in the right direction, but we should just use the useBaseUrl() hook in @docusaurus/useBaseUrl

[tetunori]

@yangshun thanks for your review!
I've replaced useDocusaurusContext() with useBaseUrl().
lint/prettier/test was OK and this fix seems to work fine on my local site .
but I'm not 100% sure about how to use useBaseUrl() function so please re-review my new code. Thanks.
(Since this is my 1st PR in GitHub, I'm excited about your reaction!)

[tetunori]

I found a side effect and it might be caused by useBaseUrl().
Other than top page, search result page URL was wrong.
For example, we are in the page <url>/<baseUrl>/docs/intro and search there, destination is expected as <url>/<baseUrl>/search?q=<word> but actual was <url>/<baseUrl>/docs/search?q=<word>. Please confirm useBaseUrl() behavior.

[lex111]

@tetunori due to the Rules of hooks, in this case it will not be possible to use useBaseUrl hook. Therefore, your original solution was correct.

[yangshun]

You should use it as such -

const newPath = useBaseUrl(`search?q=${searchValue}`);

[lex111]

Don’t call Hooks inside loops, conditions, or nested functions

Would that work? I do not think so. Therefore siteConfig.baseUrl is a good option.

[yangshun]

This is a hook, why would it not work? How would useHistory() and useLocation() work then?

[lex111]

Try it yourself and you will see that it does not work.

    navigateToSearchPage: (searchValue) => { 
      const searchUrl = useBaseUrl(`search?q=${searchValue}`); // Not work - call hook in nested functions.
      history.push(searchUrl);
    },

[yangshun]

If we put it at the top level of the function scope, alongside useLocation() and useHistory(), and not in the navigateToSearchPage() function, would that not work? That is my suggestion.

[lex111]

Nope, this way also will not work.

[tetunori]

Try it yourself and you will see that it does not work.

    navigateToSearchPage: (searchValue) => { 
      const searchUrl = useBaseUrl(`search?q=${searchValue}`); // Not work - call hook in nested functions.
      history.push(searchUrl);
    },

I confirmed this did not work because of the error "Invalid hook call".

[tetunori]

If we put it at the top level of the function scope, alongside useLocation() and useHistory(), and not in the navigateToSearchPage() function, would that not work? That is my suggestion.

Thanks, but in the top level of the function scope, we cannot get searchValue defined in navigateToSearchPage(). So your suggestion would not work, I think.

[yangshun]

You're right, I failed to realize we need searchValue. Hmm I guess this is fine then. Mind leaving a comment in the code and why we're doing this? Could add a link to this PR in the comment to give more context :)

[tetunori]

@yangshun @lex111 Thanks for your reviews, guys.
I will add the comment including this PR URL.

[tetunori]

Added link to this PR and did following tests.

• Linted (yarn prettier && yarn lint).
• Tested (yarn test)
  • but target file did not have any tests.
• Replacing the changed file on my test site to ensure that the defect is fixed.

[lex111]

@tetunori thank you!