Skip to content

Commit 49c8178

Browse files
authored
Merge pull request #2381 from redpanda-data/jc/fix-mf-router-recreation-loop
fix(federation): prevent router recreation loop in MF mode
2 parents b49b906 + 266ca00 commit 49c8178

4 files changed

Lines changed: 175 additions & 6 deletions

File tree

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/**
2+
* Copyright 2026 Redpanda Data, Inc.
3+
*
4+
* Use of this software is governed by the Business Source License
5+
* included in the file https://github.com/redpanda-data/redpanda/blob/dev/licenses/bsl.md
6+
*
7+
* As of the Change Date specified in that file, in accordance with
8+
* the Business Source License, use of this software will be governed
9+
* by the Apache License, Version 2.0
10+
*/
11+
12+
import { render } from '@testing-library/react';
13+
14+
const mockNavigate = vi.fn();
15+
const mockLocation = { pathname: '/topics', searchStr: '' };
16+
const mockRouter = {};
17+
18+
vi.mock('@tanstack/react-router', () => ({
19+
useNavigate: () => mockNavigate,
20+
useLocation: () => mockLocation,
21+
useRouter: () => mockRouter,
22+
}));
23+
24+
vi.mock('../../config', () => ({
25+
config: {
26+
jwt: undefined as string | undefined,
27+
clusterId: undefined as string | undefined,
28+
},
29+
isEmbedded: vi.fn(() => false),
30+
}));
31+
32+
vi.mock('../../hubspot/hubspot.helper', () => ({
33+
trackHubspotPage: vi.fn(),
34+
}));
35+
36+
vi.mock('../../state/app-global', () => ({
37+
appGlobal: {
38+
setNavigate: vi.fn(),
39+
setRouter: vi.fn(),
40+
setLocation: vi.fn(),
41+
},
42+
}));
43+
44+
vi.mock('../../state/backend-api', () => ({
45+
api: {
46+
errors: [],
47+
},
48+
}));
49+
50+
import { RouterSync } from './router-sync';
51+
import { config, isEmbedded } from '../../config';
52+
53+
describe('RouterSync', () => {
54+
beforeEach(() => {
55+
vi.clearAllMocks();
56+
config.jwt = undefined;
57+
config.clusterId = undefined;
58+
mockLocation.pathname = '/topics';
59+
});
60+
61+
test('does not dispatch [console] navigated events when not embedded', () => {
62+
const dispatchSpy = vi.spyOn(window, 'dispatchEvent');
63+
vi.mocked(isEmbedded).mockReturnValue(false);
64+
65+
render(<RouterSync />);
66+
67+
const consoleEvents = dispatchSpy.mock.calls.filter(
68+
([event]) => event instanceof CustomEvent && event.type === '[console] navigated'
69+
);
70+
expect(consoleEvents).toHaveLength(0);
71+
72+
dispatchSpy.mockRestore();
73+
});
74+
75+
test('does not dispatch [console] navigated events in federated mode (clusterId set)', () => {
76+
const dispatchSpy = vi.spyOn(window, 'dispatchEvent');
77+
vi.mocked(isEmbedded).mockReturnValue(true);
78+
config.clusterId = 'test-cluster-123';
79+
mockLocation.pathname = '/schema-registry';
80+
81+
render(<RouterSync />);
82+
83+
const consoleEvents = dispatchSpy.mock.calls.filter(
84+
([event]) => event instanceof CustomEvent && event.type === '[console] navigated'
85+
);
86+
expect(consoleEvents).toHaveLength(0);
87+
88+
dispatchSpy.mockRestore();
89+
});
90+
91+
test('dispatches [console] navigated events in embedded non-federated mode', () => {
92+
const dispatchSpy = vi.spyOn(window, 'dispatchEvent');
93+
vi.mocked(isEmbedded).mockReturnValue(true);
94+
config.clusterId = undefined;
95+
mockLocation.pathname = '/schema-registry';
96+
97+
render(<RouterSync />);
98+
99+
const consoleEvents = dispatchSpy.mock.calls.filter(
100+
([event]) => event instanceof CustomEvent && event.type === '[console] navigated'
101+
);
102+
expect(consoleEvents).toHaveLength(1);
103+
expect((consoleEvents[0][0] as CustomEvent).detail).toBe('/schema-registry');
104+
105+
dispatchSpy.mockRestore();
106+
});
107+
});

frontend/src/components/misc/router-sync.tsx

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import { useLocation, useNavigate, useRouter } from '@tanstack/react-router';
1313
import { useEffect, useRef } from 'react';
1414

15-
import { isEmbedded } from '../../config';
15+
import { config as appConfig, isEmbedded } from '../../config';
1616
import { trackHubspotPage } from '../../hubspot/hubspot.helper';
1717
import { appGlobal } from '../../state/app-global';
1818
import { api } from '../../state/backend-api';
@@ -54,10 +54,13 @@ export const RouterSync = () => {
5454
appGlobal.setLocation(location);
5555
}, [location]);
5656

