Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/router-replace-option.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
22 changes: 20 additions & 2 deletions packages/ui/src/common/__tests__/withRedirect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,25 @@ describe('withRedirect', () => {

render(<WithHOC />, { wrapper });

expect(fixtures.router.navigate).toHaveBeenCalledWith('/');
expect(fixtures.router.navigate).toHaveBeenCalledWith('/', undefined);
});

it('forwards the navigate options to the redirect', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withUser({});
});

const WithHOC = withRedirect(
() => <></>,
() => true,
() => '/',
undefined,
{ replace: true },
);

render(<WithHOC />, { wrapper });

expect(fixtures.router.navigate).toHaveBeenCalledWith('/', { replace: true });
});

it('does no redirects to the redirect url provided when the condition is not met', async () => {
Expand All @@ -39,6 +57,6 @@ describe('withRedirect', () => {

render(<WithHOC />, { wrapper });

expect(fixtures.router.navigate).not.toHaveBeenCalledWith('/');
expect(fixtures.router.navigate).not.toHaveBeenCalled();
});
});
3 changes: 2 additions & 1 deletion packages/ui/src/common/withRedirect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export function withRedirect<P extends AvailableComponentProps>(
condition: ComponentGuard,
redirectUrl: RedirectUrl,
warning?: string,
navigateOptions?: { replace?: boolean },
): (props: P) => null | JSX.Element {
const displayName = Component.displayName || Component.name || 'Component';
Component.displayName = displayName;
Expand All @@ -36,7 +37,7 @@ export function withRedirect<P extends AvailableComponentProps>(
}
// TODO: Fix this properly
// eslint-disable-next-line @typescript-eslint/no-floating-promises
navigate(redirectUrl({ clerk, environment, options }));
navigate(redirectUrl({ clerk, environment, options }), navigateOptions);
}
}, []);

Expand Down
6 changes: 3 additions & 3 deletions packages/ui/src/router/BaseRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export const BaseRouter = ({
useHistoryChangeObserver(refreshEvents, observerRefresh);

// TODO: Look into the real possible types of globalNavigate
const baseNavigate = async (toURL: URL | undefined): Promise<unknown> => {
const baseNavigate = async (toURL: URL | undefined, { replace }: { replace?: boolean } = {}): Promise<unknown> => {
if (!toURL) {
return;
}
Expand All @@ -243,7 +243,7 @@ export const BaseRouter = ({
if (isOutsideOfUIComponent || isCrossOrigin) {
isNavigatingRef.current = true;
try {
return await clerkNavigate(toURL.href);
return await clerkNavigate(toURL.href, { replace });
} finally {
isNavigatingRef.current = false;
}
Expand All @@ -263,7 +263,7 @@ export const BaseRouter = ({
}
isNavigatingRef.current = true;
try {
const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } });
const internalNavRes = await internalNavigate(toURL, { replace, metadata: { navigationType: 'internal' } });
// We need to flushSync to guarantee the re-render happens before handing things back to the caller,
// otherwise setActive might emit, and children re-render with the old navigation state.
// An alternative solution here could be to return a deferred promise, set that to state together
Expand Down
10 changes: 8 additions & 2 deletions packages/ui/src/router/HashRouter.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { hasUrlInFragment, stripOrigin } from '@clerk/shared/internal/clerk-js/url';
import type { NavigateOptions } from '@clerk/shared/types';
import React from 'react';

import type { RefreshEvent } from './BaseRouter';
Expand All @@ -14,11 +15,16 @@ interface HashRouterProps {
}

export const HashRouter = ({ preservedParams, children }: HashRouterProps): JSX.Element => {
const internalNavigate = async (toURL: URL): Promise<void> => {
const internalNavigate = async (toURL: URL, options?: NavigateOptions): Promise<void> => {
if (!toURL) {
return;
}
window.location.hash = stripOrigin(toURL).substring(1 + hashRouterBase.length);
const hash = stripOrigin(toURL).substring(1 + hashRouterBase.length);
if (options?.replace) {
window.location.replace('#' + hash);
} else {
window.location.hash = hash;
}
return Promise.resolve();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/router/Route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ export function Route(props: RouteProps): JSX.Element | null {
return newGetMatchData(path, index) ? true : false;
},
resolve: resolve,
navigate: (to: string, { searchParams } = {}) => {
navigate: (to: string, { searchParams, replace } = {}) => {
const toURL = resolve(to, { searchParams });
return router.baseNavigate(toURL);
return router.baseNavigate(toURL, { replace });
},
refresh: router.refresh,
params: paramsDict,
Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/router/RouteContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ export interface RouteContextValue {
indexPath: string;
currentPath: string;
matches: (path?: string, index?: boolean) => boolean;
baseNavigate: (toURL: URL) => Promise<unknown>;
navigate: (to: string, options?: { searchParams?: URLSearchParams }) => Promise<unknown>;
baseNavigate: (toURL: URL, options?: { replace?: boolean }) => Promise<unknown>;
navigate: (to: string, options?: { searchParams?: URLSearchParams; replace?: boolean }) => Promise<unknown>;
resolve: (to: string) => URL;
refresh: () => void;
params: { [key: string]: string };
Expand Down
26 changes: 24 additions & 2 deletions packages/ui/src/router/__tests__/HashRouter.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ vi.mock('@clerk/shared/react', () => {
};
});

const Button = ({ to, children }: React.PropsWithChildren<{ to: string }>) => {
const Button = ({ to, replace, children }: React.PropsWithChildren<{ to: string; replace?: boolean }>) => {
const router = useRouter();
return (
<button
onClick={() => {
void router.navigate(to);
void router.navigate(to, { replace });
}}
>
{children}
Expand All @@ -44,6 +44,12 @@ const Tester = () => (
<div id='index'>Index</div>
<Button to='foo'>Internal</Button>
<Button to='/external'>External</Button>
<Button
to='foo'
replace
>
Replace
</Button>
</Route>
<Route path='foo'>
<div id='bar'>Bar</div>
Expand Down Expand Up @@ -105,5 +111,21 @@ describe('HashRouter', () => {

expect(mockNavigate).toHaveBeenNthCalledWith(1, 'https://www.example.com/external');
});

it('replaces the hash instead of pushing it for internal navigation with replace', async () => {
const replace = vi.fn((to: string) => {
// @ts-ignore
window.location = new URL(to, window.location.href);
});
// @ts-ignore
window.location.replace = replace;
render(<Tester />);

const button = screen.getByRole('button', { name: /Replace/i });
await userEvent.click(button);

expect(replace).toHaveBeenCalledWith('#/foo?preserved=1');
expect(screen.queryByText('Bar')).toBeInTheDocument();
});
});
});
Loading