Skip to content

Commit e053fa3

Browse files
committed
fix(router-core): preserve parsed location URL access under sharing
1 parent fd76e8d commit e053fa3

6 files changed

Lines changed: 76 additions & 43 deletions

File tree

docs/router/api/router/ParsedLocationType.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ interface ParsedLocation {
1515
hash: string
1616
maskedLocation?: ParsedLocation
1717
unmaskOnReload?: boolean
18-
url: URL
18+
getUrl: () => URL
1919
origin: string
2020
}
2121
```
2222

2323
> [!NOTE]
24-
> The `url` property of a `ParsedLocation` is a getter, and the `URL` may be computed
25-
> on demand. In hot loops, relying on this property may have a negative performance impact.
24+
> `getUrl()` returns a memoized `URL` that is created on demand. In hot loops,
25+
> repeatedly calling this method may have a negative performance impact.

packages/router-core/src/location.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ export interface ParsedLocation<TSearchObj extends AnySchema = {}> {
5353
*/
5454
origin: string
5555
/**
56-
* A `URL` object representation of the location. This object is created lazily,
57-
* so reading this property is not free.
56+
* Returns a memoized `URL` object representation of the location.
57+
* The `URL` is created lazily, so calling this method is not free.
5858
*/
59-
url: URL
59+
getUrl: () => URL
6060
}

packages/router-core/src/router.ts

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,7 +1278,6 @@ export class RouterCore<
12781278
const searchStr = this.options.stringifySearch(parsedSearch)
12791279

12801280
return initializeParsedLocation({
1281-
__proto__: parsedLocationPrototype,
12821281
href: pathname + searchStr + hash,
12831282
publicHref: href,
12841283
pathname: decodePath(pathname).path,
@@ -1291,7 +1290,7 @@ export class RouterCore<
12911290
) as any,
12921291
hash: decodePath(hash.slice(1)).path,
12931292
state: replaceEqualDeep(previousLocation?.state, state),
1294-
} as ParsedLocationWithoutUrl<FullSearchSchema<TRouteTree>>)
1293+
})
12951294
}
12961295

12971296
// Before we do any processing, we need to allow rewrites to modify the URL
@@ -1309,7 +1308,6 @@ export class RouterCore<
13091308
const fullPath = url.href.replace(url.origin, '')
13101309

13111310
return initializeParsedLocation({
1312-
__proto__: parsedLocationPrototype,
13131311
href: fullPath,
13141312
publicHref: href,
13151313
pathname: decodePath(url.pathname).path,
@@ -2005,7 +2003,6 @@ export class RouterCore<
20052003

20062004
return initializeParsedLocation(
20072005
{
2008-
__proto__: parsedLocationPrototype,
20092006
publicHref,
20102007
href,
20112008
pathname: nextPathname,
@@ -2958,36 +2955,33 @@ function comparePaths(a: string, b: string) {
29582955
return normalize(a) === normalize(b)
29592956
}
29602957

2961-
type ParsedLocationWithoutUrl<TSearchObj extends AnySchema = {}> = Omit<
2958+
type ParsedLocationWithoutGetUrl<TSearchObj extends AnySchema = {}> = Omit<
29622959
ParsedLocation<TSearchObj>,
2963-
'url'
2964-
> & { __proto__: typeof parsedLocationPrototype }
2960+
'getUrl'
2961+
>
29652962

29662963
const parsedLocationUrls = new WeakMap<ParsedLocation<any>, URL>()
29672964

2968-
function getParsedLocationUrl(location: ParsedLocation<any>) {
2969-
let url = parsedLocationUrls.get(location)
2965+
function getParsedLocationUrl(this: ParsedLocation<any>) {
2966+
let url = parsedLocationUrls.get(this)
29702967

29712968
if (!url) {
2972-
url = new URL(location.publicHref, location.origin)
2973-
parsedLocationUrls.set(location, url)
2969+
url = new URL(this.publicHref, this.origin)
2970+
parsedLocationUrls.set(this, url)
29742971
}
29752972

29762973
return url
29772974
}
29782975

2979-
const parsedLocationPrototype = {
2980-
get url() {
2981-
return getParsedLocationUrl(this as ParsedLocation<any>)
2982-
},
2983-
}
2984-
29852976
function initializeParsedLocation<TSearchObj extends AnySchema = {}>(
2986-
location: ParsedLocationWithoutUrl<TSearchObj>,
2977+
location: ParsedLocationWithoutGetUrl<TSearchObj>,
29872978
url?: URL | null,
29882979
): ParsedLocation<TSearchObj> {
2989-
if (url) parsedLocationUrls.set(location as any, url)
2990-
return location as any
2980+
const parsedLocation = location as ParsedLocation<TSearchObj>
2981+
parsedLocation.getUrl =
2982+
getParsedLocationUrl as ParsedLocation<TSearchObj>['getUrl']
2983+
if (url) parsedLocationUrls.set(parsedLocation as any, url)
2984+
return parsedLocation
29912985
}
29922986

29932987
/**

packages/router-core/tests/build-location.test.ts

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test, vi } from 'vitest'
22
import { createMemoryHistory } from '@tanstack/history'
33
import { BaseRootRoute, BaseRoute } from '../src'
4+
import { replaceEqualDeep } from '../src/utils'
45
import { createTestRouter } from './routerTestUtils'
56

67
describe('buildLocation - params function receives parsed params', () => {
@@ -1207,6 +1208,7 @@ describe('buildLocation - location output structure', () => {
12071208
// Verify all expected properties exist
12081209
expect(location).toEqual({
12091210
external: false,
1211+
getUrl: expect.any(Function),
12101212
hash: 'section',
12111213
href: '/posts?page=1#section',
12121214
origin: 'http://localhost:3000',
@@ -1248,7 +1250,7 @@ describe('buildLocation - location output structure', () => {
12481250
expect(location.href).toBe('/posts')
12491251
})
12501252

1251-
test('location should expose a memoized lazy url', async () => {
1253+
test('location should expose a memoized lazy getUrl method', async () => {
12521254
const rootRoute = new BaseRootRoute({})
12531255
const postsRoute = new BaseRoute({
12541256
getParentRoute: () => rootRoute,
@@ -1270,14 +1272,14 @@ describe('buildLocation - location output structure', () => {
12701272
hash: 'section',
12711273
})
12721274

1273-
expect(Object.keys(location)).not.toContain('url')
1274-
expect(location.url).toBe(location.url)
1275-
expect(location.url.pathname).toBe('/posts')
1276-
expect(location.url.search).toBe('?page=1')
1277-
expect(location.url.hash).toBe('#section')
1275+
expect(location.getUrl).toBeDefined()
1276+
expect(location.getUrl()).toBe(location.getUrl())
1277+
expect(location.getUrl().pathname).toBe('/posts')
1278+
expect(location.getUrl().search).toBe('?page=1')
1279+
expect(location.getUrl().hash).toBe('#section')
12781280
})
12791281

1280-
test('router state location should expose url after parsing history', async () => {
1282+
test('router state location should expose getUrl after parsing history', async () => {
12811283
const rootRoute = new BaseRootRoute({})
12821284
const postsRoute = new BaseRoute({
12831285
getParentRoute: () => rootRoute,
@@ -1295,9 +1297,44 @@ describe('buildLocation - location output structure', () => {
12951297

12961298
await router.load()
12971299

1298-
expect(router.state.location.url.pathname).toBe('/posts')
1299-
expect(router.state.location.url.search).toBe('?page=1')
1300-
expect(router.state.location.url.hash).toBe('#section')
1300+
expect(router.state.location.getUrl().pathname).toBe('/posts')
1301+
expect(router.state.location.getUrl().search).toBe('?page=1')
1302+
expect(router.state.location.getUrl().hash).toBe('#section')
1303+
})
1304+
1305+
test('replaceEqualDeep should preserve getUrl on structurally shared locations', async () => {
1306+
const rootRoute = new BaseRootRoute({})
1307+
const postsRoute = new BaseRoute({
1308+
getParentRoute: () => rootRoute,
1309+
path: '/posts',
1310+
})
1311+
1312+
const routeTree = rootRoute.addChildren([postsRoute])
1313+
1314+
const router = createTestRouter({
1315+
routeTree,
1316+
history: createMemoryHistory({
1317+
initialEntries: ['/posts?page=1#section'],
1318+
}),
1319+
})
1320+
1321+
await router.load()
1322+
1323+
const prev = router.state.location
1324+
const next = router.buildLocation({
1325+
to: '/posts',
1326+
search: { page: 2 },
1327+
hash: 'section',
1328+
})
1329+
1330+
const shared = replaceEqualDeep(prev, next)
1331+
1332+
expect(shared).not.toBe(prev)
1333+
expect(shared.getUrl).toBe(prev.getUrl)
1334+
expect(shared.search).not.toBe(prev.search)
1335+
expect(shared.getUrl().pathname).toBe('/posts')
1336+
expect(shared.getUrl().search).toBe('?page=2')
1337+
expect(shared.getUrl().hash).toBe('#section')
13011338
})
13021339
})
13031340

packages/router-core/tests/mask.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ describe('buildLocation - route masks', () => {
9090
expect(location.maskedLocation).toBeDefined()
9191
expect(location.maskedLocation!.pathname).toBe('/photos/123')
9292
expect(location.pathname).toBe('/photos/123/modal')
93-
expect(location.maskedLocation!.url.pathname).toBe('/photos/123')
94-
expect(location.url.pathname).toBe('/photos/123/modal')
93+
expect(location.maskedLocation!.getUrl().pathname).toBe('/photos/123')
94+
expect(location.getUrl().pathname).toBe('/photos/123/modal')
9595
})
9696

9797
test('should set params to {} when maskParams is false', () => {

packages/router-core/tests/rewrite.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,12 @@ describe('rewrite origin behavior', () => {
5353
expect(router.state.location.pathname).toBe('/about')
5454
expect(router.state.location.href).toBe('/about?lang=en#team')
5555
expect(router.state.location.publicHref).toBe('/docs/about?lang=en#team')
56-
expect(router.state.location.url.href).toBe(
56+
expect(router.state.location.getUrl().href).toBe(
5757
'https://public.example.com/docs/about?lang=en#team',
5858
)
59-
expect(router.state.location.url.origin).toBe('https://public.example.com')
59+
expect(router.state.location.getUrl().origin).toBe(
60+
'https://public.example.com',
61+
)
6062
})
6163

6264
test('buildLocation exposes the current origin to output rewrites', async () => {
@@ -91,10 +93,10 @@ describe('rewrite origin behavior', () => {
9193
'https://public.example.com/docs/about?lang=en#team',
9294
)
9395
expect(location.external).toBe(true)
94-
expect(location.url.href).toBe(
96+
expect(location.getUrl().href).toBe(
9597
'https://public.example.com/docs/about?lang=en#team',
9698
)
97-
expect(location.url.origin).toBe('https://public.example.com')
99+
expect(location.getUrl().origin).toBe('https://public.example.com')
98100
})
99101

100102
test('buildAndCommitLocation uses origin-aware rewrites when href is provided', async () => {
@@ -126,7 +128,7 @@ describe('rewrite origin behavior', () => {
126128
expect(router.state.location.pathname).toBe('/about')
127129
expect(router.state.location.href).toBe('/about?lang=en#team')
128130
expect(router.state.location.publicHref).toBe('/docs/about?lang=en#team')
129-
expect(router.state.location.url.href).toBe(
131+
expect(router.state.location.getUrl().href).toBe(
130132
'https://public.example.com/docs/about?lang=en#team',
131133
)
132134
})

0 commit comments

Comments
 (0)