diff --git a/packages/nextjs/src/client/routing/parameterization.ts b/packages/nextjs/src/client/routing/parameterization.ts index a45f0faab8fd..da25c1beb840 100644 --- a/packages/nextjs/src/client/routing/parameterization.ts +++ b/packages/nextjs/src/client/routing/parameterization.ts @@ -12,42 +12,67 @@ let cachedManifestString: string | undefined = undefined; const compiledRegexCache: Map = new Map(); const routeResultCache: Map = new Map(); +// Specificity ranks for a single route segment, from most to least specific. `END` is the rank of +// the position just past the last segment of a route, so that a route which stops is compared +// against whatever the longer route continues with. +const SEGMENT_STATIC = 0; +const SEGMENT_DYNAMIC = 1; +const SEGMENT_END = 2; +const SEGMENT_CATCH_ALL = 3; +const SEGMENT_OPTIONAL_CATCH_ALL = 4; + /** - * Calculate the specificity score for a route path. - * Lower scores indicate more specific routes. + * Calculate the specificity rank for a single route segment. + * Lower ranks indicate more specific segments. */ -function getRouteSpecificity(routePath: string): number { - const segments = routePath.split('/').filter(Boolean); - let score = 0; - - for (const segment of segments) { - if (segment.startsWith(':')) { - const paramName = segment.substring(1); - if (paramName.endsWith('*?')) { - // Optional catch-all: [[...param]] - score += 1000; - } else if (paramName.endsWith('*')) { - // Required catch-all: [...param] - score += 100; - } else { - // Regular dynamic segment: [param] - score += 10; - } - } - // Static segments add 0 to score as they are most specific +function getSegmentSpecificity(segment: string | undefined): number { + if (segment === undefined) { + // The route has no more segments + return SEGMENT_END; + } + if (!segment.startsWith(':')) { + // Static segment: matches exactly one known value + return SEGMENT_STATIC; } - if (segments.length > 0) { - // Add a small penalty based on inverse of segment count - // This ensures that routes with more segments are preferred - // e.g., '/:locale/foo' is more specific than '/:locale' - // We use a small value (1 / segments.length) so it doesn't override the main scoring - // but breaks ties between routes with the same number of dynamic segments - const segmentCountPenalty = 1 / segments.length; - score += segmentCountPenalty; + const paramName = segment.substring(1); + if (paramName.endsWith('*?')) { + // Optional catch-all: [[...param]] + return SEGMENT_OPTIONAL_CATCH_ALL; + } + if (paramName.endsWith('*')) { + // Required catch-all: [...param] + return SEGMENT_CATCH_ALL; + } + // Regular dynamic segment: [param] + return SEGMENT_DYNAMIC; +} + +/** + * Compare two route paths by specificity, ordering the most specific route first. + * + * Routes are compared segment by segment, with the first segment they disagree on deciding the + * winner. Comparing aggregate scores instead would rank a short catch-all like '/:locale/:rest*' + * above a longer but strictly narrower route like '/:locale/guides/:category/:rest*', because the + * longer route accumulates more score simply by having more segments. + * + * Routes of differing lengths are compared one segment past the shorter one, where `SEGMENT_END` + * decides whether continuing narrows the route or widens it: '/:locale/foo' is more specific than + * '/:locale', but '/:locale' is more specific than '/:locale/:rest*'. + */ +function compareRouteSpecificity(routePathA: string, routePathB: string): number { + const segmentsA = routePathA.split('/').filter(Boolean); + const segmentsB = routePathB.split('/').filter(Boolean); + + const comparedSegmentCount = Math.min(segmentsA.length, segmentsB.length) + 1; + for (let i = 0; i < comparedSegmentCount; i++) { + const difference = getSegmentSpecificity(segmentsA[i]) - getSegmentSpecificity(segmentsB[i]); + if (difference !== 0) { + return difference; + } } - return score; + return 0; } /** @@ -198,7 +223,7 @@ export const maybeParameterizeRoute = (route: string): string | undefined => { const matches = findMatchingRoutes(normalizedRoute, staticRoutes, dynamicRoutes); // We can always do the `sort()` call, it will short-circuit when it has one array item - const result = matches.sort((a, b) => getRouteSpecificity(a) - getRouteSpecificity(b))[0]; + const result = matches.sort(compareRouteSpecificity)[0]; routeResultCache.set(normalizedRoute, result); diff --git a/packages/nextjs/src/config/manifest/createRouteManifest.ts b/packages/nextjs/src/config/manifest/createRouteManifest.ts index 487ab05c55d8..d7f6ad59428e 100644 --- a/packages/nextjs/src/config/manifest/createRouteManifest.ts +++ b/packages/nextjs/src/config/manifest/createRouteManifest.ts @@ -2,6 +2,12 @@ import * as fs from 'fs'; import * as path from 'path'; import type { RouteInfo, RouteManifest } from './types'; +/** + * Param names that are treated as an optional i18n prefix, so that unprefixed paths of the default + * locale (e.g. next-intl's `localePrefix: 'as-needed'`) still match their localized route pattern. + */ +export const DEFAULT_LOCALE_PARAM_NAMES = ['locale', 'lang', 'language', 'lng']; + export type CreateRouteManifestOptions = { // For starters we only support app router appDirPath?: string; @@ -14,11 +20,17 @@ export type CreateRouteManifestOptions = { * Base path for the application, if any. This will be prefixed to all routes. */ basePath?: string; + /** + * Param names to treat as an optional i18n prefix. Replaces (does not extend) the defaults. + * Pass an empty array to disable optional prefix matching entirely. + */ + localeParamNames?: string[]; }; let manifestCache: RouteManifest | null = null; let lastAppDirPath: string | null = null; let lastIncludeRouteGroups: boolean | undefined = undefined; +let lastLocaleParamNames: string[] | undefined = undefined; function isPageFile(filename: string): boolean { return filename === 'page.tsx' || filename === 'page.jsx' || filename === 'page.ts' || filename === 'page.js'; @@ -48,7 +60,10 @@ function getDynamicRouteSegment(name: string): string { return `:${name.slice(1, -1)}`; } -function buildRegexForDynamicRoute(routePath: string): { +function buildRegexForDynamicRoute( + routePath: string, + localeParamNames: string[], +): { regex: string; paramNames: string[]; hasOptionalPrefix: boolean; @@ -100,20 +115,19 @@ function buildRegexForDynamicRoute(routePath: string): { pattern = `^/${regexSegments.join('/')}$`; } - return { regex: pattern, paramNames, hasOptionalPrefix: hasOptionalPrefix(paramNames) }; + return { regex: pattern, paramNames, hasOptionalPrefix: hasOptionalPrefix(paramNames, localeParamNames) }; } /** - * Detect if the first parameter is a common i18n prefix segment - * Common patterns: locale, lang, language + * Detect if the first parameter is an i18n prefix segment */ -function hasOptionalPrefix(paramNames: string[]): boolean { +function hasOptionalPrefix(paramNames: string[], localeParamNames: string[]): boolean { const firstParam = paramNames[0]; if (firstParam === undefined) { return false; } - return firstParam === 'locale' || firstParam === 'lang' || firstParam === 'language'; + return localeParamNames.includes(firstParam); } /** @@ -130,7 +144,12 @@ function checkForGenerateStaticParams(pageFilePath: string): boolean { } } -function scanAppDirectory(dir: string, basePath: string = '', includeRouteGroups: boolean = false): RouteManifest { +function scanAppDirectory( + dir: string, + basePath: string = '', + includeRouteGroups: boolean = false, + localeParamNames: string[] = DEFAULT_LOCALE_PARAM_NAMES, +): RouteManifest { const dynamicRoutes: RouteInfo[] = []; const staticRoutes: RouteInfo[] = []; const isrRoutes: string[] = []; @@ -153,7 +172,7 @@ function scanAppDirectory(dir: string, basePath: string = '', includeRouteGroups } if (isDynamic) { - const { regex, paramNames, hasOptionalPrefix } = buildRegexForDynamicRoute(routePath); + const { regex, paramNames, hasOptionalPrefix } = buildRegexForDynamicRoute(routePath, localeParamNames); dynamicRoutes.push({ path: routePath, regex, @@ -188,7 +207,7 @@ function scanAppDirectory(dir: string, basePath: string = '', includeRouteGroups } const newBasePath = routeSegment ? `${basePath}/${routeSegment}` : basePath; - const subRoutes = scanAppDirectory(fullPath, newBasePath, includeRouteGroups); + const subRoutes = scanAppDirectory(fullPath, newBasePath, includeRouteGroups, localeParamNames); dynamicRoutes.push(...subRoutes.dynamicRoutes); staticRoutes.push(...subRoutes.staticRoutes); @@ -231,8 +250,15 @@ export function createRouteManifest(options?: CreateRouteManifestOptions): Route }; } + const localeParamNames = options?.localeParamNames ?? DEFAULT_LOCALE_PARAM_NAMES; + // Check if we can use cached version - if (manifestCache && lastAppDirPath === targetDir && lastIncludeRouteGroups === options?.includeRouteGroups) { + if ( + manifestCache && + lastAppDirPath === targetDir && + lastIncludeRouteGroups === options?.includeRouteGroups && + lastLocaleParamNames?.join(',') === localeParamNames.join(',') + ) { return manifestCache; } @@ -240,6 +266,7 @@ export function createRouteManifest(options?: CreateRouteManifestOptions): Route targetDir, options?.basePath, options?.includeRouteGroups, + localeParamNames, ); const manifest: RouteManifest = { @@ -252,6 +279,7 @@ export function createRouteManifest(options?: CreateRouteManifestOptions): Route manifestCache = manifest; lastAppDirPath = targetDir; lastIncludeRouteGroups = options?.includeRouteGroups; + lastLocaleParamNames = localeParamNames; return manifest; } diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts index 1786b90849eb..c30ff63efec9 100644 --- a/packages/nextjs/src/config/types.ts +++ b/packages/nextjs/src/config/types.ts @@ -630,6 +630,11 @@ export type SentryBuildOptions = { * routeManifestInjection: { * exclude: (route) => route.includes('hidden') * } + * + * // Treat a custom param name as an optional i18n prefix + * routeManifestInjection: { + * localeParamNames: ['lng'] + * } * ``` */ routeManifestInjection?: @@ -649,6 +654,20 @@ export type SentryBuildOptions = { * - A function that receives a route path and returns `true` to exclude it */ exclude?: Array | ((route: string) => boolean); + + /** + * Route param names that represent an i18n locale prefix, e.g. the `lng` in `app/[lng]/page.tsx`. + * + * Routes whose first param matches one of these names are also matched against paths that omit + * the prefix, so that unprefixed default-locale URLs (e.g. next-intl's `localePrefix: 'as-needed'`) + * are parameterized as the localized route instead of falling through to a catch-all. + * + * This replaces the built-in list rather than extending it. Pass an empty array to disable + * optional prefix matching entirely. + * + * @default ['locale', 'lang', 'language', 'lng'] + */ + localeParamNames?: string[]; }; /** diff --git a/packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts b/packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts index a4620beb3db1..f1022a6c7ff2 100644 --- a/packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts +++ b/packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts @@ -98,6 +98,7 @@ export function maybeCreateRouteManifest( const manifest = createRouteManifest({ basePath: incomingUserNextConfigObject.basePath, + localeParamNames: userSentryOptions.routeManifestInjection?.localeParamNames, }); // Apply route exclusion filter if configured diff --git a/packages/nextjs/test/client/parameterization.test.ts b/packages/nextjs/test/client/parameterization.test.ts index 18bd61bee210..88817c2cb518 100644 --- a/packages/nextjs/test/client/parameterization.test.ts +++ b/packages/nextjs/test/client/parameterization.test.ts @@ -643,6 +643,55 @@ describe('maybeParameterizeRoute', () => { // Catch-all should be used when no more specific routes match expect(maybeParameterizeRoute('/some/random/path')).toBe('/:catchall*'); }); + + it('should prefer a longer route over a shorter catch-all that also matches', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/:locale/:notFound*', + regex: '^/([^/]+)/(.+)$', + paramNames: ['locale', 'notFound'], + }, + { + path: '/:locale/guides/:category/:rest*', + regex: '^/([^/]+)/guides/([^/]+)/(.+)$', + paramNames: ['locale', 'category', 'rest'], + }, + ], + }; + globalWithInjectedManifest._sentryRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRoute('/fr/guides/renting/foo')).toBe('/:locale/guides/:category/:rest*'); + + // The catch-all still wins where nothing narrower matches + expect(maybeParameterizeRoute('/fr/anything/else')).toBe('/:locale/:notFound*'); + }); + + it('should prefer a route that ends over one that continues into a catch-all', () => { + const manifest: RouteManifest = { + staticRoutes: [], + dynamicRoutes: [ + { + path: '/:locale/:notFound*', + regex: '^/([^/]+)/(.+)$', + paramNames: ['locale', 'notFound'], + hasOptionalPrefix: true, + }, + { + path: '/:locale', + regex: '^/([^/]+)$', + paramNames: ['locale'], + hasOptionalPrefix: true, + }, + ], + }; + globalWithInjectedManifest._sentryRouteManifest = JSON.stringify(manifest); + + // '/fr' matches '/:locale' directly, and '/:locale/:notFound*' only via the optional prefix + expect(maybeParameterizeRoute('/fr')).toBe('/:locale'); + expect(maybeParameterizeRoute('/')).toBe('/:locale'); + }); }); describe('i18n routing with optional prefix', () => { diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/about/page.tsx b/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/about/page.tsx new file mode 100644 index 000000000000..5d33b5d14573 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/about/page.tsx @@ -0,0 +1 @@ +// beep diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/page.tsx b/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/page.tsx new file mode 100644 index 000000000000..5d33b5d14573 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/app-custom/[loc]/page.tsx @@ -0,0 +1 @@ +// beep diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/[...notFound]/page.tsx b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/[...notFound]/page.tsx new file mode 100644 index 000000000000..5d33b5d14573 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/[...notFound]/page.tsx @@ -0,0 +1 @@ +// beep diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/guides/[category]/[...rest]/page.tsx b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/guides/[category]/[...rest]/page.tsx new file mode 100644 index 000000000000..5d33b5d14573 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/guides/[category]/[...rest]/page.tsx @@ -0,0 +1 @@ +// beep diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/page.tsx b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/page.tsx new file mode 100644 index 000000000000..5d33b5d14573 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/app/[lng]/page.tsx @@ -0,0 +1 @@ +// beep diff --git a/packages/nextjs/test/config/manifest/suites/locale-prefix/locale-prefix.test.ts b/packages/nextjs/test/config/manifest/suites/locale-prefix/locale-prefix.test.ts new file mode 100644 index 000000000000..d6009e82ed82 --- /dev/null +++ b/packages/nextjs/test/config/manifest/suites/locale-prefix/locale-prefix.test.ts @@ -0,0 +1,78 @@ +import { GLOBAL_OBJ } from '@sentry/core'; +import path from 'path'; +import { afterEach, describe, expect, test } from 'vitest'; +import { maybeParameterizeRoute } from '../../../../../src/client/routing/parameterization'; +import { createRouteManifest } from '../../../../../src/config/manifest/createRouteManifest'; +import type { RouteManifest } from '../../../../../src/config/manifest/types'; + +const globalWithInjectedManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + _sentryRouteManifest: string | undefined; +}; + +function getRoute(manifest: RouteManifest, routePath: string): RouteManifest['dynamicRoutes'][number] { + const route = manifest.dynamicRoutes.find(r => r.path === routePath); + if (!route) { + throw new Error(`Route ${routePath} not found in manifest`); + } + return route; +} + +describe('locale-prefix', () => { + const originalManifest = globalWithInjectedManifest._sentryRouteManifest; + + afterEach(() => { + globalWithInjectedManifest._sentryRouteManifest = originalManifest; + }); + + describe('default locale param names', () => { + const manifest = createRouteManifest({ appDirPath: path.join(__dirname, 'app') }); + + test('flags `lng` as an optional prefix', () => { + expect(getRoute(manifest, '/:lng').hasOptionalPrefix).toBe(true); + expect(getRoute(manifest, '/:lng/guides/:category/:rest*').hasOptionalPrefix).toBe(true); + expect(getRoute(manifest, '/:lng/:notFound*').hasOptionalPrefix).toBe(true); + }); + + test('parameterizes unprefixed default-locale paths against the real route, not the catch-all', () => { + globalWithInjectedManifest._sentryRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRoute('/guides/renting/foo')).toBe('/:lng/guides/:category/:rest*'); + expect(maybeParameterizeRoute('/fr/guides/renting/foo')).toBe('/:lng/guides/:category/:rest*'); + expect(maybeParameterizeRoute('/does/not/exist')).toBe('/:lng/:notFound*'); + }); + + test('parameterizes the locale root as the locale page, not the catch-all', () => { + globalWithInjectedManifest._sentryRouteManifest = JSON.stringify(manifest); + + expect(maybeParameterizeRoute('/')).toBe('/:lng'); + expect(maybeParameterizeRoute('/fr')).toBe('/:lng'); + }); + }); + + describe('custom locale param names', () => { + test('flags configured param names as an optional prefix', () => { + const manifest = createRouteManifest({ + appDirPath: path.join(__dirname, 'app-custom'), + localeParamNames: ['loc'], + }); + + expect(getRoute(manifest, '/:loc').hasOptionalPrefix).toBe(true); + expect(getRoute(manifest, '/:loc/about').hasOptionalPrefix).toBe(true); + }); + + test('replaces rather than extends the defaults', () => { + const manifest = createRouteManifest({ + appDirPath: path.join(__dirname, 'app'), + localeParamNames: ['loc'], + }); + + expect(getRoute(manifest, '/:lng').hasOptionalPrefix).toBe(false); + }); + + test('disables optional prefix matching when passed an empty list', () => { + const manifest = createRouteManifest({ appDirPath: path.join(__dirname, 'app'), localeParamNames: [] }); + + expect(getRoute(manifest, '/:lng').hasOptionalPrefix).toBe(false); + }); + }); +});