Skip to content

Commit cf75d71

Browse files
authored
Do not abort watch mode on errors in watchChange (#4427)
* Recovers from errors in the watchChange hook * Also emit END event * Fix direct npm install * Await watchChange and closeWatcher hooks * Extract emitter and improve coverage * Update docs
1 parent 6881753 commit cf75d71

12 files changed

Lines changed: 249 additions & 127 deletions

File tree

cli/run/watch-cli.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export async function watch(command: Record<string, any>): Promise<void> {
5656
return;
5757
}
5858
if (watcher) {
59-
watcher.close();
59+
await watcher.close();
6060
}
6161
start(options, warnings);
6262
} catch (err: any) {
@@ -136,12 +136,12 @@ export async function watch(command: Record<string, any>): Promise<void> {
136136
});
137137
}
138138

139-
function close(code: number | null): void {
139+
async function close(code: number | null): Promise<void> {
140140
process.removeListener('uncaughtException', close);
141141
// removing a non-existent listener is a no-op
142142
process.stdin.removeListener('end', close);
143143

144-
if (watcher) watcher.close();
144+
if (watcher) await watcher.close();
145145
if (configWatcher) configWatcher.close();
146146

147147
if (code) {

docs/05-plugin-development.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export default ({
5050

5151
- Plugins should have a clear name with `rollup-plugin-` prefix.
5252
- Include `rollup-plugin` keyword in `package.json`.
53-
- Plugins should be tested. We recommend [mocha](https://github.com/mochajs/mocha) or [ava](https://github.com/avajs/ava) which support promises out of the box.
53+
- Plugins should be tested. We recommend [mocha](https://github.com/mochajs/mocha) or [ava](https://github.com/avajs/ava) which support Promises out of the box.
5454
- Use asynchronous methods when it is possible.
5555
- Document your plugin in English.
5656
- Make sure your plugin outputs correct source mappings if appropriate.
@@ -68,7 +68,7 @@ The name of the plugin, for use in error messages and warnings.
6868

6969
To interact with the build process, your plugin object includes 'hooks'. Hooks are functions which are called at various stages of the build. Hooks can affect how a build is run, provide information about a build, or modify a build once complete. There are different kinds of hooks:
7070

71-
- `async`: The hook may also return a promise resolving to the same type of value; otherwise, the hook is marked as `sync`.
71+
- `async`: The hook may also return a Promise resolving to the same type of value; otherwise, the hook is marked as `sync`.
7272
- `first`: If several plugins implement this hook, the hooks are run sequentially until a hook returns a value other than `null` or `undefined`.
7373
- `sequential`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is async, subsequent hooks of this kind will wait until the current hook is resolved.
7474
- `parallel`: If several plugins implement this hook, all of them will be run in the specified plugin order. If a hook is async, subsequent hooks of this kind will be run in parallel and not wait for the current hook.
@@ -96,9 +96,9 @@ Called on each `rollup.rollup` build. This is the recommended hook to use when y
9696

9797
#### `closeWatcher`
9898

99-
**Type:** `() => void`<br> **Kind:** `sync, sequential`<br> **Previous/Next Hook:** This hook can be triggered at any time both during the build and the output generation phases. If that is the case, the current build will still proceed but no new [`watchChange`](guide/en/#watchchange) events will be triggered ever.
99+
**Type:** `() => void`<br> **Kind:** `async, parallel`<br> **Previous/Next Hook:** This hook can be triggered at any time both during the build and the output generation phases. If that is the case, the current build will still proceed but no new [`watchChange`](guide/en/#watchchange) events will be triggered ever.
100100

101-
Notifies a plugin when watcher process closes and all open resources should be closed too. This hook cannot be used by output plugins.
101+
Notifies a plugin when the watcher process will close so that all open resources can be closed too. If a Promise is returned, Rollup will wait for the Promise to resolve before closing the process. This hook cannot be used by output plugins.
102102

103103
#### `load`
104104

@@ -302,9 +302,9 @@ You can use [`this.getModuleInfo`](guide/en/#thisgetmoduleinfo) to find out the
302302

303303
#### `watchChange`
304304

305-
**Type:** `watchChange: (id: string, change: {event: 'create' | 'update' | 'delete'}) => void`<br> **Kind:** `sync, sequential`<br> **Previous/Next Hook:** This hook can be triggered at any time both during the build and the output generation phases. If that is the case, the current build will still proceed but a new build will be scheduled to start once the current build has completed, starting again with [`options`](guide/en/#options).
305+
**Type:** `watchChange: (id: string, change: {event: 'create' | 'update' | 'delete'}) => void`<br> **Kind:** `async, parallel`<br> **Previous/Next Hook:** This hook can be triggered at any time both during the build and the output generation phases. If that is the case, the current build will still proceed but a new build will be scheduled to start once the current build has completed, starting again with [`options`](guide/en/#options).
306306

307-
Notifies a plugin whenever rollup has detected a change to a monitored file in `--watch` mode. This hook cannot be used by output plugins. Second argument contains additional details of change event.
307+
Notifies a plugin whenever rollup has detected a change to a monitored file in `--watch` mode. If a Promise is returned, Rollup will wait for the Promise to resolve before scheduling another build. This hook cannot be used by output plugins. The second argument contains additional details of the change event.
308308

309309
### Output Generation Hooks
310310

@@ -770,7 +770,7 @@ Loads and parses the module corresponding to the given id, attaching additional
770770
771771
This allows you to inspect the final content of modules before deciding how to resolve them in the [`resolveId`](guide/en/#resolveid) hook and e.g. resolve to a proxy module instead. If the module becomes part of the graph later, there is no additional overhead from using this context function as the module will not be parsed again. The signature allows you to directly pass the return value of [`this.resolve`](guide/en/#thisresolve) to this function as long as it is neither `null` nor external.
772772
773-
The returned promise will resolve once the module has been fully transformed and parsed but before any imports have been resolved. That means that the resulting `ModuleInfo` will have empty `importedIds`, `dynamicallyImportedIds`, `importedIdResolutions` and `dynamicallyImportedIdResolutions`. This helps to avoid deadlock situations when awaiting `this.load` in a `resolveId` hook. If you are interested in `importedIds` and `dynamicallyImportedIds`, you can either implement a `moduleParsed` hook or pass the `resolveDependencies` flag, which will make the promise returned by `this.load` wait until all dependency ids have been resolved.
773+
The returned Promise will resolve once the module has been fully transformed and parsed but before any imports have been resolved. That means that the resulting `ModuleInfo` will have empty `importedIds`, `dynamicallyImportedIds`, `importedIdResolutions` and `dynamicallyImportedIdResolutions`. This helps to avoid deadlock situations when awaiting `this.load` in a `resolveId` hook. If you are interested in `importedIds` and `dynamicallyImportedIds`, you can either implement a `moduleParsed` hook or pass the `resolveDependencies` flag, which will make the Promise returned by `this.load` wait until all dependency ids have been resolved.
774774
775775
Note that with regard to the `moduleSideEffects`, `syntheticNamedExports` and `meta` options, the same restrictions apply as for the `resolveId` hook: Their values only have an effect if the module has not been loaded yet. Thus, it is very important to use `this.resolve` first to find out if any plugins want to set special values for these options in their `resolveId` hook, and pass these options on to `this.load` if appropriate. The example below showcases how this can be handled to add a proxy module for modules containing a special code comment. Note the special handling for re-exporting the default export:
776776

docs/build-hooks.mmd

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ flowchart TB
3232
transform("transform"):::hook-sequential
3333
click transform "/guide/en/#transform" _parent
3434

35-
watchchange("watchChange"):::hook-sequential-sync
35+
watchchange("watchChange"):::hook-parallel
3636
click watchchange "/guide/en/#watchchange" _parent
3737

38-
closewatcher("closeWatcher"):::hook-sequential-sync
38+
closewatcher("closeWatcher"):::hook-parallel
3939
click closewatcher "/guide/en/#closewatcher" _parent
4040

4141
options

package.json

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"rollup": "dist/bin/rollup"
1010
},
1111
"scripts": {
12-
"build": "shx rm -rf dist && git rev-parse HEAD > .commithash && rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
12+
"build": "shx rm -rf dist && node scripts/update-git-commit.js && rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
1313
"build:cjs": "shx rm -rf dist && rollup --config rollup.config.ts --configPlugin typescript --configTest && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
1414
"build:bootstrap": "node dist/bin/rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
1515
"ci:lint": "npm run lint:nofix",
@@ -22,10 +22,9 @@
2222
"perf": "npm run build:cjs && node --expose-gc scripts/perf.js",
2323
"perf:debug": "node --inspect-brk scripts/perf-debug.js",
2424
"perf:init": "node scripts/perf-init.js",
25-
"postinstall": "husky install",
26-
"postpublish": "pinst --enable && git push && git push --tags",
25+
"postpublish": "git push && git push --tags",
2726
"prepare": "husky install && npm run build",
28-
"prepublishOnly": "pinst --disable && npm ci && npm run lint:nofix && npm run security && npm run build:bootstrap && npm run test:all",
27+
"prepublishOnly": "npm ci && npm run lint:nofix && npm run security && npm run build:bootstrap && npm run test:all",
2928
"security": "npm audit",
3029
"test": "npm run build && npm run test:all",
3130
"test:cjs": "npm run build:cjs && npm run test:only",
@@ -97,7 +96,6 @@
9796
"magic-string": "^0.25.7",
9897
"mocha": "^9.2.1",
9998
"nyc": "^15.1.0",
100-
"pinst": "^3.0.0",
10199
"prettier": "^2.5.1",
102100
"pretty-bytes": "^5.6.0",
103101
"pretty-ms": "^7.0.1",

scripts/update-git-commit.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const { execSync } = require('child_process');
2+
const { writeFileSync } = require('fs');
3+
const { join } = require('path');
4+
5+
let revision;
6+
try {
7+
revision = execSync('git rev-parse HEAD').toString().trim();
8+
} catch (e) {
9+
console.warn('Could not determine git commit when building Rollup.');
10+
revision = '(could not be determined)';
11+
}
12+
writeFileSync(join(__dirname, '../.commithash'), revision);

src/Graph.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,11 @@ export default class Graph {
7878

7979
if (watcher) {
8080
this.watchMode = true;
81-
const handleChange: WatchChangeHook = (...args) =>
82-
this.pluginDriver.hookSeqSync('watchChange', args);
83-
const handleClose = () => this.pluginDriver.hookSeqSync('closeWatcher', []);
84-
watcher.on('change', handleChange);
85-
watcher.on('close', handleClose);
86-
watcher.once('restart', () => {
87-
watcher.removeListener('change', handleChange);
88-
watcher.removeListener('close', handleClose);
89-
});
81+
const handleChange = (...args: Parameters<WatchChangeHook>) =>
82+
this.pluginDriver.hookParallel('watchChange', args);
83+
const handleClose = () => this.pluginDriver.hookParallel('closeWatcher', []);
84+
watcher.onCurrentAwaited('change', handleChange);
85+
watcher.onCurrentAwaited('close', handleClose);
9086
}
9187
this.pluginDriver = new PluginDriver(this, options, options.plugins, this.pluginCache);
9288
this.acornParser = acorn.Parser.extend(...(options.acornInjectPlugins as any));

src/rollup/types.d.ts

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ export type WatchChangeHook = (
344344
this: PluginContext,
345345
id: string,
346346
change: { event: ChangeEvent }
347-
) => void;
347+
) => Promise<void> | void;
348348

349349
/**
350350
* use this type for plugin annotation
@@ -375,7 +375,7 @@ export interface PluginHooks extends OutputPluginHooks {
375375
buildEnd: (this: PluginContext, err?: Error) => Promise<void> | void;
376376
buildStart: (this: PluginContext, options: NormalizedInputOptions) => Promise<void> | void;
377377
closeBundle: (this: PluginContext) => Promise<void> | void;
378-
closeWatcher: (this: PluginContext) => void;
378+
closeWatcher: (this: PluginContext) => Promise<void> | void;
379379
load: LoadHook;
380380
moduleParsed: ModuleParsedHook;
381381
options: (
@@ -440,7 +440,9 @@ export type AsyncPluginHooks =
440440
| 'shouldTransformCachedModule'
441441
| 'transform'
442442
| 'writeBundle'
443-
| 'closeBundle';
443+
| 'closeBundle'
444+
| 'closeWatcher'
445+
| 'watchChange';
444446

445447
export type PluginValueHooks = 'banner' | 'footer' | 'intro' | 'outro';
446448

@@ -458,13 +460,11 @@ export type FirstPluginHooks =
458460

459461
export type SequentialPluginHooks =
460462
| 'augmentChunkHash'
461-
| 'closeWatcher'
462463
| 'generateBundle'
463464
| 'options'
464465
| 'outputOptions'
465466
| 'renderChunk'
466-
| 'transform'
467-
| 'watchChange';
467+
| 'transform';
468468

469469
export type ParallelPluginHooks =
470470
| 'banner'
@@ -477,7 +477,9 @@ export type ParallelPluginHooks =
477477
| 'renderError'
478478
| 'renderStart'
479479
| 'writeBundle'
480-
| 'closeBundle';
480+
| 'closeBundle'
481+
| 'closeWatcher'
482+
| 'watchChange';
481483

482484
interface OutputPluginValueHooks {
483485
banner: AddonHook;
@@ -898,6 +900,24 @@ interface TypedEventEmitter<T extends { [event: string]: (...args: any) => any }
898900
setMaxListeners(n: number): this;
899901
}
900902

903+
export interface RollupAwaitingEmitter<T extends { [event: string]: (...args: any) => any }>
904+
extends TypedEventEmitter<T> {
905+
close(): Promise<void>;
906+
emitAndAwait<K extends keyof T>(event: K, ...args: Parameters<T[K]>): Promise<ReturnType<T[K]>[]>;
907+
/**
908+
* Registers an event listener that will be awaited before Rollup continues
909+
* for events emitted via emitAndAwait. All listeners will be awaited in
910+
* parallel while rejections are tracked via Promise.all.
911+
* Listeners are removed automatically when removeAwaited is called, which
912+
* happens automatically after each run.
913+
*/
914+
onCurrentAwaited<K extends keyof T>(
915+
event: K,
916+
listener: (...args: Parameters<T[K]>) => Promise<ReturnType<T[K]>>
917+
): this;
918+
removeAwaited(): this;
919+
}
920+
901921
export type RollupWatcherEvent =
902922
| { code: 'START' }
903923
| { code: 'BUNDLE_START'; input?: InputOption; output: readonly string[] }
@@ -911,15 +931,12 @@ export type RollupWatcherEvent =
911931
| { code: 'END' }
912932
| { code: 'ERROR'; error: RollupError; result: RollupBuild | null };
913933

914-
export interface RollupWatcher
915-
extends TypedEventEmitter<{
916-
change: (id: string, change: { event: ChangeEvent }) => void;
917-
close: () => void;
918-
event: (event: RollupWatcherEvent) => void;
919-
restart: () => void;
920-
}> {
921-
close(): void;
922-
}
934+
export type RollupWatcher = RollupAwaitingEmitter<{
935+
change: (id: string, change: { event: ChangeEvent }) => void;
936+
close: () => void;
937+
event: (event: RollupWatcherEvent) => void;
938+
restart: () => void;
939+
}>;
923940

924941
export function watch(config: RollupWatchOptions | RollupWatchOptions[]): RollupWatcher;
925942

src/utils/PluginDriver.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -270,17 +270,6 @@ export class PluginDriver {
270270
return promise;
271271
}
272272

273-
// chains synchronously, ignores returns
274-
hookSeqSync<H extends SyncPluginHooks & SequentialPluginHooks>(
275-
hookName: H,
276-
args: Parameters<PluginHooks[H]>,
277-
replaceContext?: ReplaceContext
278-
): void {
279-
for (const plugin of this.plugins) {
280-
this.runHookSync(hookName, args, plugin, replaceContext);
281-
}
282-
}
283-
284273
/**
285274
* Run an async plugin hook and return the result.
286275
* @param hookName Name of the plugin hook. Must be either in `PluginHooks` or `OutputPluginValueHooks`.

src/watch/WatchEmitter.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { EventEmitter } from 'events';
2+
3+
type PromiseReturn<T extends (...args: any) => any> = (
4+
...args: Parameters<T>
5+
) => Promise<ReturnType<T>>;
6+
7+
export class WatchEmitter<
8+
T extends { [event: string]: (...args: any) => any }
9+
> extends EventEmitter {
10+
private awaitedHandlers: {
11+
[K in keyof T]?: PromiseReturn<T[K]>[];
12+
} = Object.create(null);
13+
14+
constructor() {
15+
super();
16+
// Allows more than 10 bundles to be watched without
17+
// showing the `MaxListenersExceededWarning` to the user.
18+
this.setMaxListeners(Infinity);
19+
}
20+
21+
// Will be overwritten by Rollup
22+
async close(): Promise<void> {}
23+
24+
emitAndAwait<K extends keyof T>(
25+
event: K,
26+
...args: Parameters<T[K]>
27+
): Promise<ReturnType<T[K]>[]> {
28+
this.emit(event as string, ...(args as any[]));
29+
return Promise.all(this.getHandlers(event).map(handler => handler(...args)));
30+
}
31+
32+
onCurrentAwaited<K extends keyof T>(
33+
event: K,
34+
listener: (...args: Parameters<T[K]>) => Promise<ReturnType<T[K]>>
35+
): this {
36+
this.getHandlers(event).push(listener);
37+
return this;
38+
}
39+
40+
removeAwaited(): this {
41+
this.awaitedHandlers = {};
42+
return this;
43+
}
44+
45+
private getHandlers<K extends keyof T>(event: K): PromiseReturn<T[K]>[] {
46+
return this.awaitedHandlers[event] || (this.awaitedHandlers[event] = []);
47+
}
48+
}

src/watch/watch-proxy.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,10 @@
1-
import { EventEmitter } from 'events';
21
import type { RollupWatcher } from '../rollup/types';
32
import { ensureArray } from '../utils/ensureArray';
43
import { errInvalidOption, error } from '../utils/error';
54
import type { GenericConfigObject } from '../utils/options/options';
5+
import { WatchEmitter } from './WatchEmitter';
66
import { loadFsEvents } from './fsevents-importer';
77

8-
class WatchEmitter extends EventEmitter {
9-
constructor() {
10-
super();
11-
// Allows more than 10 bundles to be watched without
12-
// showing the `MaxListenersExceededWarning` to the user.
13-
this.setMaxListeners(Infinity);
14-
}
15-
16-
close() {}
17-
}
18-
198
export default function watch(configs: GenericConfigObject[] | GenericConfigObject): RollupWatcher {
209
const emitter = new WatchEmitter() as RollupWatcher;
2110
const configArray = ensureArray(configs);

0 commit comments

Comments
 (0)