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
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions e2e/docs/.vuepress/theme/client/layouts/NotFound.vue
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<template>
<div class="e2e-theme-not-found">404 Not Found</div>
<div class="e2e-theme-not-found-content"><Content /></div>
</template>
2 changes: 2 additions & 0 deletions e2e/docs/404.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
routeMeta:
foo: bar
---

## NotFound H2
2 changes: 2 additions & 0 deletions e2e/docs/README.md
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
foo

## Home H2
1 change: 1 addition & 0 deletions e2e/docs/router/navigate-by-link.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- TODO -->
26 changes: 26 additions & 0 deletions e2e/docs/router/navigate-by-router.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<button id="home-with-query" @click="goHomeWithQuery">Home</button>
<button id="home-with-query-and-hash" @click="goHomeWithQueryAndHash">Home</button>
<button id="not-found-with-hash" @click="go404WithHash">404</button>
<button id="not-found-with-hash-and-query" @click="go404WithHashAndQuery">404</button>

<script setup lang="ts">
import { useRouter } from 'vuepress/client';

const router = useRouter();

const goHomeWithQuery = () => {
router.push('/?home=true');
}

const goHomeWithQueryAndHash = () => {
router.push('/?home=true#home');
}

const go404WithHash = () => {
router.push('/404.html#404');
}

const go404WithHashAndQuery = () => {
router.push('/404.html#404?notFound=true');
}
</script>
16 changes: 0 additions & 16 deletions e2e/docs/router/navigation.md

This file was deleted.

12 changes: 12 additions & 0 deletions e2e/docs/router/resolve-route-query-hash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Resolve Route FullPath

## Includes Query And Hash

- Search Query: {{ JSON.stringify(resolveRoute('/?query=1')) }}
- Hash: {{ JSON.stringify(resolveRoute('/#hash')) }}
- Search Query And Hash: {{ JSON.stringify(resolveRoute('/?query=1#hash')) }}
- Permalink And Search Query: {{ JSON.stringify(resolveRoute('/routes/permalinks/ascii-non-ascii.md?query=1')) }}

<script setup>
import { resolveRoute } from 'vuepress/client'
</script>
11 changes: 11 additions & 0 deletions e2e/tests/router/navigate-by-link.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { expect, test } from '@playwright/test'
import { BASE } from '../../utils/env'

test.beforeEach(async ({ page }) => {
await page.goto('router/navigate-by-link.html')
})

test('TODO', async ({ page }) => {
// TODO
await expect(page).toHaveURL(`${BASE}router/navigate-by-link.html`)
Mister-Hope marked this conversation as resolved.
Show resolved Hide resolved
})
30 changes: 30 additions & 0 deletions e2e/tests/router/navigate-by-router.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { expect, test } from '@playwright/test'
import { BASE } from '../../utils/env'

test.beforeEach(async ({ page }) => {
await page.goto('router/navigate-by-router.html')
})

test('should preserve query', async ({ page }) => {
await page.locator('#home-with-query').click()
await expect(page).toHaveURL(`${BASE}?home=true`)
await expect(page.locator('#home-h2')).toHaveText('Home H2')
})

test('should preserve query and hash', async ({ page }) => {
await page.locator('#home-with-query-and-hash').click()
await expect(page).toHaveURL(`${BASE}?home=true#home`)
await expect(page.locator('#home-h2')).toHaveText('Home H2')
})

test('should preserve hash', async ({ page }) => {
await page.locator('#not-found-with-hash').click()
await expect(page).toHaveURL(`${BASE}404.html#404`)
await expect(page.locator('#notfound-h2')).toHaveText('NotFound H2')
})

test('should preserve hash and query', async ({ page }) => {
await page.locator('#not-found-with-hash-and-query').click()
await expect(page).toHaveURL(`${BASE}404.html#404?notFound=true`)
await expect(page.locator('#notfound-h2')).toHaveText('NotFound H2')
})
18 changes: 0 additions & 18 deletions e2e/tests/router/navigation.spec.ts

This file was deleted.

35 changes: 35 additions & 0 deletions e2e/tests/router/resolve-route-query-hash.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect, test } from '@playwright/test'

