diff --git a/packages/solid-router/src/link.tsx b/packages/solid-router/src/link.tsx index 0b2d101d40..dbc0208ff4 100644 --- a/packages/solid-router/src/link.tsx +++ b/packages/solid-router/src/link.tsx @@ -131,63 +131,111 @@ export function useLinkProps< { equals: (prev, next) => prev.href === next.href }, ) - const next = Solid.createMemo(() => { - // Rebuild when inherited search/hash or the current route context changes. - const _fromLocation = currentLocation() - const nextOptions = { _fromLocation, ...options } as any - // untrack because router-core will also access stores, which are signals in solid - return Solid.untrack(() => router.buildLocation(nextOptions)) - }) + type LinkState = readonly [ + href: string | undefined, + external: string | undefined, + active: boolean, + ] + + let nextLocation: ReturnType + + const linkState = Solid.createMemo( + (previous: LinkState | undefined): LinkState => { + const current = currentLocation() + // Rebuild when inherited search/hash or the current route context changes. + const nextOptions = { _fromLocation: current, ...options } as any + // untrack because router-core will also access stores, which are signals in solid + nextLocation = Solid.untrack(() => router.buildLocation(nextOptions)) + const location = nextLocation.maskedLocation ?? nextLocation + const publicHref = location.publicHref + const href = options.disabled + ? undefined + : location.external + ? publicHref + : router.history.createHref(publicHref) || '/' + + let external: string | undefined + if (href !== undefined && location.external) { + // Block dangerous protocols for external links + if (isDangerousProtocol(href, router.protocolAllowlist)) { + if (process.env.NODE_ENV !== 'production') { + console.warn(`Blocked Link with dangerous protocol: ${href}`) + } + } else { + external = href + } + } else { + const to = options.to + if ( + !isSafeInternal(to) && + typeof to === 'string' && + to.indexOf(':') !== -1 + ) { + try { + new URL(to) + // Block dangerous protocols like javascript:, blob:, data: + if (isDangerousProtocol(to, router.protocolAllowlist)) { + if (process.env.NODE_ENV !== 'production') { + console.warn(`Blocked Link with dangerous protocol: ${to}`) + } + } else { + external = to + } + } catch {} + } + } - const hrefOption = Solid.createMemo(() => { - 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 active = false + if (external === undefined) { + const activeOptions = local.activeOptions + if (activeOptions?.exact) { + active = exactPathTest( + current.pathname, + nextLocation.pathname, + router.basepath, + ) + } else { + const currentPath = removeTrailingSlash( + current.pathname, + router.basepath, + ) + const nextPath = removeTrailingSlash( + nextLocation.pathname, + router.basepath, + ) + active = + currentPath.startsWith(nextPath) && + (currentPath.length === nextPath.length || + currentPath[nextPath.length] === '/') + } - return { - href: router.history.createHref(publicHref) || '/', - external: false, - } - }) + if (active && (activeOptions?.includeSearch ?? true)) { + active = deepEqual(current.search, nextLocation.search, { + partial: !activeOptions?.exact, + ignoreUndefined: !activeOptions?.explicitUndefined, + }) + } - const externalLink = Solid.createMemo(() => { - const _href = hrefOption() - if (_href?.external) { - // Block dangerous protocols for external links - if (isDangerousProtocol(_href.href, router.protocolAllowlist)) { - if (process.env.NODE_ENV !== 'production') { - console.warn(`Blocked Link with dangerous protocol: ${_href.href}`) + if (active && activeOptions?.includeHash) { + const currentHash = + shouldHydrateHash && !hasHydrated() ? '' : current.hash + active = currentHash === nextLocation.hash } - return undefined } - return _href.href - } - const to = options.to - const safeInternal = isSafeInternal(to) - if (safeInternal) return undefined - if (typeof to !== 'string' || to.indexOf(':') === -1) return undefined - try { - new URL(to as any) - // Block dangerous protocols like javascript:, blob:, data: - if (isDangerousProtocol(to, router.protocolAllowlist)) { - if (process.env.NODE_ENV !== 'production') { - console.warn(`Blocked Link with dangerous protocol: ${to}`) - } - return undefined + + if ( + previous && + previous[0] === href && + previous[1] === external && + previous[2] === active + ) { + return previous } - return to - } catch {} - return undefined - }) + return [href, external, active] + }, + ) + + const externalLink = () => linkState()[1] const preload = Solid.createMemo(() => { if (options.reloadDocument || externalLink()) { @@ -198,62 +246,16 @@ export function useLinkProps< const preloadDelay = () => local.preloadDelay ?? router.options.defaultPreloadDelay ?? 0 - const isActive = Solid.createMemo(() => { - if (externalLink()) return false - const activeOptions = local.activeOptions - const current = currentLocation() - const nextLocation = next() - - if (activeOptions?.exact) { - const testExact = exactPathTest( - current.pathname, - nextLocation.pathname, - router.basepath, - ) - if (!testExact) { - return false - } - } else { - const currentPath = removeTrailingSlash(current.pathname, router.basepath) - const nextPath = removeTrailingSlash( - nextLocation.pathname, - router.basepath, - ) - - const pathIsFuzzyEqual = - currentPath.startsWith(nextPath) && - (currentPath.length === nextPath.length || - currentPath[nextPath.length] === '/') - if (!pathIsFuzzyEqual) { - return false - } - } - - if (activeOptions?.includeSearch ?? true) { - const searchTest = deepEqual(current.search, nextLocation.search, { - partial: !activeOptions?.exact, - ignoreUndefined: !activeOptions?.explicitUndefined, - }) - if (!searchTest) { - return false - } - } - - if (activeOptions?.includeHash) { - const currentHash = - shouldHydrateHash && !hasHydrated() ? '' : current.hash - return currentHash === nextLocation.hash - } - return true - }) - - const doPreload = () => - router - .preloadRoute({ ...options, _builtLocation: next() } as any) + const doPreload = () => { + // Refresh the privately held location even when published link state is stable. + linkState() + return router + .preloadRoute({ ...options, _builtLocation: nextLocation } as any) .catch((err: any) => { console.warn(err) console.warn(preloadWarning) }) + } const preloadViewportIoCallback = ( entry: IntersectionObserverEntry | undefined, @@ -419,10 +421,11 @@ export function useLinkProps< } const resolvedProps = Solid.createMemo(() => { - const active = isActive() + const state = linkState() + const active = state[2] const base = { - href: hrefOption()?.href, + href: state[0], ref: mergeRefs(setRef, options.ref), onClick, onBlur, diff --git a/packages/solid-router/tests/link.test.tsx b/packages/solid-router/tests/link.test.tsx index 7642030465..e9bf131301 100644 --- a/packages/solid-router/tests/link.test.tsx +++ b/packages/solid-router/tests/link.test.tsx @@ -613,6 +613,164 @@ describe('Link', () => { }) }) + test('does not republish link props when destination state is unchanged', async () => { + const activeProps = vi.fn(() => ({ class: 'active' })) + const inactiveProps = vi.fn(() => ({ class: 'inactive' })) + const rootRoute = createRootRoute({ + component: () => ( + <> + + Target + + + + ), + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>

Index

, + }) + const otherRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/other', + component: () =>

