refactor based on coderabbit recommendations

Author: nlynzaad

packages/react-router/src/link.tsx

@@ -99,7 +99,13 @@ export function useLinkProps<
     structuralSharing: true as any,
   })
 
-  const { getFromPath, activeLocation } = useActiveLocation()
+  // subscribe to location here to re-build fromPath if it changes
+  const routerLocation = useRouterState({
+    select: (s) => s.location,
+    structuralSharing: true as any,
+  })
+
+  const { getFromPath } = useActiveLocation()
 
   const from = getFromPath(options.from)
 
@@ -110,7 +116,7 @@ export function useLinkProps<
     // eslint-disable-next-line react-hooks/exhaustive-deps
     [
       router,
-      activeLocation,
+      routerLocation,
       currentSearch,
       from,
       options._fromLocation,

packages/react-router/src/useActiveLocation.ts

@@ -2,35 +2,25 @@ import { last } from '@tanstack/router-core'
 import { useCallback, useEffect, useState } from 'react'
 import { useRouter } from './useRouter'
 import { useMatch } from './useMatch'
-import type { AnyRouteMatch, ParsedLocation } from '@tanstack/router-core'
+import { useRouterState } from './useRouterState'
+import type { ParsedLocation } from '@tanstack/router-core'
 
-export type UseLocationResult = {
-  activeLocationMatch: AnyRouteMatch | undefined
+export type UseActiveLocationResult = {
+  activeLocation: ParsedLocation
   getFromPath: (from?: string) => string
+  setActiveLocation: (location?: ParsedLocation) => void
 }
 
-export const useActiveLocation = (location?: ParsedLocation) => {
-  const { matchRoutes, state } = useRouter()
-  const [activeLocation, setActiveLocation] = useState<ParsedLocation>(
-    location ?? state.location,
-  )
-  const [customActiveLocation, _setCustomActiveLocation] =
-    useState<ParsedLocation>(location ?? state.location)
-  const [useCustomActiveLocation, setUseCustomActiveLocation] =
-    useState(!!location)
+export const useActiveLocation = (location?: ParsedLocation): UseActiveLocationResult => {
+  const router = useRouter()
+  const routerLocation = useRouterState({select: (state) => state.location})
+  const [activeLocation, setActiveLocation] = useState<ParsedLocation>(location ?? routerLocation)
+  const [customActiveLocation, setCustomActiveLocation] = useState<ParsedLocation | undefined>(location)
 
   useEffect(() => {
-    if (!useCustomActiveLocation) {
-      setActiveLocation(state.location)
-    } else {
-      setActiveLocation(customActiveLocation)
-    }
-  }, [state.location, useCustomActiveLocation, customActiveLocation])
-
-  const setCustomActiveLocation = (location: ParsedLocation) => {
-    _setCustomActiveLocation(location)
-    setUseCustomActiveLocation(true)
-  }
+    setActiveLocation(customActiveLocation ?? routerLocation)
+  }, [routerLocation, customActiveLocation])
+
 
   const currentRouteMatch = useMatch({
     strict: false,
@@ -39,15 +29,15 @@ export const useActiveLocation = (location?: ParsedLocation) => {
 
   const getFromPath = useCallback(
     (from?: string) => {
-      const activeLocationMatches = matchRoutes(activeLocation, {
+      const activeLocationMatches = router.matchRoutes(activeLocation, {
         _buildLocation: false,
       })
 
       const activeLocationMatch = last(activeLocationMatches)
 
       return from ?? activeLocationMatch?.fullPath ?? currentRouteMatch.fullPath
     },
-    [activeLocation, currentRouteMatch.fullPath, matchRoutes],
+    [activeLocation, currentRouteMatch.fullPath, router],
   )
 
   return {

packages/solid-router/src/useActiveLocation.ts

@@ -1,51 +1,42 @@
 import { last } from '@tanstack/router-core'
-import { createEffect, createSignal } from 'solid-js'
-import * as Solid from 'solid-js'
+import {  createEffect, createMemo, createSignal } from 'solid-js'
 import { useMatch } from './useMatch'
 import { useRouter } from './useRouter'
 import { useRouterState } from './useRouterState'
+import type {Accessor} from 'solid-js';
 import type { ParsedLocation } from '@tanstack/router-core'
 
-export function useActiveLocation(location?: ParsedLocation) {
+export type UseLocationResult = {
+  activeLocation: Accessor<ParsedLocation>
+  getFromPath: (from?: string) => Accessor<string>
+  setActiveLocation: (location?: ParsedLocation) => void
+}
+
+export function useActiveLocation(location?: ParsedLocation): UseLocationResult {
   const router = useRouter()
-  const [activeLocation, setActiveLocation] = createSignal<ParsedLocation>(
-    location ?? useRouterState({ select: (state) => state.location })(),
-  )
-  const [customActiveLocation, _setCustomActiveLocation] =
-    createSignal<ParsedLocation>(
-      location ?? useRouterState({ select: (state) => state.location })(),
-    )
-  const [useCustomActiveLocation, setUseCustomActiveLocation] =
-    createSignal(!!location)
+  // we are not using a variable here for router state location since we need to only calculate that if the location is not passed in. It can result in unnecessary history actions if we do that.
+  const [activeLocation, setActiveLocation] = createSignal<ParsedLocation>(location ?? useRouterState({select: s => s.location})())
+  const [customActiveLocation, setCustomActiveLocation] = createSignal<ParsedLocation | undefined>(location)
 
   createEffect(() => {
-    if (!useCustomActiveLocation()) {
-      setActiveLocation(useRouterState({ select: (state) => state.location }))
-    } else {
-      setActiveLocation(customActiveLocation())
-    }
+    setActiveLocation(customActiveLocation() ?? useRouterState({select: s => s.location})())
   })
 
-  const setCustomActiveLocation = (location: ParsedLocation) => {
-    _setCustomActiveLocation(location)
-    setUseCustomActiveLocation(true)
-  }
-
-  const matchIndex = useMatch({
+  const currentRouteMatch = useMatch({
     strict: false,
-    select: (match) => match.index,
+    select: (match) => match,
   })
 
   const getFromPath = (from?: string) =>
-    Solid.createMemo(() => {
-      const currentRouteMatches = router.matchRoutes(activeLocation(), {
+    createMemo(() => {
+      const currentRouteMatches = router.matchRoutes(customActiveLocation() ?? activeLocation(), {
         _buildLocation: false,
       })
 
       return (
         from ??
         last(currentRouteMatches)?.fullPath ??
-        router.state.matches[matchIndex()]!.fullPath
+        currentRouteMatch().fullPath
       )
     })