From 4409e6dd4612c2a99be6656fd9fe5505a899c3fe Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 26 Nov 2023 13:09:38 +0100 Subject: [PATCH] Allow this.addWatchFile in all hooks (#5270) * Allow this.addWatchFile in all hooks also trigger close hook in watch mode * Try without closing the bundle * Respect early "closed" state * Try closing the bundle again * Do not close but add documentation --- docs/plugin-development/index.md | 2 +- src/rollup/rollup.ts | 4 +- src/utils/PluginContext.ts | 6 +-- src/utils/logs.ts | 7 --- src/watch/watch.ts | 8 ++- .../add-watch-file-generate/_config.js | 22 -------- .../samples/add-watch-file-generate/main.js | 1 - .../add-watch-file-generate/watched.js | 0 test/utils.js | 23 +++++++++ test/watch/index.js | 50 +++++++++++++++---- 10 files changed, 76 insertions(+), 47 deletions(-) delete mode 100644 test/function/samples/add-watch-file-generate/_config.js delete mode 100644 test/function/samples/add-watch-file-generate/main.js delete mode 100644 test/function/samples/add-watch-file-generate/watched.js diff --git a/docs/plugin-development/index.md b/docs/plugin-development/index.md index 0a6ae7bdb..929d62e21 100644 --- a/docs/plugin-development/index.md +++ b/docs/plugin-development/index.md @@ -1182,7 +1182,7 @@ A number of utility functions and informational bits can be accessed from within | ----: | :--------------------- | | Type: | `(id: string) => void` | -Adds additional files to be monitored in watch mode so that changes to these files will trigger rebuilds. `id` can be an absolute path to a file or directory or a path relative to the current working directory. This context function can only be used in hooks during the build phase, i.e. in `buildStart`, `load`, `resolveId`, and `transform`. +Adds additional files to be monitored in watch mode so that changes to these files will trigger rebuilds. `id` can be an absolute path to a file or directory or a path relative to the current working directory. This context function can be used in all plugin hooks except `closeBundle`. However, it will not have an effect when used in [output generation hooks](#output-generation-hooks) if [`watch.skipWrite`](../configuration-options/index.md#watch-skipwrite) is set to `true`. **Note:** Usually in watch mode to improve rebuild speed, the `transform` hook will only be triggered for a given module if its contents actually changed. Using `this.addWatchFile` from within the `transform` hook will make sure the `transform` hook is also reevaluated for this module if the watched file changes. diff --git a/src/rollup/rollup.ts b/src/rollup/rollup.ts index e642f92c6..57b33bc61 100644 --- a/src/rollup/rollup.ts +++ b/src/rollup/rollup.ts @@ -100,7 +100,9 @@ export async function rollupInternal( return handleGenerateWrite(false, inputOptions, unsetInputOptions, rawOutputOptions, graph); }, - watchFiles: Object.keys(graph.watchFiles), + get watchFiles() { + return Object.keys(graph.watchFiles); + }, async write(rawOutputOptions: OutputOptions) { if (result.closed) return error(logAlreadyClosed()); diff --git a/src/utils/PluginContext.ts b/src/utils/PluginContext.ts index a3ef169ce..00d3f4aaf 100644 --- a/src/utils/PluginContext.ts +++ b/src/utils/PluginContext.ts @@ -10,10 +10,9 @@ import type { import type { FileEmitter } from './FileEmitter'; import { createPluginCache, getCacheForUncacheablePlugin, NO_CACHE } from './PluginCache'; import { BLANK, EMPTY_OBJECT } from './blank'; -import { BuildPhase } from './buildPhase'; import { getLogHandler } from './logHandler'; import { LOGLEVEL_DEBUG, LOGLEVEL_INFO, LOGLEVEL_WARN } from './logging'; -import { error, logInvalidRollupPhaseForAddWatchFile, logPluginError } from './logs'; +import { error, logPluginError } from './logs'; import { normalizeLog } from './options/options'; import { parseAst } from './parseAst'; import { ANONYMOUS_OUTPUT_PLUGIN_PREFIX, ANONYMOUS_PLUGIN_PREFIX } from './pluginUtils'; @@ -54,9 +53,6 @@ export function getPluginContext( return { addWatchFile(id) { - if (graph.phase >= BuildPhase.GENERATE) { - return this.error(logInvalidRollupPhaseForAddWatchFile()); - } graph.watchFiles[id] = true; }, cache: cacheInstance, diff --git a/src/utils/logs.ts b/src/utils/logs.ts index 6deecb3ea..6adfab612 100644 --- a/src/utils/logs.ts +++ b/src/utils/logs.ts @@ -588,13 +588,6 @@ export function logInvalidFunctionPluginHook(hook: string, plugin: string): Roll }; } -export function logInvalidRollupPhaseForAddWatchFile(): RollupLog { - return { - code: INVALID_ROLLUP_PHASE, - message: `Cannot call "addWatchFile" after the build has finished.` - }; -} - export function logInvalidRollupPhaseForChunkEmission(): RollupLog { return { code: INVALID_ROLLUP_PHASE, diff --git a/src/watch/watch.ts b/src/watch/watch.ts index 5179082f8..97734de2a 100644 --- a/src/watch/watch.ts +++ b/src/watch/watch.ts @@ -207,7 +207,13 @@ export class Task { return; } this.updateWatchedFiles(result); - this.skipWrite || (await Promise.all(this.outputs.map(output => result!.write(output)))); + if (!this.skipWrite) { + await Promise.all(this.outputs.map(output => result!.write(output))); + if (this.closed) { + return; + } + this.updateWatchedFiles(result!); + } await this.watcher.emitter.emit('event', { code: 'BUNDLE_END', duration: Date.now() - start, diff --git a/test/function/samples/add-watch-file-generate/_config.js b/test/function/samples/add-watch-file-generate/_config.js deleted file mode 100644 index 862acfce3..000000000 --- a/test/function/samples/add-watch-file-generate/_config.js +++ /dev/null @@ -1,22 +0,0 @@ -const path = require('node:path'); - -module.exports = defineTest({ - description: 'throws when adding watch files during generate', - options: { - plugins: [ - { - name: 'test-plugin', - renderStart() { - this.addWatchFile(path.join(__dirname, 'watched.js')); - } - } - ] - }, - generateError: { - code: 'PLUGIN_ERROR', - hook: 'renderStart', - message: 'Cannot call "addWatchFile" after the build has finished.', - plugin: 'test-plugin', - pluginCode: 'INVALID_ROLLUP_PHASE' - } -}); diff --git a/test/function/samples/add-watch-file-generate/main.js b/test/function/samples/add-watch-file-generate/main.js deleted file mode 100644 index a9244a453..000000000 --- a/test/function/samples/add-watch-file-generate/main.js +++ /dev/null @@ -1 +0,0 @@ -throw new Error('Not executed'); diff --git a/test/function/samples/add-watch-file-generate/watched.js b/test/function/samples/add-watch-file-generate/watched.js deleted file mode 100644 index e69de29bb..000000000 diff --git a/test/utils.js b/test/utils.js index d4968f096..f0f49b3bc 100644 --- a/test/utils.js +++ b/test/utils.js @@ -38,6 +38,29 @@ exports.wait = function wait(ms) { }); }; +/** + * @param {Promise} promise + * @param {number} timeoutMs + * @param {() => unknown} onTimeout + * @return {Promise} + */ +exports.withTimeout = function withTimeout(promise, timeoutMs, onTimeout) { + let timeoutId; + return Promise.race([ + promise.then(() => clearTimeout(timeoutId)), + new Promise((resolve, reject) => { + timeoutId = setTimeout(() => { + try { + onTimeout(); + resolve(); + } catch (error) { + reject(error); + } + }, timeoutMs); + }) + ]); +}; + function normaliseError(error) { if (!error) { throw new Error(`Expected an error but got ${JSON.stringify(error)}`); diff --git a/test/watch/index.js b/test/watch/index.js index 48350cef4..a8a6ed180 100644 --- a/test/watch/index.js +++ b/test/watch/index.js @@ -8,7 +8,7 @@ const { copy } = require('fs-extra'); * @type {import('../../src/rollup/types')} Rollup */ const rollup = require('../../dist/rollup'); -const { atomicWriteFileSync, wait } = require('../utils'); +const { atomicWriteFileSync, wait, withTimeout } = require('../utils'); describe('rollup.watch', () => { let watcher; @@ -612,7 +612,7 @@ describe('rollup.watch', () => { input: 'test/_tmp/input/main.js', plugins: { async watchChange() { - await new Promise(resolve => setTimeout(resolve, 300)); + await wait(300); if (fail) { this.error('Failed in watchChange'); } @@ -1385,11 +1385,28 @@ describe('rollup.watch', () => { describe('addWatchFile', () => { it('supports adding additional watch files in plugin hooks', async () => { const watchChangeIds = new Set(); + const buildEndFile = resolve('test/_tmp/input/buildEnd'); const buildStartFile = resolve('test/_tmp/input/buildStart'); + const generateBundleFile = resolve('test/_tmp/input/generateBUndle'); const loadFile = resolve('test/_tmp/input/load'); + const moduleParsedFile = resolve('test/_tmp/input/moduleParsed'); + const renderChunkFile = resolve('test/_tmp/input/renderChunk'); + const renderStartFile = resolve('test/_tmp/input/renderStart'); const resolveIdFile = resolve('test/_tmp/input/resolveId'); const transformFile = resolve('test/_tmp/input/transform'); - const watchFiles = [buildStartFile, loadFile, resolveIdFile, transformFile]; + const writeBundleFile = resolve('test/_tmp/input/writeBundle'); + const watchFiles = [ + buildEndFile, + buildStartFile, + generateBundleFile, + loadFile, + moduleParsedFile, + renderChunkFile, + renderStartFile, + resolveIdFile, + transformFile, + writeBundleFile + ]; await copy('test/watch/samples/basic', 'test/_tmp/input'); await Promise.all(watchFiles.map(file => writeFile(file, 'initial'))); @@ -1402,18 +1419,36 @@ describe('rollup.watch', () => { exports: 'auto' }, plugins: { + buildEnd() { + this.addWatchFile(buildEndFile); + }, buildStart() { this.addWatchFile(buildStartFile); }, + generateBundle() { + this.addWatchFile(generateBundleFile); + }, load() { this.addWatchFile(loadFile); }, + moduleParsed() { + this.addWatchFile(moduleParsedFile); + }, + renderChunk() { + this.addWatchFile(renderChunkFile); + }, + renderStart() { + this.addWatchFile(renderStartFile); + }, resolveId() { this.addWatchFile(resolveIdFile); }, transform() { this.addWatchFile(transformFile); }, + writeBundle() { + this.addWatchFile(writeBundleFile); + }, watchChange(id) { watchChangeIds.add(id); } @@ -1795,12 +1830,9 @@ function sequence(watcher, events, timeout = 300) { go(); }); - return Promise.race([ - sequencePromise.then(() => wait(100)), - wait(20_000).then(() => { - throw new Error(`Test timed out\n${handledEvents.join('\n')}`); - }) - ]); + return withTimeout(sequencePromise, 20_000, () => { + throw new Error(`Test timed out\n${handledEvents.join('\n')}`); + }).then(() => wait(100)); } function getTimeDiffInMs(previous) {