const testCases = [
{
path: '/?query=1',
notFound: false,
},
{
path: '/#hash',
notFound: false,
},
{
path: '/?query=1#hash',
notFound: false,
},
{
path: encodeURI('/永久链接-ascii-中文/?query=1'),
notFound: false,
},
]

test('should resolve routes when including both the query and hash', async ({
page,
}) => {
const listItemsLocator = await page
.locator('.e2e-theme-content #includes-query-and-hash + ul > li')
.all()

for (const [index, li] of listItemsLocator.entries()) {
const textContent = await li.textContent()
const resolvedRoute = JSON.parse(/: (\{.*\})\s*$/.exec(textContent!)![1])
expect(resolvedRoute.path).toEqual(testCases[index].path)
expect(resolvedRoute.notFound).toEqual(testCases[index].notFound)
}
})
4 changes: 2 additions & 2 deletions packages/client/src/components/RouteLink.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { computed, defineComponent, h } from 'vue'
import type { SlotsType, VNode } from 'vue'
import { useRoute, useRouter } from 'vue-router'
import { resolveRoutePath } from '../router/index.js'
import { resolveRouteFullPath } from '../router/index.js'

/**
* Forked from https:/vuejs/router/blob/941b2131e80550009e5221d4db9f366b1fea3fd5/packages/router/src/RouterLink.ts#L293
Expand Down Expand Up @@ -91,7 +91,7 @@ export const RouteLink = defineComponent({
const path = computed(() =>
props.to.startsWith('#') || props.to.startsWith('?')
? props.to
: `${__VUEPRESS_BASE__}${resolveRoutePath(props.to, route.path).substring(1)}`,
: `${__VUEPRESS_BASE__}${resolveRouteFullPath(props.to, route.path).substring(1)}`,
)

return () =>
Expand Down
1 change: 1 addition & 0 deletions packages/client/src/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export type { Router, RouteLocationNormalizedLoaded } from 'vue-router'
export { useRoute, useRouter } from 'vue-router'

export * from './resolveRoute.js'
export * from './resolveRouteFullPath.js'
export * from './resolveRoutePath.js'
23 changes: 17 additions & 6 deletions packages/client/src/router/resolveRoute.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { splitPath } from '@vuepress/shared'
import { routes } from '../internal/routes.js'
import type { Route, RouteMeta } from '../types/index.js'
import { resolveRoutePath } from './resolveRoutePath.js'
Expand All @@ -15,15 +16,25 @@ export const resolveRoute = <T extends RouteMeta = RouteMeta>(
path: string,
currentPath?: string,
): ResolvedRoute<T> => {
const routePath = resolveRoutePath(path, currentPath)
const route = routes.value[routePath] ?? {
...routes.value['/404.html'],
notFound: true,
// get only the pathname from the path
const { pathname, hashAndQueries } = splitPath(path)

// resolve the route path
const routePath = resolveRoutePath(pathname, currentPath)
const routeFullPath = routePath + hashAndQueries

// the route not found
if (!routes.value[routePath]) {
return {
...routes.value['/404.html'],
path: routeFullPath,
notFound: true,
} as ResolvedRoute<T>
}

return {
path: routePath,
...routes.value[routePath],
path: routeFullPath,
notFound: false,
...route,
} as ResolvedRoute<T>
}
13 changes: 13 additions & 0 deletions packages/client/src/router/resolveRouteFullPath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { splitPath } from '@vuepress/shared'
import { resolveRoutePath } from './resolveRoutePath.js'

/**
* Resolve route full path with given raw path
*/
export const resolveRouteFullPath = (
path: string,
currentPath?: string,
): string => {
const { pathname, hashAndQueries } = splitPath(path)
return resolveRoutePath(pathname, currentPath) + hashAndQueries
}
33 changes: 21 additions & 12 deletions packages/client/src/router/resolveRoutePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,30 @@ import { redirects, routes } from '../internal/routes.js'
* Resolve route path with given raw path
*/
export const resolveRoutePath = (
path: string,
pathname: string,
currentPath?: string,
): string => {
// normalized path
const normalizedPath = normalizeRoutePath(path, currentPath)
if (routes.value[normalizedPath]) return normalizedPath
const normalizedRoutePath = normalizeRoutePath(pathname, currentPath)

// encoded path
const encodedPath = encodeURI(normalizedPath)
if (routes.value[encodedPath]) return encodedPath
// check if the normalized path is in routes
if (routes.value[normalizedRoutePath]) return normalizedRoutePath

// redirected path or fallback to the normalized path
return (
redirects.value[normalizedPath] ||
redirects.value[encodedPath] ||
normalizedPath
)
// check encoded path
const encodedRoutePath = encodeURI(normalizedRoutePath)

if (routes.value[encodedRoutePath]) {
return encodedRoutePath
}

// check redirected path with normalized path and encoded path
const redirectedRoutePath =
redirects.value[normalizedRoutePath] || redirects.value[encodedRoutePath]

if (redirectedRoutePath) {
return redirectedRoutePath
}

// default to normalized route path
return normalizedRoutePath
}
1 change: 1 addition & 0 deletions packages/shared/src/utils/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from './inferRoutePath'
export * from './normalizeRoutePath.js'
export * from './resolveLocalePath.js'
export * from './resolveRoutePathFromUrl.js'
export * from './splitPath.js'
17 changes: 8 additions & 9 deletions packages/shared/src/utils/routes/normalizeRoutePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@ import { inferRoutePath } from './inferRoutePath.js'
const FAKE_HOST = 'http://.'

