From 1601872a8547a3a44bf22e090e500d52eea2f296 Mon Sep 17 00:00:00 2001
From: Jack Palmer <jackpalmer@spotify.com>
Date: Tue, 20 Dec 2022 10:09:11 +0000
Subject: [PATCH] fix: Limit route resolve to only when location pathname
 changes

Signed-off-by: Jack Palmer <jackpalmer@spotify.com>
---
 .../src/routing/useRouteRef.test.tsx          | 107 +++++++++++++++++-
 .../src/routing/useRouteRef.tsx               |   6 +-
 2 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/packages/core-plugin-api/src/routing/useRouteRef.test.tsx b/packages/core-plugin-api/src/routing/useRouteRef.test.tsx
index 62f6d6da42..618f40d281 100644
--- a/packages/core-plugin-api/src/routing/useRouteRef.test.tsx
+++ b/packages/core-plugin-api/src/routing/useRouteRef.test.tsx
@@ -16,10 +16,11 @@
 
 import { renderHook } from '@testing-library/react-hooks';
 import React from 'react';
-import { MemoryRouter } from 'react-router-dom';
+import { MemoryRouter, Router } from 'react-router-dom';
 import { createVersionedContextForTesting } from '@backstage/version-bridge';
 import { useRouteRef } from './useRouteRef';
 import { createRouteRef } from './RouteRef';
+import { createBrowserHistory } from 'history';
 
 describe('v1 consumer', () => {
   const context = createVersionedContextForTesting('routing-context');
@@ -49,4 +50,108 @@ describe('v1 consumer', () => {
       }),
     );
   });
+
+  it('re-resolves the routeFunc when the search parameters change', () => {
+    const resolve = jest.fn(() => () => '/hello');
+    context.set({ 1: { resolve } });
+
+    const routeRef = createRouteRef({ id: 'ref1' });
+    const history = createBrowserHistory();
+    history.push('/my-page');
+
+    const { rerender } = renderHook(() => useRouteRef(routeRef), {
+      wrapper: ({ children }) => (
+        <Router
+          location={history.location}
+          navigator={history}
+          children={children}
+        />
+      ),
+    });
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+
+    history.push('/my-new-page');
+    rerender();
+
+    expect(resolve).toHaveBeenCalledTimes(2);
+  });
+
+  it('does not re-resolve the routeFunc the location pathname does not change', () => {
+    const resolve = jest.fn(() => () => '/hello');
+    context.set({ 1: { resolve } });
+
+    const routeRef = createRouteRef({ id: 'ref1' });
+    const history = createBrowserHistory();
+    history.push('/my-page');
+
+    const { rerender } = renderHook(() => useRouteRef(routeRef), {
+      wrapper: ({ children }) => (
+        <Router
+          location={history.location}
+          navigator={history}
+          children={children}
+        />
+      ),
+    });
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+
+    history.push('/my-page');
+    rerender();
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not re-resolve the routeFunc when the search parameter changes', () => {
+    const resolve = jest.fn(() => () => '/hello');
+    context.set({ 1: { resolve } });
+
+    const routeRef = createRouteRef({ id: 'ref1' });
+    const history = createBrowserHistory();
+    history.push('/my-page');
+
+    const { rerender } = renderHook(() => useRouteRef(routeRef), {
+      wrapper: ({ children }) => (
+        <Router
+          location={history.location}
+          navigator={history}
+          children={children}
+        />
+      ),
+    });
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+
+    history.push('/my-page?foo=bar');
+    rerender();
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not re-resolve the routeFunc when the hash parameter changes', () => {
+    const resolve = jest.fn(() => () => '/hello');
+    context.set({ 1: { resolve } });
+
+    const routeRef = createRouteRef({ id: 'ref1' });
+    const history = createBrowserHistory();
+    history.push('/my-page');
+
+    const { rerender } = renderHook(() => useRouteRef(routeRef), {
+      wrapper: ({ children }) => (
+        <Router
+          location={history.location}
+          navigator={history}
+          children={children}
+        />
+      ),
+    });
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+
+    history.push('/my-page#foo');
+    rerender();
+
+    expect(resolve).toHaveBeenCalledTimes(1);
+  });
 });
diff --git a/packages/core-plugin-api/src/routing/useRouteRef.tsx b/packages/core-plugin-api/src/routing/useRouteRef.tsx
index 563bb2035c..cf292b110f 100644
--- a/packages/core-plugin-api/src/routing/useRouteRef.tsx
+++ b/packages/core-plugin-api/src/routing/useRouteRef.tsx
@@ -85,7 +85,7 @@ export function useRouteRef<Params extends AnyParams>(
     | SubRouteRef<Params>
     | ExternalRouteRef<Params, any>,
 ): RouteFunc<Params> | undefined {
-  const sourceLocation = useLocation();
+  const { pathname } = useLocation();
   const versionedContext = useVersionedContext<{ 1: RouteResolver }>(
     'routing-context',
   );
@@ -95,8 +95,8 @@ export function useRouteRef<Params extends AnyParams>(
 
   const resolver = versionedContext.atVersion(1);
   const routeFunc = useMemo(
-    () => resolver && resolver.resolve(routeRef, sourceLocation),
-    [resolver, routeRef, sourceLocation],
+    () => resolver && resolver.resolve(routeRef, { pathname }),
+    [resolver, routeRef, pathname],
   );
 
   if (!versionedContext) {