feat: external directories in physical() virtual route mounts#7196
Open
feat: external directories in physical() virtual route mounts#7196
physical() virtual route mounts#7196Conversation
Contributor
|
View your CI Pipeline Execution ↗ for commit c207ebb
☁️ Nx Cloud last updated this comment at |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds support for mounting physical route directories located outside the configured routes directory by exposing generator physical directories, registering external-directory file watchers in the router plugin (Vite/webpack/rspack), and adding tests/fixtures that validate route-tree updates from external files. Changes
Sequence Diagram(s)sequenceDiagram
participant FS as File System
participant Watcher as Chokidar (watcher)
participant Plugin as Router Plugin
participant Generator as Generator
participant Output as Generated Route Output
FS->>Watcher: add / change / unlink external file
Watcher->>Plugin: notify(event, file)
Plugin->>Generator: generate({ file, event }) / consult getPhysicalDirectories()
Generator->>Output: write updated routeTree file
Plugin->>Output: trigger dev-server HMR or plugin hooks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Changeset Version Preview2 package(s) bumped directly, 7 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/router-plugin/src/core/router-generator-plugin.ts (1)
147-150: Pre-existing:tapwith async callback doesn't await the close operations.The
watchClosehook uses.tap()with an async callback, buttapdoesn't await the returned promise. This meanshandle.close()andexternalHandle.close()may not complete before the process exits.This is a pre-existing pattern in the codebase (not introduced by this PR), but worth noting for future improvement.
♻️ Optional: Use tapPromise if the hook supports it
If
watchClosesupportstapPromise, you could use that instead:-compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { - if (handle) await handle.close() - if (externalHandle) await externalHandle.close() -}) +compiler.hooks.watchClose.tapPromise(PLUGIN_NAME, async () => { + if (handle) await handle.close() + if (externalHandle) await externalHandle.close() +})Alternatively, use synchronous tap without await if the close operations are fire-and-forget:
-compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { - if (handle) await handle.close() - if (externalHandle) await externalHandle.close() -}) +compiler.hooks.watchClose.tap(PLUGIN_NAME, () => { + handle?.close() + externalHandle?.close() +})Also applies to: 188-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/src/core/router-generator-plugin.ts` around lines 147 - 150, The watchClose hook currently uses compiler.hooks.watchClose.tap with an async callback, but tap does not await promises so handle.close() and externalHandle.close() may not finish; update the hook to use compiler.hooks.watchClose.tapPromise (or the synchronous tap pattern if promises can't be awaited) and move the await calls into that promise-returning callback so that await handle.close() and await externalHandle.close() are properly awaited; change both occurrences referencing compiler.hooks.watchClose.tap, handle.close, and externalHandle.close accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/router-plugin/src/core/router-generator-plugin.ts`:
- Around line 147-150: The watchClose hook currently uses
compiler.hooks.watchClose.tap with an async callback, but tap does not await
promises so handle.close() and externalHandle.close() may not finish; update the
hook to use compiler.hooks.watchClose.tapPromise (or the synchronous tap pattern
if promises can't be awaited) and move the await calls into that
promise-returning callback so that await handle.close() and await
externalHandle.close() are properly awaited; change both occurrences referencing
compiler.hooks.watchClose.tap, handle.close, and externalHandle.close
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d63eecd-9880-4049-a3a7-7b1c42498b91
📒 Files selected for processing (9)
.changeset/virtual-route-external-physical.mdpackages/router-generator/src/generator.tspackages/router-generator/tests/generator.test.tspackages/router-generator/tests/generator/virtual-physical-external-abs/external-target/bar.tsxpackages/router-generator/tests/generator/virtual-physical-external-abs/external-target/foo.tsxpackages/router-generator/tests/generator/virtual-physical-external-abs/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-physical-external-abs/routes/__root.tsxpackages/router-generator/tests/generator/virtual-physical-external-abs/routes/index.tsxpackages/router-plugin/src/core/router-generator-plugin.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router-plugin/src/core/router-generator-plugin.ts`:
- Around line 18-20: The current startsWith check can misclassify sibling paths;
change the filter in the generator pipeline that uses
generator.getPhysicalDirectories() to use Node's path.relative to determine true
subdirectory relationships: for each dir compute const rel =
path.relative(routesDirectoryPath, dir) and treat dir as internal if rel === ''
or !rel.startsWith('..') (i.e., it's the same or a descendant), so keep only
directories where rel.startsWith('..') (or otherwise not a descendant). Import
path and replace the startsWith check with this path.relative-based test in the
function that performs the filtering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4a6aedb-d291-4819-a522-3677d4da16b5
📒 Files selected for processing (5)
.changeset/virtual-route-external-physical.mdpackages/router-generator/tests/generator/virtual-physical-external-abs/external-target/bar.tsxpackages/router-generator/tests/generator/virtual-physical-external-abs/external-target/foo.tsxpackages/router-plugin/src/core/router-generator-plugin.tspackages/router-plugin/tests/router-generator-plugin-watcher.test.ts
✅ Files skipped from review due to trivial changes (3)
- .changeset/virtual-route-external-physical.md
- packages/router-generator/tests/generator/virtual-physical-external-abs/external-target/bar.tsx
- packages/router-generator/tests/generator/virtual-physical-external-abs/external-target/foo.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router-plugin/src/core/router-generator-plugin.ts`:
- Around line 186-189: The watchClose hook is registered with
compiler.hooks.watchClose.tap using an async callback (referenced symbols:
compiler.hooks.watchClose.tap, PLUGIN_NAME, handle, externalHandle), which means
the hook won't wait for the async work; change the registration to use
tapPromise so the returned Promise is awaited: replace
compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { if (handle) await
handle.close(); if (externalHandle) await externalHandle.close(); }) with
compiler.hooks.watchClose.tapPromise(PLUGIN_NAME, async () => { ... }) so
handle.close() and externalHandle.close() are properly awaited.
- Around line 148-151: The callback passed to compiler.hooks.watchClose.tap
(where PLUGIN_NAME is used) is currently declared async so handle.close() and
externalHandle.close() return promises that won't be awaited by the synchronous
tap; change the callback to a synchronous function and either fire-and-forget
the promises or explicitly void them and handle errors: inside the
compiler.hooks.watchClose.tap(PLUGIN_NAME, ...) callback call void
handle?.close()?.catch(err => /* log or ignore */) and void
externalHandle?.close()?.catch(err => /* log or ignore */) (or otherwise call
handle.close() and externalHandle.close() and attach .catch handlers) so cleanup
is not misrepresented as awaited.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70874636-449c-4225-a349-6a7f8f6b7af5
📒 Files selected for processing (1)
packages/router-plugin/src/core/router-generator-plugin.ts
Comment on lines
148
to
151
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | ||
| if (handle) { | ||
| await handle.close() | ||
| } | ||
| if (handle) await handle.close() | ||
| if (externalHandle) await externalHandle.close() | ||
| }) |
Contributor
There was a problem hiding this comment.
async callback in synchronous tap hook won't await cleanup.
The watchClose hook uses .tap() which is synchronous and won't wait for the async callback's promises. The async keyword here is misleading—handle.close() returns a Promise that won't be awaited.
This could cause watchers to not be fully closed during rapid dev server restarts.
Proposed fix: use fire-and-forget or void the promises explicitly
compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => {
- if (handle) await handle.close()
- if (externalHandle) await externalHandle.close()
+ if (handle) void handle.close()
+ if (externalHandle) void externalHandle.close()
})Or alternatively, if cleanup needs to be awaited, investigate if rspack supports an async shutdown hook.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | |
| if (handle) { | |
| await handle.close() | |
| } | |
| if (handle) await handle.close() | |
| if (externalHandle) await externalHandle.close() | |
| }) | |
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | |
| if (handle) void handle.close() | |
| if (externalHandle) void externalHandle.close() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-plugin/src/core/router-generator-plugin.ts` around lines 148
- 151, The callback passed to compiler.hooks.watchClose.tap (where PLUGIN_NAME
is used) is currently declared async so handle.close() and
externalHandle.close() return promises that won't be awaited by the synchronous
tap; change the callback to a synchronous function and either fire-and-forget
the promises or explicitly void them and handle errors: inside the
compiler.hooks.watchClose.tap(PLUGIN_NAME, ...) callback call void
handle?.close()?.catch(err => /* log or ignore */) and void
externalHandle?.close()?.catch(err => /* log or ignore */) (or otherwise call
handle.close() and externalHandle.close() and attach .catch handlers) so cleanup
is not misrepresented as awaited.
Comment on lines
186
to
189
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | ||
| if (handle) { | ||
| await handle.close() | ||
| } | ||
| if (handle) await handle.close() | ||
| if (externalHandle) await externalHandle.close() | ||
| }) |
Contributor
There was a problem hiding this comment.
Same async/tap mismatch as rspack.
Identical issue: the synchronous tap hook won't await the async callback's promises.
Proposed fix
compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => {
- if (handle) await handle.close()
- if (externalHandle) await externalHandle.close()
+ if (handle) void handle.close()
+ if (externalHandle) void externalHandle.close()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | |
| if (handle) { | |
| await handle.close() | |
| } | |
| if (handle) await handle.close() | |
| if (externalHandle) await externalHandle.close() | |
| }) | |
| compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { | |
| if (handle) void handle.close() | |
| if (externalHandle) void externalHandle.close() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-plugin/src/core/router-generator-plugin.ts` around lines 186
- 189, The watchClose hook is registered with compiler.hooks.watchClose.tap
using an async callback (referenced symbols: compiler.hooks.watchClose.tap,
PLUGIN_NAME, handle, externalHandle), which means the hook won't wait for the
async work; change the registration to use tapPromise so the returned Promise is
awaited: replace compiler.hooks.watchClose.tap(PLUGIN_NAME, async () => { if
(handle) await handle.close(); if (externalHandle) await externalHandle.close();
}) with compiler.hooks.watchClose.tapPromise(PLUGIN_NAME, async () => { ... })
so handle.close() and externalHandle.close() are properly awaited.
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts`:
- Around line 71-74: The test 'regenerates routeTree on add/remove in an
external physical mount' has a 20_000 ms timeout that is smaller than the
worst-case duration because waitUntil is invoked multiple times with 10s
defaults plus settle delays; increase the test timeout in the it(...) call to a
safe upper bound (for example 60_000 ms) so CI won't prematurely fail, and keep
the change localized to the test declaration in
router-generator-plugin-watcher.test.ts where the test name and timeout object
are defined.
- Around line 6-9: Reorder the imports so they satisfy the import/order rule:
move the type import "import type { ViteDevServer } from 'vite'" to come after
the local/project imports (the imports of physical, rootRoute from
'@tanstack/virtual-file-routes' and tanstackRouterGenerator from '../src/vite').
Locate the symbols ViteDevServer, physical, rootRoute, and
tanstackRouterGenerator and adjust the import block ordering so external/local
groups follow the project's ESLint import/order configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd6900b1-1d30-43ee-8ff5-33c2986ef6fc
📒 Files selected for processing (1)
packages/router-plugin/tests/router-generator-plugin-watcher.test.ts
Comment on lines
+6
to
+9
| import type { ViteDevServer } from 'vite' | ||
|
|
||
| import { physical, rootRoute } from '@tanstack/virtual-file-routes' | ||
| import { tanstackRouterGenerator } from '../src/vite' |
Contributor
There was a problem hiding this comment.
Fix import ordering to satisfy import/order.
Line 6 currently violates the configured import order (vite type import must come after the local import), which can fail lint/CI.
Proposed diff
import { afterEach, beforeEach, describe, it } from 'vitest'
import { createServer } from 'vite'
-import type { ViteDevServer } from 'vite'
import { physical, rootRoute } from '@tanstack/virtual-file-routes'
import { tanstackRouterGenerator } from '../src/vite'
+import type { ViteDevServer } from 'vite'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { ViteDevServer } from 'vite' | |
| import { physical, rootRoute } from '@tanstack/virtual-file-routes' | |
| import { tanstackRouterGenerator } from '../src/vite' | |
| import { afterEach, beforeEach, describe, it } from 'vitest' | |
| import { createServer } from 'vite' | |
| import { physical, rootRoute } from '@tanstack/virtual-file-routes' | |
| import { tanstackRouterGenerator } from '../src/vite' | |
| import type { ViteDevServer } from 'vite' |
🧰 Tools
🪛 ESLint
[error] 6-6: vite type import should occur after import of ../src/vite
(import/order)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts` around
lines 6 - 9, Reorder the imports so they satisfy the import/order rule: move the
type import "import type { ViteDevServer } from 'vite'" to come after the
local/project imports (the imports of physical, rootRoute from
'@tanstack/virtual-file-routes' and tanstackRouterGenerator from '../src/vite').
Locate the symbols ViteDevServer, physical, rootRoute, and
tanstackRouterGenerator and adjust the import block ordering so external/local
groups follow the project's ESLint import/order configuration.
Comment on lines
+71
to
+74
| it( | ||
| 'regenerates routeTree on add/remove in an external physical mount', | ||
| { timeout: 20_000 }, | ||
| async () => { |
Contributor
There was a problem hiding this comment.
Test timeout is lower than worst-case polling duration.
This test can run longer than 20s (waitUntil is called 3 times with default 10s each, plus settle delays), so CI may timeout before helper-level failures surface.
Proposed diff
it(
'regenerates routeTree on add/remove in an external physical mount',
- { timeout: 20_000 },
+ { timeout: 35_000 },
async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it( | |
| 'regenerates routeTree on add/remove in an external physical mount', | |
| { timeout: 20_000 }, | |
| async () => { | |
| it( | |
| 'regenerates routeTree on add/remove in an external physical mount', | |
| { timeout: 35_000 }, | |
| async () => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts` around
lines 71 - 74, The test 'regenerates routeTree on add/remove in an external
physical mount' has a 20_000 ms timeout that is smaller than the worst-case
duration because waitUntil is invoked multiple times with 10s defaults plus
settle delays; increase the test timeout in the it(...) call to a safe upper
bound (for example 60_000 ms) so CI won't prematurely fail, and keep the change
localized to the test declaration in router-generator-plugin-watcher.test.ts
where the test name and timeout object are defined.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/router-plugin/tests/router-generator-plugin-watcher.test.ts (2)
71-74:⚠️ Potential issue | 🟠 MajorIncrease test timeout above the combined polling upper bound.
Line 73 sets
30_000, but this test can legitimately spend ~3 * 20_000 + 2 * 1_000before helper-level failures surface. The outer timeout may preempt the real assertion path in slower CI.Proposed diff
it( 'regenerates routeTree on add/remove in an external physical mount', - { timeout: 30_000 }, + { timeout: 70_000 }, async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts` around lines 71 - 74, The test "regenerates routeTree on add/remove in an external physical mount" uses a 30_000ms outer timeout which is too low for the combined helper polling max (~62_000ms); update the timeout in the it(...) call (the test declaring string 'regenerates routeTree on add/remove in an external physical mount') to a larger value (e.g., 90_000 or at least 70_000) so the outer Mocha timeout exceeds the combined polling upper bound and prevents premature test termination.
6-9:⚠️ Potential issue | 🟡 MinorFix import grouping to satisfy
import/order.
import type { ViteDevServer } from 'vite'is still positioned before local imports, which keeps the lint error active.Proposed diff
import { afterEach, beforeEach, describe, it } from 'vitest' import { createServer } from 'vite' -import type { ViteDevServer } from 'vite' import { physical, rootRoute } from '@tanstack/virtual-file-routes' import { tanstackRouterGenerator } from '../src/vite' +import type { ViteDevServer } from 'vite'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts` around lines 6 - 9, Move the type import for ViteDevServer so it follows the other external imports (or group it with them) instead of being placed before local imports; specifically, reorder the top-of-file imports so "import type { ViteDevServer } from 'vite'" appears alongside the external imports that include the package imports and after which the local imports "physical", "rootRoute" and "tanstackRouterGenerator" remain grouped together, ensuring the import/order linter rule is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/router-plugin/tests/router-generator-plugin-watcher.test.ts`:
- Around line 71-74: The test "regenerates routeTree on add/remove in an
external physical mount" uses a 30_000ms outer timeout which is too low for the
combined helper polling max (~62_000ms); update the timeout in the it(...) call
(the test declaring string 'regenerates routeTree on add/remove in an external
physical mount') to a larger value (e.g., 90_000 or at least 70_000) so the
outer Mocha timeout exceeds the combined polling upper bound and prevents
premature test termination.
- Around line 6-9: Move the type import for ViteDevServer so it follows the
other external imports (or group it with them) instead of being placed before
local imports; specifically, reorder the top-of-file imports so "import type {
ViteDevServer } from 'vite'" appears alongside the external imports that include
the package imports and after which the local imports "physical", "rootRoute"
and "tanstackRouterGenerator" remain grouped together, ensuring the import/order
linter rule is satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d036106-6692-49f8-9966-982e0d2e14ea
📒 Files selected for processing (1)
packages/router-plugin/tests/router-generator-plugin-watcher.test.ts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We exclude the tests/tmp-watcher-*/** glob from the TypeScript project so that leftover temp directories from interrupted watcher test runs no longer cause tsc type errors. We also switch the Vite test server to polling (usePolling: true, interval: 100) and raise the per-call waitUntil and test timeouts so that file-change events are detected reliably on CI container filesystems that don't support inotify.
Tip
✅ We verified this fix by re-running @tanstack/router-plugin:test:unit, @tanstack/router-plugin:test:types.
Suggested Fix changes
diff --git a/packages/router-plugin/src/core/router-generator-plugin.ts b/packages/router-plugin/src/core/router-generator-plugin.ts
index a6aa217754..df5aab1267 100644
--- a/packages/router-plugin/src/core/router-generator-plugin.ts
+++ b/packages/router-plugin/src/core/router-generator-plugin.ts
@@ -93,23 +93,24 @@ export const unpluginRouterGeneratorFactory: UnpluginFactory<
initConfigAndGenerator({ root: config.root })
await generate()
},
- configureServer(server) {
+ async configureServer(server) {
const external = getExternalPhysicalDirs(
generator,
getRoutesDirectoryPath(),
)
if (external.length === 0) return
- for (const dir of external) {
- server.watcher.add(dir)
- }
- const onEvent =
- (event: 'create' | 'update' | 'delete') => (file: string) => {
- if (!external.some((dir) => isInside(dir, file))) return
- void generate({ file, event })
- }
- server.watcher.on('add', onEvent('create'))
- server.watcher.on('change', onEvent('update'))
- server.watcher.on('unlink', onEvent('delete'))
+ // Use a dedicated chokidar watcher for external dirs. Adding paths to
+ // server.watcher when those paths are already inside the Vite root
+ // causes chokidar internal-state conflicts and 'add'/'unlink' events
+ // are silently dropped. A fresh watcher mirrors the rspack/webpack
+ // approach and avoids the conflict entirely.
+ const chokidar = await import('chokidar')
+ const handle = chokidar
+ .watch(external, { ignoreInitial: true })
+ .on('add', (file) => generate({ file, event: 'create' }))
+ .on('change', (file) => generate({ file, event: 'update' }))
+ .on('unlink', (file) => generate({ file, event: 'delete' }))
+ server.httpServer?.once('close', () => void handle.close())
},
},
rspack(compiler) {
diff --git a/packages/router-plugin/tests/router-generator-plugin-watcher.test.ts b/packages/router-plugin/tests/router-generator-plugin-watcher.test.ts
index 1fce84bb63..12ce84f1e3 100644
--- a/packages/router-plugin/tests/router-generator-plugin-watcher.test.ts
+++ b/packages/router-plugin/tests/router-generator-plugin-watcher.test.ts
@@ -75,7 +75,7 @@ describe('router-generator-plugin vite watcher', () => {
it(
'regenerates routeTree on add/remove in an external physical mount',
- { timeout: 30_000 },
+ { timeout: 60_000 },
async () => {
server = await createServer({
root: fixtureDir,
@@ -106,8 +106,9 @@ describe('router-generator-plugin vite watcher', () => {
const betaPath = path.join(externalDir, 'beta.tsx')
await writeFile(betaPath, makeRouteFile('/ext/beta'))
await settle()
- await waitUntil(() =>
- routeTreeIncludes(generatedRouteTree, "'/ext/beta'"),
+ await waitUntil(
+ () => routeTreeIncludes(generatedRouteTree, "'/ext/beta'"),
+ { timeoutMs: 20_000 },
)
await rm(betaPath)
@@ -115,6 +116,7 @@ describe('router-generator-plugin vite watcher', () => {
await waitUntil(
async () =>
!(await routeTreeIncludes(generatedRouteTree, "'/ext/beta'")),
+ { timeoutMs: 20_000 },
)
},
)
diff --git a/packages/router-plugin/tsconfig.json b/packages/router-plugin/tsconfig.json
index 0e74c8c099..66623a3f40 100644
--- a/packages/router-plugin/tsconfig.json
+++ b/packages/router-plugin/tsconfig.json
@@ -1,7 +1,11 @@
{
"extends": "../../tsconfig.json",
"include": ["src", "vite.config.ts", "tests"],
- "exclude": ["tests/**/test-files/**", "tests/**/snapshots/**"],
+ "exclude": [
+ "tests/**/test-files/**",
+ "tests/**/snapshots/**",
+ "tests/tmp-watcher-*/**"
+ ],
"compilerOptions": {
"jsx": "react-jsx",
"types": ["node"]
Or Apply changes locally with:
npx nx-cloud apply-locally Ry5s-ZV31
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow
physical('/prefix', dir)to mount a directory outsideroutesDirectory.Useful in monorepos where one package owns the pages and another composes them. Previously I had a script symlinking files into a gitignored
_pages/dir as a workaround.The generator already handled external paths correctly. The missing piece is file watching. Added
Generator.getPhysicalDirectories()and extended the vite/rspack/webpack watchers to track external mounts so adds/removes regeneraterouteTree.gen.ts.Example:
My use-case is that I have a shared frontend-core package with all the routes, UI components etc, which use just tanstack router.
I then extend that from two different client packages:
I'd like to be able to have the virtual routes from my client packages be able to watch the routes in the frontend-core package
Summary by CodeRabbit
New Features
Tests