/**
* Normalize the given path to the final route path
* Normalize the given pathname path to the final route path
*/
export const normalizeRoutePath = (path: string, current?: string): string => {
if (!path.startsWith('/') && current) {
export const normalizeRoutePath = (
pathname: string,
current?: string,
): string => {
if (!pathname.startsWith('/') && current) {
// the relative path should be resolved against the current path
const loc = current.slice(0, current.lastIndexOf('/'))

const { pathname, search, hash } = new URL(`${loc}/${path}`, FAKE_HOST)

return inferRoutePath(pathname) + search + hash
return inferRoutePath(new URL(`${loc}/${pathname}`, FAKE_HOST).pathname)
}

const [pathname, ...queryAndHash] = path.split(/(\?|#)/)

return inferRoutePath(pathname) + queryAndHash.join('')
return inferRoutePath(pathname)
}
17 changes: 17 additions & 0 deletions packages/shared/src/utils/routes/splitPath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const SPLIT_CHAR_REGEXP = /(#|\?)/

/**
* Split a path into pathname and hashAndQueries
*/
export const splitPath = (
path: string,
): {
pathname: string
hashAndQueries: string
} => {
const [pathname, ...hashAndQueries] = path.split(SPLIT_CHAR_REGEXP)
return {
pathname,
hashAndQueries: hashAndQueries.join(''),
}
}
39 changes: 0 additions & 39 deletions packages/shared/tests/routes/normalizeRoutePath.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,42 +204,3 @@ describe('should normalize clean paths correctly', () => {
}),
)
})

describe('should normalize paths with query correctly', () => {
Mister-Hope marked this conversation as resolved.
Show resolved Hide resolved
testCases
.map(([[path, current], expected]) => [
[`${path}?foo=bar`, current],
`${expected}?foo=bar`,
])
.forEach(([[path, current], expected]) =>
it(`${current ? `"${current}"-` : ''}"${path}" -> "${expected}"`, () => {
expect(normalizeRoutePath(path, current)).toBe(expected)
}),
)
})

describe('should normalize paths with hash correctly', () => {
testCases
.map(([[path, current], expected]) => [
[`${path}#foobar`, current],
`${expected}#foobar`,
])
.map(([[path, current], expected]) =>
it(`${current ? `"${current}"-` : ''}"${path}" -> "${expected}"`, () => {
expect(normalizeRoutePath(path, current)).toBe(expected)
}),
)
})

describe('should normalize paths with query and hash correctly', () => {
testCases
.map(([[path, current], expected]) => [
[`${path}?foo=1&bar=2#foobar`, current],
`${expected}?foo=1&bar=2#foobar`,
])
.map(([[path, current], expected]) =>
it(`${current ? `"${current}"-` : ''}"${path}" -> "${expected}"`, () => {
expect(normalizeRoutePath(path, current)).toBe(expected)
}),
)
})
Loading
Loading