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

fix: fix route resolving error with hash and queries (close #1561) #1562

Merged
merged 21 commits into from
May 27, 2024

Conversation

pengzhanbo
Copy link
Member

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Provide a description in this PR that addresses what the PR is solving. If this PR is going to solve an existing issue, please reference the issue (e.g. close #123).

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Documentation update
  • Other

Description

fix: #1561

@coveralls
Copy link

coveralls commented May 19, 2024

Pull Request Test Coverage Report for Build 9248171287

Details

  • 8 of 25 (32.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 41.031%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/client/src/router/resolveRouteFullPath.ts 0 3 0.0%
packages/client/src/router/resolveRoute.ts 0 5 0.0%
packages/client/src/router/resolveRoutePath.ts 0 9 0.0%
Totals Coverage Status
Change from base Build 9211217292: -0.1%
Covered Lines: 690
Relevant Lines: 1721

💛 - Coveralls

Mister-Hope
Mister-Hope previously approved these changes May 19, 2024
@digital-codes
Copy link

digital-codes commented May 19, 2024

changing fullPath to path in client/dist/app.js like so fixes the issue with query strings
...
router.beforeResolve(async (to, from) => {
if (to.path !== from.path || from === START_LOCATION) {
const route = resolveRoute(to.path);
...

Not sure what's the exact difference and why it worked up to rc8 (at least) and broke with rc11
Also not sure about side effects ...

you have
const fullPath = to.fullPath.split('#')[0]

Will fix hash but not query parms, right?

@pengzhanbo
Copy link
Member Author

changing fullPath to path in client/dist/app.js like so fixes the issue with query strings ... router.beforeResolve(async (to, from) => { if (to.path !== from.path || from === START_LOCATION) { const route = resolveRoute(to.path); ...

Not sure what's the exact difference and why it worked up to rc8 (at least) and broke with rc11 Also not sure about side effects ...

you have const fullPath = to.fullPath.split('#')[0]

Will fix hash but not query parms, right?

The bug was caused by the fact that vuepress internally needs to resolve the route to obtain the route instance through pathname, but route.fullPath contains pathname, query, hash, and other content, leading to parsing errors and failure to match the route instance.

You can refer to the changes made in this PR submission for more information.

@Mister-Hope
Copy link
Member

I updated the pr to improve the original fix performance.

Also now API are unified to avoid misleading.

  • All parameters which is expected with no hash and queries are renamed as pathname

  • The above function are named with "Route Path`

  • Any parameters which may have hash and queries are called path

  • The above functions are named with "Route Full Path"

@Mister-Hope Mister-Hope changed the title fix: route resolve error causing 404 #1561 fix: route resolve error with hash and queries, close #1561 May 20, 2024
Mister-Hope
Mister-Hope previously approved these changes May 20, 2024
@meteorlxy meteorlxy changed the title fix: route resolve error with hash and queries, close #1561 fix: fix route resolving error with hash and queries (close #1561) May 23, 2024
@meteorlxy meteorlxy merged commit 4df59d4 into vuepress:main May 27, 2024
30 checks passed
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

Successfully merging this pull request may close these issues.

[Bug report] When the page path includes hash/ query, the page will load a 404 error.
5 participants