-
Notifications
You must be signed in to change notification settings - Fork 221
fix: App Router discovery for nested parallel-slot sub-routes #458
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -476,6 +476,24 @@ describe("appRouter - route discovery", () => { | |
| }); | ||
| }); | ||
|
|
||
| it("does not create nested @slot sub-routes without a children default fallback", async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good test. The name accurately describes what it asserts. Note that this test gives It might be worth adding a companion test for the other case: |
||
| await withTempDir("vinext-app-slot-missing-children-default-", async (tmpDir) => { | ||
| const appDir = path.join(tmpDir, "app"); | ||
| await mkdir(path.join(appDir, "inbox", "@modal", "profile"), { recursive: true }); | ||
| await writeFile(path.join(appDir, "inbox", "page.tsx"), EMPTY_PAGE); | ||
| await writeFile(path.join(appDir, "inbox", "@modal", "default.tsx"), EMPTY_PAGE); | ||
| await writeFile(path.join(appDir, "inbox", "@modal", "profile", "page.tsx"), EMPTY_PAGE); | ||
|
|
||
| invalidateAppRouteCache(); | ||
| const routes = await appRouter(appDir); | ||
| const patterns = routes.map((route) => route.pattern); | ||
|
|
||
| expect(patterns).toContain("/inbox"); | ||
| expect(patterns).not.toContain("/inbox/profile"); | ||
| expect(matchAppRoute("/inbox/profile", routes)).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| it("rejects non-terminal catch-all intercept targets", async () => { | ||
| await withTempDir("vinext-app-intercept-nonterminal-catchall-", async (tmpDir) => { | ||
| const appDir = path.join(tmpDir, "app"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the right fix — previously, if
childrenDefaultwasnull, synthetic routes were created withpagePath: null, which would 500 at render time. Combining the two guards into onecontinueis clean.Minor: the original code had the
subPathMap.size === 0check first (cheap) before thefindFilecall (does filesystem I/O). The new code always callsfindFileeven whensubPathMapis empty. Consider preserving the short-circuit:This avoids a pointless
findFilecall when there are no sub-paths. Not a big deal sincefs.existsSyncis fast, but it's a free optimization and arguably clearer about the two independent exit conditions.