Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
35 changes: 20 additions & 15 deletions packages/react-router/src/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,29 @@ export function useLinkProps<
[router, _options],
)

// 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 hrefOptionPublicHref = next.maskedLocation
? next.maskedLocation.publicHref
: next.publicHref
const hrefOptionExternal = next.maskedLocation
? next.maskedLocation.external
: next.external
const hrefOption = React.useMemo(() => {
if (disabled) {
return undefined
if (disabled) return undefined

// Full URL means rewrite changed the origin - treat as external-like
if (hrefOptionExternal) {
return { href: hrefOptionPublicHref, external: true }
}
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
}

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

const externalLink = React.useMemo(() => {
if (hrefOption?.external) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/tests/redirect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ describe('redirect', () => {
expect(currentRedirect.headers.get('Location')).toEqual('/about')
expect(currentRedirect.options).toEqual({
_fromLocation: {
external: false,
hash: '',
href: '/',
publicHref: '/',
Expand All @@ -368,7 +369,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
5 changes: 2 additions & 3 deletions packages/router-core/src/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@ export interface ParsedLocation<TSearchObj extends AnySchema = {}> {
publicHref: string
/**
* @private
* @description The full URL of the location.
* @private
* @description Whether the publicHref is external (different origin from rewrite).
*/
url: URL
external: boolean
/**
* @internal
* @description Match snapshot for fast-path on back/forward navigation.
Expand Down
65 changes: 38 additions & 27 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,8 @@ export class RouterCore<
return {
href: fullPath,
publicHref: href,
url: url,
pathname: decodePath(url.pathname),
external: !!this.rewrite && url.origin !== this.origin,
searchStr,
search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any,
hash: decodePath(url.hash.split('#').reverse()[0] ?? ''),
Expand Down Expand Up @@ -1883,25 +1883,41 @@ 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
let href: string
let publicHref: string
let external = false

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
external = true
} 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,
state: nextState as any,
hash: hash ?? '',
external,
unmaskOnReload: dest.unmaskOnReload,
_matchSnapshot: matchSnapshot,
}
Expand Down Expand Up @@ -2017,9 +2033,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 +2216,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 +2293,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 +2629,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
32 changes: 15 additions & 17 deletions packages/solid-router/src/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,25 +134,23 @@ export function useLinkProps<
})

const hrefOption = Solid.createMemo(() => {
if (_options().disabled) {
return undefined
if (_options().disabled) return undefined
// 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
const external = location.external

if (external) {
return { href: publicHref, external: true }
}
let href
const maskedLocation = next().maskedLocation
if (maskedLocation) {
href = maskedLocation.url.href
} else {
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
}

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

const externalLink = Solid.createMemo(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/solid-router/tests/redirect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,8 @@ describe('redirect', () => {
expect(currentRedirect.headers.get('Location')).toEqual('/about')
expect(currentRedirect.options).toEqual({
_fromLocation: {
external: false,
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
Loading
Loading