fix(dev): avoid app reloads for colocated files#1014
fix(dev): avoid app reloads for colocated files#1014james-elicx merged 1 commit intocloudflare:mainfrom
Conversation
Adding or removing any JavaScript or TypeScript file under app currently invalidates the App Router route cache and sends a full reload. That is wrong for colocated components and helpers because the App Router only treats specific file conventions and metadata files as route structure. The watcher was using the broad page extension matcher for app files, which confused ordinary project files with route-affecting files. Add a dev route-file predicate aligned with vinext route and metadata discovery, then use it before invalidating the generated RSC entry and root params modules. Tests cover route convention files, metadata files, private folders, sibling directory prefix safety, and colocated non-route files.
commit: |
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| const routePrefix = visibleRoutePrefix(parts); | ||
| for (const [metaType, config] of Object.entries(METADATA_FILE_MAP)) { | ||
| if (!matchMetadataFileBaseName(metaType, baseName)) continue; | ||
| if (!config.nestable && routePrefix !== "") return false; |
There was a problem hiding this comment.
Nit: isMetadataRouteFile correctly returns false when !config.nestable && routePrefix !== "", but it does so only for the first matching metaType. If a future METADATA_FILE_MAP entry happened to share a base name with an existing non-nestable entry, the early return false would prevent the loop from checking subsequent entries.
This can't happen today because all base names in the map are unique, but making the intent explicit would be a minor hardening:
| if (!config.nestable && routePrefix !== "") return false; | |
| if (!config.nestable && routePrefix !== "") continue; |
This way, a non-nestable mismatch just moves on to the next entry instead of short-circuiting the entire function.
| expect(shouldInvalidateAppRouteFile(appDir, "/project/app/@modal/default.tsx", matcher)).toBe( | ||
| true, | ||
| ); | ||
| expect(shouldInvalidateAppRouteFile(appDir, "/project/app/robots.ts", matcher)).toBe(true); |
There was a problem hiding this comment.
Nice that you cover the non-nestable robots.ts at root. Consider also adding a test for a nestable metadata route in a nested directory (e.g. app/blog/sitemap.xml -> true) to confirm the nestable path works end-to-end. Currently the only nested metadata test is opengraph-image.png, which covers static extensions but not dynamic nestable files.
Also worth adding a case for app/global-error.tsx -> true since that's a separate code path (isRootGlobalError) that isn't currently exercised in the test.
|
|
||
| function visibleRoutePrefix(parts: readonly string[]): string { | ||
| const visibleParts = parts | ||
| .slice(0, -1) |
There was a problem hiding this comment.
Minor: intercepting route directories like (.)shop and (..)settings use the ( prefix but don't end with ), so they pass through the route-group filter here and contribute to the visible prefix. That seems correct — intercepting routes are real route segments — just noting it for future readers.
|
Review submitted. Here's a summary of what I reviewed and the feedback: Verdict: Approved The PR correctly narrows the App Router dev watcher to only invalidate on files that actually affect routing — structure files ( Three minor comments posted:
|
What this changes
Stops the App Router dev watcher from invalidating the generated route entry and sending a full reload when ordinary colocated files are added or removed under
app/, such asapp/components/Button.tsx.The watcher now invalidates only files that can affect App Router structure or generated metadata route entries: route conventions such as
page,route,layout,template,loading,error, access fallback files, rootglobal-error, and vinext metadata route files.Fixes #1013.
Why
Next.js explicitly treats arbitrary project files in
app/as safely colocated and non-routable. The source backs that up in the route discovery matcher:createValidFileMatcherbuilds leaf-only app route matchersvinext was using only the broad page extension regex for
app/watcher add/unlink events, so any.ts,.tsx,.js, or.jsxfile underapp/looked route-affecting and triggered a full reload.Approach
Add a small pure predicate,
shouldInvalidateAppRouteFile, that mirrors vinext's current App Router and metadata discovery boundaries:app/directory, fixing sibling-prefix cases likeapp-utils/_foldersThe watcher still keeps the broad extension behavior for the Pages Router, since every matching file under
pages/is routable by design unless filtered elsewhere.Validation
vp test run tests/file-matcher.test.tsvp test run tests/file-matcher.test.ts tests/page-extensions-routing.test.tsvp check tests/file-matcher.test.ts packages/vinext/src/server/dev-route-files.ts packages/vinext/src/index.ts packages/vinext/src/server/metadata-routes.tsvp run vinext#buildThe package build completed successfully with the existing virtual-module externalization warnings for
private-next-instrumentation-client,virtual:vinext-rsc-entry, andvirtual:vite-rsc/client-references.Risks / follow-ups
The metadata side intentionally follows vinext's current
scanMetadataFilesbehavior, not every future Next.js metadata convention. If vinext expands metadata route support later, this predicate should be updated with the same source of truth.