Other

, + }) + const targetRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: () =>

Target route

, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, otherRoute, targetRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + render(() => ) + + const link = await screen.findByTestId('stable-link') + const initialInactiveCalls = inactiveProps.mock.calls.length + expect(initialInactiveCalls).toBeGreaterThan(0) + + await router.navigate({ to: '/other' }) + expect(await screen.findByText('Other')).toBeInTheDocument() + expect(inactiveProps).toHaveBeenCalledTimes(initialInactiveCalls) + + await router.navigate({ to: '/target' }) + expect(await screen.findByText('Target route')).toBeInTheDocument() + expect(link).toHaveClass('active') + expect(activeProps).toHaveBeenCalledTimes(1) + + await router.navigate({ to: '/other' }) + expect(await screen.findByText('Other')).toBeInTheDocument() + expect(inactiveProps).toHaveBeenCalledTimes(initialInactiveCalls + 1) + }) + + test('does not republish props for an active fuzzy link between descendants', async () => { + const activeProps = vi.fn(() => ({ class: 'active' })) + const rootRoute = createRootRoute({ + component: () => ( + <> + + Posts + + + + ), + }) + const postRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/posts/$postId', + component: () =>

Post

, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([postRoute]), + history: createMemoryHistory({ initialEntries: ['/posts/one'] }), + }) + + render(() => ) + + const link = await screen.findByTestId('stable-active-link') + const initialActiveCalls = activeProps.mock.calls.length + expect(link).toHaveClass('active') + expect(initialActiveCalls).toBeGreaterThan(0) + + await router.navigate({ to: '/posts/$postId', params: { postId: 'two' } }) + expect(router.state.location.pathname).toBe('/posts/two') + expect(link).toHaveClass('active') + expect(activeProps).toHaveBeenCalledTimes(initialActiveCalls) + }) + + test('preloads the latest built location when published link state is unchanged', async () => { + const rootRoute = createRootRoute({ + component: () => ( + <> + + Target + + + + ), + }) + const firstRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/first', + component: () =>

First

, + }) + const secondRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/second', + component: () =>

Second

, + }) + const targetRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/target', + component: () =>

Target

, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([ + firstRoute, + secondRoute, + targetRoute, + ]), + history: createMemoryHistory({ initialEntries: ['/first'] }), + }) + const preloadRouteSpy = vi.spyOn(router, 'preloadRoute') + + render(() => ) + + const link = await screen.findByTestId('stable-preload-link') + fireEvent.focus(link) + await waitFor(() => expect(preloadRouteSpy).toHaveBeenCalledTimes(1)) + const firstBuiltLocation = (preloadRouteSpy.mock.calls[0]![0] as any) + ._builtLocation + + await router.navigate({ to: '/second' }) + expect(await screen.findByText('Second')).toBeInTheDocument() + + fireEvent.mouseOver(link) + await waitFor(() => expect(preloadRouteSpy).toHaveBeenCalledTimes(2)) + const secondBuiltLocation = (preloadRouteSpy.mock.calls[1]![0] as any) + ._builtLocation + + expect(secondBuiltLocation).not.toBe(firstBuiltLocation) + expect(secondBuiltLocation.href).toBe(firstBuiltLocation.href) + }) + test('updates exact and fuzzy active state before the next route renders', async () => { const postLoader = createControlledPromise()