New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Watch files onbuild, even if build fails #3081
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3081 +/- ##
=========================================
+ Coverage 88.95% 89.06% +0.1%
=========================================
Files 165 165
Lines 5721 5713 -8
Branches 1742 1738 -4
=========================================
- Hits 5089 5088 -1
+ Misses 385 383 -2
+ Partials 247 242 -5
Continue to review full report at Codecov.
|
* Do not add files to Graph.watchFiles after all files have been loaded but immediately when starting to load them so that the same watchFile logic can be used for successful and unsuccessful builds (and files added by plugins are not ignored on errors) * Do not unwatch files that have been removed if an error occurred but only when a build has succeeded * Do not re-watch files of the previous build when an error occurred as those are still being watched; just additionally watch any files attached to the error * Make sure that watching a file automatically adds it to the list of "watched" files and only reset this list when clearing unneeded files * Remove logic to filter for absolute ids for now to restore the old behaviour * Replace some .forEach with for loops for slightly improved performance
given but for any error that occurs during the build phase when there are already some watchFiles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks a lot for this PR and sorry if I may have overstepped by adding some changes of my own. Please have a look at the comments and see if everything works as expected. I think this is a great improvement to watch mode that should be released soon!
Damn, no idea why the tests are failing on CI while they appear to be running nicely locally, will investigate |
Ok, it seems only chokidar can reliably keep watching deleted files, so I made this a chokidar-only feature. Another good argument for switching to always using chokidar at some point. |
d695346
to
e2e0a27
Compare
@lukastaegert just tested it on my local project and it works great! Thanks for the thorough look at it! |
@mhkeller @lukastaegert Using rollup v1.25.1 I'm still seeing the original issue with watch not firing after an initial error. Looking through this PR, it seems like it should be fixed when using chokidar, but it doesn't work here (chokidar v3, macOS). I'm testing by introducing a syntax error before starting
I have this in my
It doesn't make any difference to activate polling mode either using Any other ideas? |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #1773
Description
This PR is a fix for #1773 which created inconsistent behavior between how errors were handled when using rollup watch. Before, if you had a syntax error in your file, the watch process would never get started. Now, it will watch all files in the dependency graph up until the error and keep your watch process running. This avoids the user from having to restart the watch process once the error is fixed and matches the normal rollup watch behavior.
It does this by adding the
graph
object to the error and then adding the watchers for files already scanned in the graph.Still needed:
watchFiles
array so those need to be fixed.