Skip to content

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

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

fix(build): emptyOutDir should happen for watch rebuilds#22207
sapphi-red merged 3 commits intovitejs: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
Copy link
Copy Markdown
Member

bluwy commented Apr 12, 2026

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.

@hyf0
Copy link
Copy Markdown
Contributor

hyf0 commented Apr 13, 2026

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
Copy link
Copy Markdown
Member

bluwy commented Apr 13, 2026

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