57-
// Notify shell (Cloud UI) when Console navigates internally
58-
// This enables bidirectional sync between TanStack Router and React Router DOM
57+
// Notify shell (Cloud UI) when Console navigates internally.
58+
// Skip in MFv2 federated mode — console-app.tsx handles navigation sync
59+
// via the onRouteChange callback. Dispatching events here too creates a
60+
// feedback loop: Console dispatches → cloud-ui navigates → Console sees
61+
// new path → dispatches again.
5962
useEffect(() => {
60-
if (isEmbedded() && location.pathname && previousPathRef.current !== location.pathname) {
63+
if (isEmbedded() && !isFederatedMode() && location.pathname && previousPathRef.current !== location.pathname) {
6164
// Dispatch event for shell to sync its router state
6265
window.dispatchEvent(new CustomEvent('[console] navigated', { detail: location.pathname }));
6366
previousPathRef.current = location.pathname;
@@ -66,3 +69,13 @@ export const RouterSync = () => {
6669

6770
return null;
6871
};
72+
73+
/**
74+
* Detect MFv2 federated mode. In federated mode, console-app.tsx sets up its own
75+
* navigation sync via onRouteChange — the legacy window event dispatch is not needed.
76+
*/
77+
function isFederatedMode(): boolean {
78+
// config.clusterId is only set in federated mode (via setup() in console-app.tsx).
79+
// In standalone mode, it's undefined.
80+
return appConfig.clusterId !== undefined && appConfig.clusterId !== '';
81+
}

frontend/src/federation/console-app.test.tsx

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,49 @@ describe('ConsoleApp', () => {
262262
});
263263
});
264264

265+
describe('Router Stability', () => {
266+
test('does not recreate router when initialPath prop changes', async () => {
267+
const { createRouter, createMemoryHistory } = await import('@tanstack/react-router');
268+
269+
const { rerender } = render(<ConsoleApp {...defaultProps} initialPath="/topics" />);
270+
271+
await waitFor(() => {
272+
expect(mockGetAccessToken).toHaveBeenCalled();
273+
});
274+
275+
const createRouterCallCount = vi.mocked(createRouter).mock.calls.length;
276+
const createMemoryHistoryCallCount = vi.mocked(createMemoryHistory).mock.calls.length;
277+
278+
// Rerender with a different initialPath (simulates cloud-ui navigation)
279+
rerender(<ConsoleApp {...defaultProps} initialPath="/groups" />);
280+
281+
// Router should NOT have been recreated
282+
expect(vi.mocked(createRouter).mock.calls.length).toBe(createRouterCallCount);
283+
expect(vi.mocked(createMemoryHistory).mock.calls.length).toBe(createMemoryHistoryCallCount);
284+
});
285+
286+
test('uses first initialPath for memory history, ignores subsequent changes', async () => {
287+
const { createMemoryHistory } = await import('@tanstack/react-router');
288+
289+
const { rerender } = render(<ConsoleApp {...defaultProps} initialPath="/topics" />);
290+
291+
await waitFor(() => {
292+
expect(mockGetAccessToken).toHaveBeenCalled();
293+
});
294+
295+
// Verify initial path was used
296+
expect(vi.mocked(createMemoryHistory)).toHaveBeenCalledWith(
297+
expect.objectContaining({ initialEntries: ['/topics'] })
298+
);
299+
300+
const callCount = vi.mocked(createMemoryHistory).mock.calls.length;
301+
302+
// Changing initialPath should not create a new memory history
303+
rerender(<ConsoleApp {...defaultProps} initialPath="/schema-registry" />);
304+
expect(vi.mocked(createMemoryHistory).mock.calls.length).toBe(callCount);
305+
});
306+
});
307+
265308
describe('Search Params Handling', () => {
266309
test('onRouteChange includes searchStr when route resolves with search params', async () => {
267310
// Get a reference to the subscribe callback so we can invoke it manually

frontend/src/federation/console-app.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,16 @@ function ConsoleAppInner({
234234
[configOverrides?.urlOverride?.grpc, tokenRefreshInterceptor]
235235
);
236236

237+
// Capture initialPath on first render only — subsequent navigation is handled
238+
// by the navigateTo prop via router.navigate(). Including initialPath in the
239+
// useMemo deps would recreate the entire router on every host navigation,
240+
// remounting all route components and retriggering all data fetches.
241+
const initialPathRef = useRef(initialPath);
242+
237243
// Create memory history router (host controls browser URL)
238244
const router = useMemo(() => {
239245
const memoryHistory = createMemoryHistory({
240-
initialEntries: [initialPath],
246+
initialEntries: [initialPathRef.current],
241247
});
242248

243249
const r = createRouter({
@@ -252,7 +258,7 @@ function ConsoleAppInner({
252258
});
253259

254260
return r;
255-
}, [initialPath, queryClient, dataplaneTransport]);
261+
}, [queryClient, dataplaneTransport]);
256262

257263
// Subscribe to route changes and notify host (with loop prevention)
258264
useEffect(() => {

0 commit comments

Comments
 (0)