Skip to content

Commit

Permalink
Merge branch 'fix/sort-pathnames-usepathname' into canary
Browse files Browse the repository at this point in the history
  • Loading branch information
amannn committed Jun 26, 2024
2 parents 605c9a6 + 8d3ab38 commit cb15046
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 149 deletions.
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

0 comments on commit cb15046

Please sign in to comment.