Skip to content

Commit 6037878

Browse files
committed
feat(build): add validation to detect ambiguous app routes at build time
Introduces validateAppPaths() to catch route conflicts that were previously going undetected until runtime. Routes with different dynamic segment names (e.g., [slug] vs [modalSlug]) that normalize to the same pattern now trigger clear build-time errors with actionable guidance. This prevents subtle routing issues in parallel routes and interception routes where structural equivalence wasn't being validated during the build process.
1 parent 4427869 commit 6037878

File tree

4 files changed

+564
-4
lines changed

4 files changed

+564
-4
lines changed

packages/next/src/build/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ import {
229229
writeValidatorFile,
230230
} from '../server/lib/router-utils/route-types-utils'
231231
import { Lockfile } from './lockfile'
232+
import { validateAppPaths } from './validate-app-paths'
232233

233234
type Fallback = null | boolean | string
234235

@@ -1385,6 +1386,13 @@ export default async function build(
13851386
}
13861387

13871388
const appPaths = Array.from(appPageKeys)
1389+
1390+
// Validate that the app paths are valid. This is currently duplicating
1391+
// the logic from packages/next/src/shared/lib/router/utils/sorted-routes.ts
1392+
// but is instead specifically focused on code that can be shared
1393+
// eventually with the development code.
1394+
validateAppPaths(appPaths)
1395+
13881396
// Interception routes are modelled as beforeFiles rewrites
13891397
rewrites.beforeFiles.push(
13901398
...generateInterceptionRoutesRewrites(appPaths, config.basePath)
Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
import { validateAppPaths } from './validate-app-paths'
2+
3+
describe('validateAppPaths', () => {
4+
// NOTE: The paths passed to validateAppPaths have already been normalized
5+
// by normalizeAppPath, which strips out parallel route segments (@modal, etc.),
6+
// route groups ((group)), and the trailing /page or /route segment.
7+
//
8+
// So app/blog/@modal/[slug]/page.tsx becomes /blog/[slug]
9+
// and app/blog/[slug]/page.tsx also becomes /blog/[slug]
10+
11+
describe('should allow valid route configurations', () => {
12+
it('allows routes with different static segments', () => {
13+
const paths = ['/blog/posts', '/blog/authors', '/about']
14+
15+
expect(() => validateAppPaths(paths)).not.toThrow()
16+
})
17+
18+
it('allows routes with different depths', () => {
19+
const paths = ['/blog', '/blog/[slug]', '/blog/[slug]/comments']
20+
21+
expect(() => validateAppPaths(paths)).not.toThrow()
22+
})
23+
24+
it('allows routes with different dynamic segment positions', () => {
25+
const paths = ['/[category]/posts', '/posts/[slug]', '/posts/featured']
26+
27+
expect(() => validateAppPaths(paths)).not.toThrow()
28+
})
29+
30+
it('allows routes with different catch-all patterns', () => {
31+
const paths = ['/docs/[...slug]', '/blog/[slug]']
32+
33+
expect(() => validateAppPaths(paths)).not.toThrow()
34+
})
35+
36+
it('allows special routes', () => {
37+
const paths = ['/_not-found', '/_global-error', '/blog/[slug]', '/about']
38+
39+
expect(() => validateAppPaths(paths)).not.toThrow()
40+
})
41+
42+
it('allows same dynamic segment names in different paths', () => {
43+
const paths = ['/blog/[slug]', '/posts/[slug]', '/docs/[slug]']
44+
45+
expect(() => validateAppPaths(paths)).not.toThrow()
46+
})
47+
48+
it('allows routes with optional catch-all', () => {
49+
const paths = ['/docs/[[...slug]]', '/blog/[slug]']
50+
51+
expect(() => validateAppPaths(paths)).not.toThrow()
52+
})
53+
54+
it('allows catch-all at the end of path', () => {
55+
const paths = ['/docs/[...slug]', '/blog/posts/[...rest]']
56+
57+
expect(() => validateAppPaths(paths)).not.toThrow()
58+
})
59+
})
60+
61+
describe('should detect ambiguous routes', () => {
62+
it('detects conflict from normalized parallel routes (most common case)', () => {
63+
// This represents:
64+
// - app/blog/[slug]/page.tsx
65+
// - app/blog/@modal/[modalSlug]/page.tsx (normalized to /blog/[modalSlug])
66+
const paths = ['/blog/[slug]', '/blog/[modalSlug]']
67+
68+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
69+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[slug\]/)
70+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[modalSlug\]/)
71+
})
72+
73+
it('detects conflict between three normalized parallel routes', () => {
74+
// This represents multiple parallel slots with dynamic segments
75+
const paths = ['/dashboard/[id]', '/dashboard/[userId]']
76+
77+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
78+
expect(() => validateAppPaths(paths)).toThrow(/dashboard\/\[id\]/)
79+
expect(() => validateAppPaths(paths)).toThrow(/dashboard\/\[userId\]/)
80+
})
81+
82+
it('detects conflict with different dynamic segment names', () => {
83+
const paths = ['/blog/[slug]', '/blog/[id]']
84+
85+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
86+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[slug\]/)
87+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[id\]/)
88+
})
89+
90+
it('detects conflict with catch-all segments', () => {
91+
const paths = ['/docs/[...slug]', '/docs/[...pages]']
92+
93+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
94+
})
95+
96+
it('detects conflict with optional catch-all segments', () => {
97+
const paths = ['/docs/[[...slug]]', '/docs/[[...pages]]']
98+
99+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
100+
})
101+
102+
it('detects multiple conflicts', () => {
103+
const paths = [
104+
'/blog/[slug]',
105+
'/blog/[id]',
106+
'/posts/[id]',
107+
'/posts/[slug]',
108+
]
109+
110+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
111+
// Should report both conflicts
112+
const error = () => validateAppPaths(paths)
113+
expect(error).toThrow(/blog/)
114+
expect(error).toThrow(/posts/)
115+
})
116+
117+
it('detects conflict with three or more routes', () => {
118+
// Three different routes that all normalize to the same pattern
119+
const paths = ['/blog/[slug]', '/blog/[id]', '/blog/[postId]']
120+
121+
expect(() => validateAppPaths(paths)).toThrow(/Ambiguous app routes/)
122+
// All three should be listed
123+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[slug\]/)
124+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[id\]/)
125+
expect(() => validateAppPaths(paths)).toThrow(/blog\/\[postId\]/)
126+
})
127+
})
128+
129+
describe('individual path validation', () => {
130+
describe('segment syntax errors', () => {
131+
it('detects three-dot character (…) instead of ...', () => {
132+
const paths = ['/docs/[…slug]']
133+
134+
expect(() => validateAppPaths(paths)).toThrow(
135+
/three-dot character \(''\)/
136+
)
137+
})
138+
139+
it('detects extra brackets in segment names', () => {
140+
const paths = ['/blog/[[slug]']
141+
142+
expect(() => validateAppPaths(paths)).toThrow(
143+
/may not start or end with extra brackets/
144+
)
145+
})
146+
147+
it('detects erroneous periods at start of segment', () => {
148+
const paths = ['/blog/[.slug]']
149+
150+
expect(() => validateAppPaths(paths)).toThrow(
151+
/may not start with erroneous periods/
152+
)
153+
})
154+
155+
it('detects optional non-catch-all segments', () => {
156+
const paths = ['/blog/[[slug]]']
157+
158+
expect(() => validateAppPaths(paths)).toThrow(
159+
/Optional route parameters are not yet supported/
160+
)
161+
})
162+
})
163+
164+
describe('duplicate slug names', () => {
165+
it('detects duplicate slug names in same path', () => {
166+
const paths = ['/blog/[slug]/posts/[slug]']
167+
168+
expect(() => validateAppPaths(paths)).toThrow(
169+
/same slug name "slug" repeat/
170+
)
171+
})
172+
173+
it('detects slug names differing only by non-word symbols', () => {
174+
const paths = ['/blog/[helloworld]/[hello-world]']
175+
176+
expect(() => validateAppPaths(paths)).toThrow(
177+
/differ only by non-word symbols/
178+
)
179+
})
180+
})
181+
182+
describe('catch-all placement', () => {
183+
it('detects catch-all not at the end', () => {
184+
const paths = ['/docs/[...slug]/more']
185+
186+
expect(() => validateAppPaths(paths)).toThrow(
187+
/Catch-all must be the last part/
188+
)
189+
})
190+
191+
it('detects optional catch-all not at the end', () => {
192+
const paths = ['/docs/[[...slug]]/more']
193+
194+
expect(() => validateAppPaths(paths)).toThrow(
195+
/Optional catch-all must be the last part/
196+
)
197+
})
198+
199+
it('detects both required and optional catch-all in same path', () => {
200+
// This would be impossible in practice but we should catch it
201+
const paths = ['/docs/[...required]/[[...optional]]']
202+
203+
expect(() => validateAppPaths(paths)).toThrow(
204+
/cannot use both a required and optional catch-all/
205+
)
206+
})
207+
})
208+
209+
describe('optional catch-all specificity conflicts', () => {
210+
it('detects route with same specificity as optional catch-all', () => {
211+
const paths = ['/docs', '/docs/[[...slug]]']
212+
213+
expect(() => validateAppPaths(paths)).toThrow(
214+
/same specificity as an optional catch-all/
215+
)
216+
})
217+
218+
it('allows optional catch-all without conflicting route', () => {
219+
const paths = ['/docs/[[...slug]]']
220+
221+
expect(() => validateAppPaths(paths)).not.toThrow()
222+
})
223+
224+
it('allows nested optional catch-all without conflict', () => {
225+
const paths = ['/docs/api/[[...slug]]', '/docs/guides']
226+
227+
expect(() => validateAppPaths(paths)).not.toThrow()
228+
})
229+
})
230+
})
231+
232+
describe('edge cases', () => {
233+
it('handles empty array', () => {
234+
expect(() => validateAppPaths([])).not.toThrow()
235+
})
236+
237+
it('handles single route', () => {
238+
expect(() => validateAppPaths(['/blog/[slug]'])).not.toThrow()
239+
})
240+
241+
it('handles complex nested structures', () => {
242+
const paths = [
243+
'/[locale]/blog/[category]/[slug]',
244+
'/[locale]/blog/[category]/featured',
245+
]
246+
247+
expect(() => validateAppPaths(paths)).not.toThrow()
248+
})
249+
250+
it('handles root route', () => {
251+
const paths = ['/', '/blog']
252+
253+
expect(() => validateAppPaths(paths)).not.toThrow()
254+
})
255+
})
256+
257+
describe('error message quality', () => {
258+
it('provides clear error message with normalized path', () => {
259+
const paths = ['/blog/[slug]', '/blog/[modalSlug]']
260+
261+
expect(() => validateAppPaths(paths)).toThrow(
262+
/Ambiguous route pattern "\/blog\/\[\*\]"/
263+
)
264+
})
265+
266+
it('provides actionable guidance', () => {
267+
const paths = ['/blog/[slug]', '/blog/[id]']
268+
269+
expect(() => validateAppPaths(paths)).toThrow(
270+
/ensure that dynamic segments have unique patterns/
271+
)
272+
})
273+
274+
it('lists all conflicting routes', () => {
275+
const paths = ['/blog/[slug]', '/blog/[id]', '/blog/[postId]']
276+
277+
try {
278+
validateAppPaths(paths)
279+
throw new Error('Should have thrown an error')
280+
} catch (error) {
281+
const message = (error as Error).message
282+
if (message === 'Should have thrown an error') {
283+
throw error
284+
}
285+
expect(message).toContain('/blog/[slug]')
286+
expect(message).toContain('/blog/[id]')
287+
expect(message).toContain('/blog/[postId]')
288+
}
289+
})
290+
291+
it('provides clear message for syntax errors', () => {
292+
const paths = ['/docs/[...slug]/more']
293+
294+
expect(() => validateAppPaths(paths)).toThrow(
295+
/in route "\/docs\/\[\.\.\.slug\]\/more"/
296+
)
297+
})
298+
})
299+
})

0 commit comments

Comments
 (0)