Skip to content

Commit 75ed818

Browse files
authored
fix: prevent change listener leak in Vite plugin (#110)
Each call to `configureServer` (e.g. on Vite dev-server restart) added a new `change` listener without removing the previous one, eventually triggering Node's `MaxListenersExceededWarning`. Store a cleanup function in the plugin closure and call it at the start of each `configureServer` invocation so only one listener is active at a time. Add tests covering registration, cleanup, file filtering, and the `disableWatch` opt-out. Ref: #108
1 parent a57caa3 commit 75ed818

File tree

2 files changed

+116
-13
lines changed

2 files changed

+116
-13
lines changed

src/plugins/vite.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
*/
3434

3535
import { resolve } from 'node:path';
36-
import type { Plugin, ViteDevServer } from 'vite';
36+
import type { Plugin } from 'vite';
3737
import {
3838
type GenerateOptions,
3939
type GenerateOutputOptions,
@@ -89,31 +89,35 @@ export interface Options extends Omit<GenerateOptions, 'output'> {
8989
* @ignore
9090
*/
9191
export function icpBindgen(options: Options): Plugin {
92+
let cleanupWatcher: (() => void) | undefined;
93+
9294
return {
9395
name: VITE_PLUGIN_NAME,
9496
async buildStart() {
9597
await run(options);
9698
},
9799
configureServer(server) {
98100
if (!options.disableWatch) {
99-
watchDidFileChanges(server, options);
101+
// Remove previous listener to prevent accumulation on server restart.
102+
cleanupWatcher?.();
103+
104+
const didFilePath = resolve(options.didFile);
105+
server.watcher.add(didFilePath);
106+
107+
const onChange = async (changedPath: string) => {
108+
if (resolve(changedPath) === resolve(didFilePath)) {
109+
await run(options);
110+
}
111+
};
112+
113+
server.watcher.on('change', onChange);
114+
cleanupWatcher = () => server.watcher.off('change', onChange);
100115
}
101116
},
102117
sharedDuringBuild: true,
103118
};
104119
}
105120

106-
function watchDidFileChanges(server: ViteDevServer, options: Options) {
107-
const didFilePath = resolve(options.didFile);
108-
109-
server.watcher.add(didFilePath);
110-
server.watcher.on('change', async (changedPath) => {
111-
if (resolve(changedPath) === resolve(didFilePath)) {
112-
await run(options);
113-
}
114-
});
115-
}
116-
117121
async function run(options: Options) {
118122
console.log(cyan(`[${VITE_PLUGIN_NAME}] Generating bindings from`), green(options.didFile));
119123

tests/vite-plugin.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* Tests for the `icpBindgen` Vite plugin's `configureServer` hook, which
3+
* watches a `.did` file for changes and re-runs code generation.
4+
*
5+
* The `generate` function is mocked so these tests focus purely on watcher
6+
* lifecycle behavior: listener registration, cleanup on server restart, file
7+
* filtering, and the `disableWatch` opt-out.
8+
*/
9+
10+
import { EventEmitter } from 'node:events';
11+
import { resolve } from 'node:path';
12+
import { beforeEach, describe, expect, it, vi } from 'vitest';
13+
import { icpBindgen } from '../src/plugins/vite.ts';
14+
15+
vi.mock('../src/core/generate/index.ts', () => ({
16+
generate: vi.fn().mockResolvedValue(undefined),
17+
}));
18+
19+
function createMockServer() {
20+
const watcher = new EventEmitter();
21+
return {
22+
watcher: Object.assign(watcher, {
23+
add: vi.fn(),
24+
off: watcher.removeListener.bind(watcher),
25+
}),
26+
};
27+
}
28+
29+
const pluginOptions = {
30+
didFile: './test.did',
31+
outDir: './out',
32+
};
33+
34+
// Our mock server only implements the subset of ViteDevServer used by the
35+
// plugin (watcher.add/on/off), so a single cast is needed here.
36+
function configureServer(
37+
plugin: ReturnType<typeof icpBindgen>,
38+
server: ReturnType<typeof createMockServer>,
39+
) {
40+
// biome-ignore lint/suspicious/noExplicitAny: partial mock of ViteDevServer
41+
(plugin.configureServer as any)(server);
42+
}
43+
44+
describe('icpBindgen vite plugin', () => {
45+
beforeEach(() => {
46+
vi.clearAllMocks();
47+
});
48+
49+
it('should register a change listener on configureServer', () => {
50+
const plugin = icpBindgen(pluginOptions);
51+
const server = createMockServer();
52+
53+
configureServer(plugin, server);
54+
55+
expect(server.watcher.listenerCount('change')).toBe(1);
56+
});
57+
58+
it('should not register a listener when disableWatch is true', () => {
59+
const plugin = icpBindgen({ ...pluginOptions, disableWatch: true });
60+
const server = createMockServer();
61+
62+
configureServer(plugin, server);
63+
64+
expect(server.watcher.listenerCount('change')).toBe(0);
65+
});
66+
67+
it('should clean up previous listener when configureServer is called again', () => {
68+
const plugin = icpBindgen(pluginOptions);
69+
const server = createMockServer();
70+
71+
// Simulate multiple configureServer calls (e.g. Vite server restarts
72+
// reusing the same watcher).
73+
for (let i = 0; i < 10; i++) {
74+
configureServer(plugin, server);
75+
}
76+
77+
expect(server.watcher.listenerCount('change')).toBe(1);
78+
});
79+
80+
it('should trigger generate only for the watched did file', async () => {
81+
const { generate } = await import('../src/core/generate/index.ts');
82+
83+
const plugin = icpBindgen(pluginOptions);
84+
const server = createMockServer();
85+
86+
configureServer(plugin, server);
87+
88+
// Emit a change for an unrelated file — should not trigger generate.
89+
server.watcher.emit('change', '/some/other/file.ts');
90+
await vi.waitFor(() => {});
91+
expect(generate).not.toHaveBeenCalled();
92+
93+
// Emit a change for the watched did file — should trigger generate.
94+
server.watcher.emit('change', resolve(pluginOptions.didFile));
95+
await vi.waitFor(() => {
96+
expect(generate).toHaveBeenCalledOnce();
97+
});
98+
});
99+
});

0 commit comments

Comments
 (0)