Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
33 changes: 20 additions & 13 deletions packages/react-router/src/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,27 @@ export function useLinkProps<
if (disabled) {
return undefined
}
let href = next.maskedLocation
? next.maskedLocation.url.href
: next.url.href

let external = false
if (router.origin) {
if (href.startsWith(router.origin)) {
href = router.history.createHref(href.replace(router.origin, '')) || '/'
} else {
external = true
}
// Use publicHref - it contains the correct href for display
// When a rewrite changes the origin, publicHref is the full URL
// Otherwise it's the origin-stripped path
// This avoids constructing URL objects in the hot path
const publicHref = next.maskedLocation
? next.maskedLocation.publicHref
: next.publicHref

// Check if publicHref is a full URL (different origin from rewrite)
const isFullUrl =
publicHref.startsWith('http://') || publicHref.startsWith('https://')
if (isFullUrl) {
// Full URL means rewrite changed the origin - treat as external-like
return { href: publicHref, external: false }
}

return {
href: router.history.createHref(publicHref) || '/',
external: false,
}
return { href, external }
}, [disabled, next.maskedLocation, next.url, router.origin, router.history])
}, [disabled, next.maskedLocation, next.publicHref, router.history])

const externalLink = React.useMemo(() => {
if (hrefOption?.external) {
Expand Down
1 change: 0 additions & 1 deletion packages/react-router/tests/redirect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ describe('redirect', () => {
__TSR_key: currentRedirect.options._fromLocation!.state.__TSR_key,
key: currentRedirect.options._fromLocation!.state.key,
},
url: new URL('http://localhost/'),
},
href: '/about',
to: '/about',
Expand Down
31 changes: 13 additions & 18 deletions packages/react-router/tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ describe('encoding: path params', () => {

await act(() => router.load())

expect(router.state.location.url.href.endsWith('/posts/tanner')).toBe(true)
expect(router.state.location.href.endsWith('/posts/tanner')).toBe(true)
expect(router.state.location.href).toBe('/posts/tanner')
expect(router.state.location.pathname).toBe('/posts/tanner')
})
Expand All @@ -362,7 +362,7 @@ describe('encoding: path params', () => {

await act(() => router.load())

expect(router.state.location.url.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
expect(router.state.location.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
true,
)
expect(router.state.location.href).toBe('/posts/%F0%9F%9A%80')
Expand All @@ -383,9 +383,7 @@ describe('encoding: path params', () => {
}),
)

expect(router.state.location.url.href.endsWith('/posts/100%2525')).toBe(
true,
)
expect(router.state.location.href.endsWith('/posts/100%2525')).toBe(true)
expect(router.state.location.href).toBe('/posts/100%2525')
expect(router.state.location.pathname).toBe('/posts/100%2525')
})
Expand All @@ -410,9 +408,7 @@ describe('encoding: path params', () => {
)

expect(
router.state.location.url.href.endsWith(
`/posts/${encodedValue}jane%25`,
),
router.state.location.href.endsWith(`/posts/${encodedValue}jane%25`),
).toBe(true)
expect(router.state.location.href).toBe(`/posts/${encodedValue}jane%25`)
expect(router.state.location.pathname).toBe(
Expand Down Expand Up @@ -447,7 +443,7 @@ describe('encoding: path params', () => {
)

expect(
router.state.location.url.href.endsWith(`/posts/${character}jane%25`),
router.state.location.href.endsWith(`/posts/${character}jane%25`),
).toBe(true)
expect(router.state.location.href).toBe(`/posts/${character}jane%25`)
expect(router.state.location.pathname).toBe(`/posts/${character}jane%25`)
Expand All @@ -461,7 +457,7 @@ describe('encoding: path params', () => {

await act(() => router.load())

expect(router.state.location.url.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
expect(router.state.location.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
true,
)
expect(router.state.location.href).toBe('/posts/%F0%9F%9A%80')
Expand All @@ -480,7 +476,7 @@ describe('encoding: path params', () => {
await act(() => router.load())

expect(
router.state.location.url.href.endsWith(
router.state.location.href.endsWith(
'/posts/framework%2Freact%2Fguide%2Ffile-based-routing%20tanstack',
),
).toBe(true)
Expand Down Expand Up @@ -627,7 +623,7 @@ describe('encoding/decoding: wildcard routes/params', () => {

await router.load()

expect(router.state.location.url.href.endsWith('/tanner')).toBe(true)
expect(router.state.location.href.endsWith('/tanner')).toBe(true)
expect(router.state.location.href).toBe('/tanner')
expect(router.state.location.pathname).toBe('/tanner')
})
Expand All @@ -639,7 +635,7 @@ describe('encoding/decoding: wildcard routes/params', () => {

await router.load()

expect(router.state.location.url.href.endsWith('/%F0%9F%9A%80')).toBe(true)
expect(router.state.location.href.endsWith('/%F0%9F%9A%80')).toBe(true)
expect(router.state.location.href).toBe('/%F0%9F%9A%80')
expect(router.state.location.pathname).toBe('/🚀')
})
Expand All @@ -657,7 +653,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
await router.load()

expect(
router.state.location.url.href.endsWith(`/100${encodedValue}100`),
router.state.location.href.endsWith(`/100${encodedValue}100`),
).toBe(true)
expect(router.state.location.href).toBe(`/100${encodedValue}100`)
expect(router.state.location.pathname).toBe(`/100${encodedValue}100`)
Expand All @@ -672,7 +668,7 @@ describe('encoding/decoding: wildcard routes/params', () => {

await router.load()

expect(router.state.location.url.href.endsWith('/%F0%9F%9A%80')).toBe(true)
expect(router.state.location.href.endsWith('/%F0%9F%9A%80')).toBe(true)
expect(router.state.location.href).toBe('/%F0%9F%9A%80')
expect(router.state.location.pathname).toBe('/🚀')
})
Expand All @@ -689,7 +685,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
await router.load()

expect(
router.state.location.url.href.endsWith(
router.state.location.href.endsWith(
'/framework%2Freact%2Fguide%2Ffile-based-routing%20tanstack',
),
).toBe(true)
Expand All @@ -711,7 +707,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
await router.load()

expect(
router.state.location.url.href.endsWith(
router.state.location.href.endsWith(
'/framework/react/guide/file-based-routing%20tanstack',
),
).toBe(true)
Expand Down Expand Up @@ -863,7 +859,6 @@ describe('encoding/decoding: URL path segment', () => {

expect(router.state.location.pathname).toBe(path)
expect(router.state.location.href).toBe(url)
expect(new URL(router.state.location.url).pathname).toBe(url)
})
})

Expand Down
6 changes: 0 additions & 6 deletions packages/router-core/src/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,6 @@ export interface ParsedLocation<TSearchObj extends AnySchema = {}> {
* If a rewrite is applied, the `href` property will be the rewritten URL.
*/
publicHref: string
/**
* @private
* @description The full URL of the location.
* @private
*/
url: URL
/**
* @internal
* @description Match snapshot for fast-path on back/forward navigation.
Expand Down
62 changes: 35 additions & 27 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,6 @@ export class RouterCore<
return {
href: fullPath,
publicHref: href,
url: url,
pathname: decodePath(url.pathname),
searchStr,
search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any,
Expand Down Expand Up @@ -1883,20 +1882,34 @@ export class RouterCore<
// Create the full path of the location
const fullPath = `${nextPathname}${searchStr}${hashStr}`

// Create the new href with full origin
const url = new URL(fullPath, this.origin)

// If a rewrite function is provided, use it to rewrite the URL
const rewrittenUrl = executeRewriteOutput(this.rewrite, url)

// Use encoded URL path for href (consistent with parseLocation)
const encodedHref = url.href.replace(url.origin, '')
// Compute href and publicHref without URL construction when no rewrite
// URL is only constructed lazily when .url is accessed (for tests/edge cases)
let href: string
let publicHref: string

if (this.rewrite) {
// With rewrite, we need to construct URL to apply the rewrite
const url = new URL(fullPath, this.origin)
const rewrittenUrl = executeRewriteOutput(this.rewrite, url)
href = url.href.replace(url.origin, '')
// If rewrite changed the origin, publicHref needs full URL
// Otherwise just use the path components
if (rewrittenUrl.origin !== this.origin) {
publicHref = rewrittenUrl.href
} else {
publicHref =
rewrittenUrl.pathname + rewrittenUrl.search + rewrittenUrl.hash
}
} else {
// Fast path: no rewrite, skip URL construction entirely
// fullPath is already the correct href (origin-stripped)
href = fullPath
publicHref = fullPath
}

return {
publicHref:
rewrittenUrl.pathname + rewrittenUrl.search + rewrittenUrl.hash,
href: encodedHref,
url: rewrittenUrl,
publicHref,
href,
pathname: nextPathname,
search: nextSearch,
searchStr,
Expand Down Expand Up @@ -2017,9 +2030,6 @@ export class RouterCore<
maskedLocation,
// eslint-disable-next-line prefer-const
hashScrollIntoView,
// don't pass url into history since it is a URL instance that cannot be serialized
// eslint-disable-next-line prefer-const
url: _url,
...nextHistory
} = next

Expand Down Expand Up @@ -2203,8 +2213,9 @@ export class RouterCore<
// be a complete path (possibly with basepath)
if (to !== undefined || !href) {
const location = this.buildLocation({ to, ...rest } as any)
href = href ?? location.url.href
publicHref = publicHref ?? location.url.href
// Use publicHref which contains the path (origin-stripped is fine for reload)
href = href ?? location.publicHref
publicHref = publicHref ?? location.publicHref
}

// Use publicHref when available and href is not a full URL,
Expand Down Expand Up @@ -2279,10 +2290,9 @@ export class RouterCore<
_includeValidateSearch: true,
})

if (
this.latestLocation.publicHref !== nextLocation.publicHref ||
nextLocation.url.origin !== this.origin
) {
// Check if location changed - origin check is unnecessary since buildLocation
// always uses this.origin when constructing URLs
if (this.latestLocation.publicHref !== nextLocation.publicHref) {
const href = this.getParsedLocationHref(nextLocation)

throw redirect({ href })
Expand Down Expand Up @@ -2616,11 +2626,9 @@ export class RouterCore<
}

getParsedLocationHref = (location: ParsedLocation) => {
let href = location.url.href
if (this.origin && location.url.origin === this.origin) {
href = href.replace(this.origin, '') || '/'
}
return href
// For redirects and external use, we need publicHref (with rewrite output applied)
// href is the internal path after rewrite input, publicHref is user-facing
return location.publicHref || '/'
}

resolveRedirect = (redirect: AnyRedirect): AnyRedirect => {
Expand Down
31 changes: 17 additions & 14 deletions packages/solid-router/src/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,25 @@ export function useLinkProps<
if (_options().disabled) {
return undefined
}
let href
const maskedLocation = next().maskedLocation
if (maskedLocation) {
href = maskedLocation.url.href
} else {
href = next().url.href
// Use publicHref - it contains the correct href for display
// When a rewrite changes the origin, publicHref is the full URL
// Otherwise it's the origin-stripped path
// This avoids constructing URL objects in the hot path
const location = next().maskedLocation ?? next()
const publicHref = location.publicHref

// Check if publicHref is a full URL (different origin from rewrite)
const isFullUrl =
publicHref.startsWith('http://') || publicHref.startsWith('https://')
if (isFullUrl) {
// Full URL means rewrite changed the origin - treat as external-like
return { href: publicHref, external: false }
}
let external = false
if (router.origin) {
if (href.startsWith(router.origin)) {
href = router.history.createHref(href.replace(router.origin, ''))
} else {
external = true
}

return {
href: router.history.createHref(publicHref) || '/',
external: false,
}
return { href, external }
})

const externalLink = Solid.createMemo(() => {
Expand Down
1 change: 0 additions & 1 deletion packages/solid-router/tests/redirect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,6 @@ describe('redirect', () => {
expect(currentRedirect.options).toEqual({
_fromLocation: {
publicHref: '/',
url: new URL('http://localhost/'),
hash: '',
href: '/',
pathname: '/',
Expand Down
2 changes: 1 addition & 1 deletion packages/solid-router/tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ describe('encoding: URL path segment', () => {
await router.load()

expect(router.state.location.pathname).toBe(path)
expect(new URL(router.state.location.url).pathname).toBe(url)
expect(router.state.location.href).toBe(url)
})
})

Expand Down
29 changes: 16 additions & 13 deletions packages/vue-router/src/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -471,23 +471,26 @@ export function useLinkProps<
return undefined
}
const nextLocation = next.value
const maskedLocation = nextLocation?.maskedLocation

let hrefValue: string
if (maskedLocation) {
hrefValue = maskedLocation.url.href
} else {
hrefValue = nextLocation?.url.href
const location = nextLocation?.maskedLocation ?? nextLocation

// Use publicHref - it contains the correct href for display
// When a rewrite changes the origin, publicHref is the full URL
// Otherwise it's the origin-stripped path
// This avoids constructing URL objects in the hot path
const publicHref = location?.publicHref
if (!publicHref) {
return undefined
}

// Handle origin stripping like Solid does
if (router.origin && hrefValue?.startsWith(router.origin)) {
hrefValue = router.history.createHref(
hrefValue.replace(router.origin, ''),
)
// Check if publicHref is a full URL (different origin from rewrite)
const isFullUrl =
publicHref.startsWith('http://') || publicHref.startsWith('https://')
if (isFullUrl) {
// Full URL means rewrite changed the origin
return publicHref
}

return hrefValue
return router.history.createHref(publicHref) || '/'
})

// Create static event handlers that don't change between renders
Expand Down
Loading
Loading