Configurable Build Delay (Currently Hardcoded at 200ms)

[mattdesl]

This PR contains:

• feature

Are tests included?

• no (see below)

Breaking Changes?

• no

List any relevant issue numbers:
resolves #3527

Description

There is currently a 200ms delay hardcoded into rollup watch, which means each rebuild takes at least 200ms before triggering any events. I'm sure this was added in early days as some sort of precaution, but I don't think it should be there without good evidence that it is needed. It seems like a small delay but consider many developers are reloading files dozens of times per minute. Just turn it off yourself locally and continue developing to see how much faster the development experience will feel with zero delay.

Regarding the need for it: at least anecdotally in my own tools (budo and subsequently canvas-sketch), I've been using a 0ms setTimeout since 2015 and have never run into any disk issues or heard any complaints about that from the many users across various platforms (I've run lots of workshops on crappy PCs etc). Perhaps there are situations where it is needed, but they seem to be rare.

If it is intended to stay in place, then at least it should be configurable, to allow developers and tool-builders to improve build times.

This PR is a first draft at adding this configuration, in a slightly clunky way:

const emitter = rollup.watch({ ...opts });
emitter.delay = 0; // force zero delay

Ideally it should be a feature of { watch } options, but this doesn't seem to fit easily within the current paradigm of the Watcher class, which handles multiple configurations and a single build delay across all of them. Further, I haven't added tests as the watch testing suite currently uses some arbitrary timeouts already, and it seems like it would be challenging to test it accurately (happy to receive suggestions and include that in this PR though!).

Also — apologies about git diff noise within this PR, it seems to have been auto-generated by the 'git commit' action. If there is any suggestion on how to improve the PR within these constraints let me know.

[lukastaegert]

The original reason for having this is that if there is a number of writes, e.g. in a scenario where Rollup watch mode is a secondary step on top of another process like a TypeScript watch mode, then having more than one update in a short amount of time would always trigger two builds: One build for the first change and another build that is queued after the first build completes with the remaining changes. Similarly this was meant to buffer a nervous watcher that triggers several times for a single write (which should be mostly solved by our switch to chokidar, though; note however that if you are on a Mac, you may not get a realistic picture as fsevents is really a superior watch backend there).

I agree though that this scenario may not be as relevant and was not really based on evidence as you suggest but rather assumption. So if we go through with this PR, we might as well change the default to 0 to see how this works.

Ideally it should be a feature of { watch } options, but this doesn't seem to fit easily within the current paradigm of the Watcher class, which handles multiple configurations and a single build delay across all of them

True, but the same goes for the wach.clearScreen option. Here the logic is that if one config sets it to false, it should be false altogether independent of other configs, cf.

rollup/cli/run/resetScreen.ts

Lines 8 to 12 in c0c206e

	for (const config of configs) {
	if (config.watch && config.watch.clearScreen === false) {
	clearScreen = false;
	}
	}

We could employ a similar logic for the delay, e.g. the actual delay is the maximum delay specified by any config, and if a config has no watch options, we assume it has the default delay, which could be 0 following your elaborations. The idea is that this option is added to a build that what benefit from fewer rebuilds for whatever reason.

An advantage of putting it there is that then these options would also be easily available in config files.

Further, I haven't added tests as the watch testing suite currently uses some arbitrary timeouts already, and it seems like it would be challenging to test it accurately (happy to receive suggestions and include that in this PR though!).

I do not think we want an accurate test as that one would likely be rather flaky. We could however create a smoke test for this option that adds a large value, e.g. one second, and tests that the rebuild did not occur after half a second but occured eventually.

[codecov]

Codecov Report

Merging #3502 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3502   +/-   ##
=======================================
  Coverage   96.29%   96.29%           
=======================================
  Files         176      176           
  Lines        6040     6044    +4     
  Branches     1781     1783    +2     
=======================================
+ Hits         5816     5820    +4     
  Misses        112      112           
  Partials      112      112           

Impacted Files	Coverage Δ
src/watch/watch.ts	99.03% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f63e54d...95404ec. Read the comment docs.

[lukastaegert]

As there was no communication, I performed the changes I mentioned and added tests and documentation.

[mattdesl]

Thanks Lukas!