Skip to content

feat(build): watch publicDir and sync changes to outDir in watch mode#22667

Open
Vladexy88x wants to merge 1 commit into
vitejs:mainfrom
Vladexy88x:fix/build-watch-public-dir
Open

feat(build): watch publicDir and sync changes to outDir in watch mode#22667
Vladexy88x wants to merge 1 commit into
vitejs:mainfrom
Vladexy88x:fix/build-watch-public-dir

Conversation

@Vladexy88x

Copy link
Copy Markdown

Description

When running vite build --watch, the public directory is only copied once when the build starts. Any later change to a file in public/ is never reflected in the out dir until a source file happens to trigger a rebuild.

This PR watches publicDir with chokidar during build watch mode and syncs changes directly into the resolved out dirs:

  • add / change events copy the file into every resolved out dir
  • unlink / unlinkDir events remove the corresponding file/directory
  • no rebuild is triggered, since public assets bypass the bundler (mirrors how the dev server serves public/ straight from disk)

fixes #18655

Implementation notes

  • The watcher reuses resolvedChokidarOptions (same options the dev server passes to chokidar), so ignoreInitial: true prevents a duplicate startup copy and the out dirs are ignored when emptyOutDir is on. It is gated on the same conditions as the initial copy in prepareOutDirPlugin (publicDir set, build.copyPublicDir, dir exists), and is closed via the rolldown watcher's close event.
  • awaitWriteFinish: { stabilityThreshold: 100, pollInterval: 10 } is set (user-overridable via build.watch.chokidar). Without it, a file written with a truncate-then-write pattern (e.g. another vite build --watch process emitting a service worker into public/, which is exactly the reporter's setup) gets copied half-written at the moment of truncation, and chokidar's internal per-file event throttling drops the follow-up event, so the out dir keeps a 0-byte file indefinitely. Reproduced on both Windows (NTFS) and Linux (ext4/inotify) with truncate→write gaps of 1–20 ms.
  • Sync errors (e.g. a source file locked or deleted between the event and the copy) are logged instead of crashing the watch process, since a synchronous throw in a chokidar listener becomes an uncaught exception.

Testing

Verified manually against the built package on Windows and the chokidar layer on Linux:

  • add / change / delete of files and directories in public/ sync to dist/ without triggering a rebuild
  • truncate-then-write with 1, 10, 50 and 250 ms gaps all end with the correct content in dist/ (the 1–20 ms cases fail without awaitWriteFinish)
  • closing the watcher via the JS API also closes the public watcher (process exits cleanly, no leak)

No automated test is included yet: reliably exercising the mid-write race requires a deliberate truncate-then-delayed-write pattern (a plain writeFileSync of a small file doesn't hit it), and I'd like maintainer input on whether a playground build-watch test with that pattern is acceptable, since it is timing-sensitive. Happy to add one.

🤖 Generated with Claude Code

When running `vite build --watch`, the public directory was only copied
once at startup. Watch publicDir with chokidar and sync added, changed
and deleted files directly into the resolved outDirs, without triggering
a full rebuild since public assets bypass the bundler.

Uses awaitWriteFinish so files truncated by an in-progress write are not
copied half-written, and logs sync errors instead of crashing the watch
process.

fixes vitejs#18655

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@daltino daltino left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: feat(build): watch publicDir and sync changes to outDir in watch mode

This is a great quality-of-life fix for a real pain point in --watch mode! The intent is clear and the approach is sensible. I have a few suggestions before this lands.


✅ What looks good

  • Correct guard: publicDir && options.copyPublicDir && fs.existsSync(publicDir) mirrors the existing copy-on-start logic nicely.
  • Lazy-importing chokidar with await import('chokidar') is consistent with how the rest of the watch code works.
  • Scoping the watcher to publicDir only keeps the blast radius small.

🟡 Issues & Suggestions

1. Watcher is never closed (potential resource leak)

The most important issue in the visible patch. The watcher instance returned by chokidar.watch() should be closed when the Rollup watcher closes, otherwise the process can hang on Ctrl+C and file descriptors leak.

// after creating the chokidar watcher
watcher.on('error', (err) => config.logger.error(`[public watcher] ${err}`))

// close it when the rollup watcher is closed
rollupWatcherObj.on('close', () => {
  chokidarWatcher.close()
})

(Naming it chokidarWatcher or similar avoids shadowing the outer watcher variable that Vite already uses.)


2. unlink / unlinkDir events are likely missing

The description mentions add/change copy events, but deleting a file from publicDir during --watch should also remove it from outDirs. Without handling unlink and unlinkDir, the out dir silently diverges from the source.

chokidarWatcher.on('unlink', (file) => {
  const relative = path.relative(publicDir, file)
  for (const outDir of outDirsArray) {
    fs.rm(path.join(outDir, relative), { force: true }, () => {})
  }
})

3. outDirsArray might include paths inside publicDir

If someone accidentally configures outDir to be a subdirectory of publicDir, the watcher will pick up its own writes and loop. It's worth adding the same guard Vite uses elsewhere:

const safeOutDirs = outDirsArray.filter(
  (outDir) => !outDir.startsWith(publicDir + path.sep),
)

4. Symlinks and atomic saves

Chokidar's default config uses usePolling: false and doesn't follow symlinks. That's usually fine, but it's worth confirming the options passed here match what the main dev-server watcher uses (config.server.watch) so behaviour is consistent across modes. At minimum, document the deliberate choice.


5. No tests added

There are existing build --watch integration tests under playground/. A test that:

  1. Starts a watch build,
  2. Writes a new file to public/,
  3. Asserts it appears in outDir without modifying any source file

…would prevent regressions and is the main thing missing before this is merge-ready.


6. Minor: prefer fs.promises.copyFile for consistency

The rest of build.ts uses async FS operations. If the copy inside the handler uses the synchronous API, it can block the event loop during large asset copies. Using fs.promises.copyFile (with a .catch(logger.error)) keeps things non-blocking.


Summary

Resource leak (watcher not closed) 🔴 blocker
Missing unlink/unlinkDir handling 🟠 should fix
outDirpublicDir loop risk 🟡 edge case
No tests 🟠 should fix
Async copy 🟢 nice-to-have

Great start — with the watcher cleanup and unlink handling addressed this will be a solid addition!

@Vladexy88x

Copy link
Copy Markdown
Author

Review: feat(build): watch publicDir and sync changes to outDir in watch mode

This is a great quality-of-life fix for a real pain point in --watch mode! The intent is clear and the approach is sensible. I have a few suggestions before this lands.

✅ What looks good

  • Correct guard: publicDir && options.copyPublicDir && fs.existsSync(publicDir) mirrors the existing copy-on-start logic nicely.
  • Lazy-importing chokidar with await import('chokidar') is consistent with how the rest of the watch code works.
  • Scoping the watcher to publicDir only keeps the blast radius small.

🟡 Issues & Suggestions

1. Watcher is never closed (potential resource leak)

The most important issue in the visible patch. The watcher instance returned by chokidar.watch() should be closed when the Rollup watcher closes, otherwise the process can hang on Ctrl+C and file descriptors leak.

// after creating the chokidar watcher
watcher.on('error', (err) => config.logger.error(`[public watcher] ${err}`))

// close it when the rollup watcher is closed
rollupWatcherObj.on('close', () => {
  chokidarWatcher.close()
})

(Naming it chokidarWatcher or similar avoids shadowing the outer watcher variable that Vite already uses.)

2. unlink / unlinkDir events are likely missing

The description mentions add/change copy events, but deleting a file from publicDir during --watch should also remove it from outDirs. Without handling unlink and unlinkDir, the out dir silently diverges from the source.

chokidarWatcher.on('unlink', (file) => {
  const relative = path.relative(publicDir, file)
  for (const outDir of outDirsArray) {
    fs.rm(path.join(outDir, relative), { force: true }, () => {})
  }
})

3. outDirsArray might include paths inside publicDir

If someone accidentally configures outDir to be a subdirectory of publicDir, the watcher will pick up its own writes and loop. It's worth adding the same guard Vite uses elsewhere:

const safeOutDirs = outDirsArray.filter(
  (outDir) => !outDir.startsWith(publicDir + path.sep),
)

4. Symlinks and atomic saves

Chokidar's default config uses usePolling: false and doesn't follow symlinks. That's usually fine, but it's worth confirming the options passed here match what the main dev-server watcher uses (config.server.watch) so behaviour is consistent across modes. At minimum, document the deliberate choice.

5. No tests added

There are existing build --watch integration tests under playground/. A test that:

  1. Starts a watch build,
  2. Writes a new file to public/,
  3. Asserts it appears in outDir without modifying any source file

…would prevent regressions and is the main thing missing before this is merge-ready.

6. Minor: prefer fs.promises.copyFile for consistency

The rest of build.ts uses async FS operations. If the copy inside the handler uses the synchronous API, it can block the event loop during large asset copies. Using fs.promises.copyFile (with a .catch(logger.error)) keeps things non-blocking.

Summary

Resource leak (watcher not closed) 🔴 blocker
Missing unlink/unlinkDir handling 🟠 should fix
outDirpublicDir loop risk 🟡 edge case
No tests 🟠 should fix
Async copy 🟢 nice-to-have
Great start — with the watcher cleanup and unlink handling addressed this will be a solid addition!

Thanks for the thorough review @daltino, appreciate you digging in! 🙏

A few of these are actually already handled in the diff — I think the "visible patch" you reviewed may have been truncated, so let me point at the relevant lines:

#1 — watcher close (blocker): Handled. The chokidar watcher is closed on the rollup watcher's close event:
watcher.on('close', () => publicWatcher.close())

#2 — unlink / unlinkDir: Both are wired up. unlink routes through syncFile(file, true) which does fs.rmSync(dest, { force: true }), and unlinkDir removes the directory recursively:
.on('unlink', (file) => syncFile(file, true))
.on('unlinkDir', (dir) => { /* rmSync(..., { recursive: true, force: true }) */ })

#4 — chokidar options consistency: The watcher spreads ...resolvedChokidarOptions — the exact options the dev server resolves from config.server.watch — so behaviour is consistent across modes. awaitWriteFinish is layered on top specifically to handle the atomic/truncate-then-write case (covered in the PR description), and ignoreInitial: true from those options avoids a duplicate startup copy.

On the points that are genuinely open:

#3 — outDir inside publicDir: Fair edge case. When emptyOutDir is on, the out dirs are already in the ignore list via resolvedChokidarOptions, so the loop can't happen there — but you're right that with emptyOutDir: false and an outDir nested under publicDir it could re-sync its own writes. The copy is idempotent so it wouldn't spin forever, but I'm happy to add an explicit filter to be safe. Will do.

#5 — tests: Agreed this is the main gap. I called it out in the description: reliably exercising the mid-write race needs a deliberate truncate-then-delayed-write, which is timing-sensitive. A simpler playground test (write to public/, assert it appears in outDir without touching source) is straightforward and regression-proofs the common path — I'll add that. The race-specific test I'd still like maintainer input on.

#6 — async copy: The sync API here is deliberate — a throw inside a chokidar listener becomes an uncaught exception, so everything's wrapped in try/catch and logged. I can switch to fs.promises.copyFile().catch(...) to avoid blocking on large assets; will weigh that against keeping the error handling simple.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build --watch doesn't watch files in public

2 participants