Skip to content

fix(build): emptyOutDir should happen for watch rebuilds - #22207

Merged
sapphi-red merged 3 commits into
vitejs:mainfrom
sapphi-red:fix/build-watch-mode-cleanup
Apr 14, 2026
Merged

sapphi-red merged 3 commits into
vitejs:mainfrom
sapphi-red:fix/build-watch-mode-cleanup

Conversation

@sapphi-red

Copy link
Copy Markdown
Member

Rollup seems to run options hook for rebuilds, but Rolldown doesn't seem to.

fixes #22205
refs rolldown/rolldown#9053

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: build labels Apr 10, 2026
@sapphi-red
sapphi-red marked this pull request as draft April 10, 2026 04:23
@sapphi-red
sapphi-red marked this pull request as ready for review April 10, 2026 05:16
@bluwy

bluwy commented Apr 12, 2026

Copy link
Copy Markdown
Member

Why choose not to run it for optimization? It lends Rollup's plugin interface so it should prioritize compatibility IMO, and optimizations come additively.

@sapphi-red

Copy link
Copy Markdown
Member Author

I assumed there's a significant performance difference. @IWANABETHATGUY Would you elaborate the rationale behind the decision?

@sapphi-red

Copy link
Copy Markdown
Member Author

I forgot to mention but we still need to use watchChange instead of options because options is called for each output in Rolldown (while Rollup only calls it once). This is due to its architecture.

@hyfdev

hyfdev commented Apr 13, 2026 •

Copy link
Copy Markdown
Contributor

Some notes:

  • Calling options hook require rust to call js. It's less performant and I don't see any necessity here(will explain below)
  • If every rebuild in watch is a fresh rebuild, then maybe it makes some sense to call options hook to allow users to do some customization. But this isn't true, I think even for Rollup, this isn't not a good idea. It causes buggy behavior if users try to changes options during watch.
  • Considering the incremental build feature of Rolldown,
    • changing options will lead to a fresh rebuild to recreate everything. This could be supported, but not worth it in paratice.
    • Options is already sealed for watch mode, it shouldn't be allowed to change.

@bluwy

bluwy commented Apr 13, 2026

Copy link
Copy Markdown
Member

I forgot to mention but we still need to use watchChange instead of options because options is called for each output in Rolldown (while Rollup only calls it once). This is due to its architecture.

Ok yeah that makes sense to me, not blocking this PR then.

  • If every rebuild in watch is a fresh rebuild, then maybe it makes some sense to call options hook to allow users to do some customization. But this isn't true, I think even for Rollup, this isn't not a good idea. It causes buggy behavior if users try to changes options during watch.

  • Considering the incremental build feature of Rolldown,

    • changing options will lead to a fresh rebuild to recreate everything. This could be supported, but not worth it in paratice.
    • Options is already sealed for watch mode, it shouldn't be allowed to change.

Thanks, I think all that makes sense to me. It is indeed rare for the options hook to change between watch reruns, and should be fine if it's "cached" between runs.

@sapphi-red
sapphi-red merged commit ee52267 into vitejs:main Apr 14, 2026
24 of 25 checks passed
@sapphi-red
sapphi-red deleted the fix/build-watch-mode-cleanup branch April 14, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: build p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vite build --watch doesn't clean up old files

3 participants