Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prefer more specific routes in usePathname when detecting the currently active pathname for localized pathnames #1152

Merged
merged 1 commit into from
Jun 26, 2024
Merged
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
4 changes: 2 additions & 2 deletions packages/next-intl/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,11 @@
},
{
"path": "dist/production/navigation.react-client.js",
"limit": "3.235 KB"
"limit": "3.355 KB"
},
{
"path": "dist/production/navigation.react-server.js",
"limit": "17.84 KB"
"limit": "17.975 KB"
},
{
"path": "dist/production/server.react-client.js",
Expand Down
62 changes: 2 additions & 60 deletions packages/next-intl/src/middleware/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '../routing/types';
import {
getLocalePrefix,
getSortedPathnames,
matchesPathname,
prefixPathname,
templateToRegex
Expand All @@ -15,65 +16,6 @@ export function getFirstPathnameSegment(pathname: string) {
return pathname.split('/')[1];
}

function isOptionalCatchAllSegment(pathname: string) {
return pathname.includes('[[...');
}

function isCatchAllSegment(pathname: string) {
return pathname.includes('[...');
}

function isDynamicSegment(pathname: string) {
return pathname.includes('[');
}

export function comparePathnamePairs(a: string, b: string): number {
const pathA = a.split('/');
const pathB = b.split('/');

const maxLength = Math.max(pathA.length, pathB.length);
for (let i = 0; i < maxLength; i++) {
const segmentA = pathA[i];
const segmentB = pathB[i];

// If one of the paths ends, prioritize the shorter path
if (!segmentA && segmentB) return -1;
if (segmentA && !segmentB) return 1;

// Prioritize static segments over dynamic segments
if (!isDynamicSegment(segmentA) && isDynamicSegment(segmentB)) return -1;
if (isDynamicSegment(segmentA) && !isDynamicSegment(segmentB)) return 1;

// Prioritize non-catch-all segments over catch-all segments
if (!isCatchAllSegment(segmentA) && isCatchAllSegment(segmentB)) return -1;
if (isCatchAllSegment(segmentA) && !isCatchAllSegment(segmentB)) return 1;

// Prioritize non-optional catch-all segments over optional catch-all segments
if (
!isOptionalCatchAllSegment(segmentA) &&
isOptionalCatchAllSegment(segmentB)
) {
return -1;
}
if (
isOptionalCatchAllSegment(segmentA) &&
!isOptionalCatchAllSegment(segmentB)
) {
return 1;
}

if (segmentA === segmentB) continue;
}

// Both pathnames are completely static
return 0;
}

export function getSortedPathnames(pathnames: Array<string>) {
const sortedPathnames = pathnames.sort(comparePathnamePairs);
return sortedPathnames;
}

export function getInternalTemplate<
AppLocales extends Locales,
AppPathnames extends Pathnames<AppLocales>
Expand Down Expand Up @@ -112,7 +54,7 @@ export function getInternalTemplate<
}

// Try to find an internal pathname that matches (this can be the case
// if all localized pathnames are different from the internal pathnames).
// if all localized pathnames are different from the internal pathnames)
for (const internalPathname of Object.keys(pathnames)) {
if (matchesPathname(internalPathname, pathname)) {
return [undefined, internalPathname];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ export default function createLocalizedPathnamesNavigation<
const locale = useTypedLocale();

// @ts-expect-error -- Mirror the behavior from Next.js, where `null` is returned when `usePathname` is used outside of Next, but the types indicate that a string is always returned.
return pathname
? getRoute({pathname, locale, pathnames: config.pathnames})
: pathname;
return pathname ? getRoute(locale, pathname, config.pathnames) : pathname;
}

function getPathname({
Expand Down
39 changes: 20 additions & 19 deletions packages/next-intl/src/navigation/shared/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {ParsedUrlQueryInput} from 'node:querystring';
import type {UrlObject} from 'url';
import {Locales, Pathnames} from '../../routing/types';
import {matchesPathname} from '../../shared/utils';
import {matchesPathname, getSortedPathnames} from '../../shared/utils';
import StrictParams from './StrictParams';

type SearchParamValue = ParsedUrlQueryInput[keyof ParsedUrlQueryInput];
Expand Down Expand Up @@ -152,28 +152,29 @@ export function compileLocalizedPathname<AppLocales extends Locales, Pathname>({
}
}

export function getRoute<AppLocales extends Locales>({
locale,
pathname,
pathnames
}: {
locale: AppLocales[number];
pathname: string;
pathnames: Pathnames<AppLocales>;
}) {
export function getRoute<AppLocales extends Locales>(
locale: AppLocales[number],
pathname: string,
pathnames: Pathnames<AppLocales>
): keyof Pathnames<AppLocales> {
const sortedPathnames = getSortedPathnames(Object.keys(pathnames));
const decoded = decodeURI(pathname);

let template = Object.entries(pathnames).find(([, routePath]) => {
const routePathname =
typeof routePath !== 'string' ? routePath[locale] : routePath;
return matchesPathname(routePathname, decoded);
})?.[0];

if (!template) {
template = pathname;
for (const internalPathname of sortedPathnames) {
const localizedPathnamesOrPathname = pathnames[internalPathname];
if (typeof localizedPathnamesOrPathname === 'string') {
const localizedPathname = localizedPathnamesOrPathname;
if (matchesPathname(localizedPathname, decoded)) {
return internalPathname;
}
} else {
if (matchesPathname(localizedPathnamesOrPathname[locale], decoded)) {
return internalPathname;
}
}
}

return template as keyof Pathnames<AppLocales>;
return pathname as keyof Pathnames<AppLocales>;
}

export function getBasePath(
Expand Down
58 changes: 58 additions & 0 deletions packages/next-intl/src/shared/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,61 @@ export function templateToRegex(template: string): RegExp {

return new RegExp(`^${regexPattern}$`);
}

function isOptionalCatchAllSegment(pathname: string) {
return pathname.includes('[[...');
}

function isCatchAllSegment(pathname: string) {
return pathname.includes('[...');
}

function isDynamicSegment(pathname: string) {
return pathname.includes('[');
}

function comparePathnamePairs(a: string, b: string): number {
const pathA = a.split('/');
const pathB = b.split('/');

const maxLength = Math.max(pathA.length, pathB.length);
for (let i = 0; i < maxLength; i++) {
const segmentA = pathA[i];
const segmentB = pathB[i];

// If one of the paths ends, prioritize the shorter path
if (!segmentA && segmentB) return -1;
if (segmentA && !segmentB) return 1;

// Prioritize static segments over dynamic segments
if (!isDynamicSegment(segmentA) && isDynamicSegment(segmentB)) return -1;
if (isDynamicSegment(segmentA) && !isDynamicSegment(segmentB)) return 1;

// Prioritize non-catch-all segments over catch-all segments
if (!isCatchAllSegment(segmentA) && isCatchAllSegment(segmentB)) return -1;
if (isCatchAllSegment(segmentA) && !isCatchAllSegment(segmentB)) return 1;

// Prioritize non-optional catch-all segments over optional catch-all segments
if (
!isOptionalCatchAllSegment(segmentA) &&
isOptionalCatchAllSegment(segmentB)
) {
return -1;
}
if (
isOptionalCatchAllSegment(segmentA) &&
!isOptionalCatchAllSegment(segmentB)
) {
return 1;
}

if (segmentA === segmentB) continue;
}

// Both pathnames are completely static
return 0;
}

export function getSortedPathnames(pathnames: Array<string>) {
return pathnames.sort(comparePathnamePairs);
}
61 changes: 1 addition & 60 deletions packages/next-intl/test/middleware/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import {
formatPathnameTemplate,
getInternalTemplate,
getNormalizedPathname,
getRouteParams,
getSortedPathnames
getRouteParams
} from '../../src/middleware/utils';

describe('getNormalizedPathname', () => {
Expand Down Expand Up @@ -169,61 +168,3 @@ describe('getInternalTemplate', () => {
]);
});
});

describe('getSortedPathnames', () => {
it('works for static routes that include the root', () => {
expect(getSortedPathnames(['/', '/foo', '/test'])).toEqual([
'/',
'/foo',
'/test'
]);
});

it('should prioritize non-catch-all routes over catch-all routes', () => {
expect(
getSortedPathnames(['/categories/[...slug]', '/categories/new'])
).toEqual(['/categories/new', '/categories/[...slug]']);
});

it('should prioritize static routes over optional catch-all routes', () => {
expect(
getSortedPathnames(['/categories/[[...slug]]', '/categories'])
).toEqual(['/categories', '/categories/[[...slug]]']);
});

it('should prioritize more specific routes over dynamic routes', () => {
expect(
getSortedPathnames(['/categories/[slug]', '/categories/new'])
).toEqual(['/categories/new', '/categories/[slug]']);
});

it('should prioritize dynamic routes over catch-all routes', () => {
expect(
getSortedPathnames(['/categories/[...slug]', '/categories/[slug]'])
).toEqual(['/categories/[slug]', '/categories/[...slug]']);
});

it('should prioritize more specific nested routes over dynamic routes', () => {
expect(
getSortedPathnames([
'/articles/[category]/[articleSlug]',
'/articles/[category]/new'
])
).toEqual([
'/articles/[category]/new',
'/articles/[category]/[articleSlug]'
]);
});

it('should prioritize more specific nested routes over catch-all routes', () => {
expect(
getSortedPathnames([
'/articles/[category]/[...articleSlug]',
'/articles/[category]/new'
])
).toEqual([
'/articles/[category]/new',
'/articles/[category]/[...articleSlug]'
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,28 @@ import {Pathnames} from '../../../src/routing';

vi.mock('next/navigation');

const locales = ['en', 'de'] as const;
const locales = ['en', 'de', 'ja'] as const;
const pathnames = {
'/': '/',
'/about': {
en: '/about',
de: '/ueber-uns'
de: '/ueber-uns',
ja: '/約'
},
'/news/[articleSlug]-[articleId]': {
en: '/news/[articleSlug]-[articleId]',
de: '/neuigkeiten/[articleSlug]-[articleId]'
de: '/neuigkeiten/[articleSlug]-[articleId]',
ja: '/ニュース/[articleSlug]-[articleId]'
},
'/categories/[...parts]': {
en: '/categories/[...parts]',
de: '/kategorien/[...parts]'
de: '/kategorien/[...parts]',
ja: '/カテゴリ/[...parts]'
},
'/categories/new': {
en: '/categories/new',
de: '/kategorien/neu',
ja: '/カテゴリ/新規'
},
'/catch-all/[[...parts]]': '/catch-all/[[...parts]]'
} satisfies Pathnames<typeof locales>;
Expand Down Expand Up @@ -83,6 +91,28 @@ describe("localePrefix: 'as-needed'", () => {
screen.getByText('/news/[articleSlug]-[articleId]');
});

it('returns the internal pathname for a more specific pathname that overlaps with another pathname', () => {
function Component() {
const pathname = usePathname();
return <>{pathname}</>;
}

vi.mocked(useNextPathname).mockImplementation(() => '/en/categories/new');
render(<Component />);
screen.getByText('/categories/new');
});

it('returns an encoded pathname correctly', () => {
function Component() {
const pathname = usePathname();
return <>{pathname}</>;
}
vi.mocked(useParams).mockImplementation(() => ({locale: 'ja'}));
vi.mocked(useNextPathname).mockImplementation(() => '/ja/%E7%B4%84');
render(<Component />);
screen.getByText('/about');
});

it('returns the internal pathname a non-default locale', () => {
vi.mocked(useParams).mockImplementation(() => ({locale: 'de'}));

Expand Down
Loading
Loading