Skip to content

Commit e7cb995

Browse files
committed
Migrate file watchers from fs.watch to VS Code FileSystemWatcher API
Replace Node.js fs.watch with VS Code's FileSystemWatcher to fix Windows test timeout caused by endless file events bug (nodejs/node#61398). Changes: - src/util/watch.ts: New watchFile() function using workspace.createFileSystemWatcher with RelativePattern for reliable cross-platform file watching - src/explorer.ts: Migrate kubeconfig watching to new API with callback pattern - src/serverlessFunction/functionModel.ts: Replace multiple fs.watch calls with single glob pattern watcher for func.yaml files - src/util/workspace.ts: Enhanced devfile watching to support all valid extensions (.yaml, .yml, .json for both devfile and .devfile variants) - test/unit/util/watch.test.ts: New unit tests with proper VS Code API mocking (tests API contract without requiring full extension host) - test/integration/watch.test.ts: Integration test for real file watching behavior (requires full VS Code environment) - .gitignore: Add .claude/ directory Benefits: - Fixes Windows fs.watch endless events bug (VS Code uses parcel-watcher) - Better performance (runs in VS Code's UtilityProcess) - Cleaner code (callbacks instead of EventEmitters) - More complete devfile detection (all valid extensions) See: - nodejs/node#61398 (fs.watch endless file events on Windows) - microsoft/vscode#287800 (100% CPU on Windows with fs.watch) - https://github.com/microsoft/vscode/wiki/File-Watcher-Internals (VS Code file watcher implementation) Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com> Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f1d1a07 commit e7cb995

7 files changed

Lines changed: 204 additions & 130 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ dist/
1111
public/
1212
test-resources/
1313
.DS_Store
14-
14+
.claude/

src/explorer.ts

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { LoginUtil } from './util/loginUtil';
3737
import { Platform } from './util/platform';
3838
import { Progress } from './util/progress';
3939
import { ExecutionContext, imagePath } from './util/utils';
40-
import { FileContentChangeNotifier, WatchUtil } from './util/watch';
40+
import { watchFile } from './util/watch';
4141
import { vsCommand } from './vscommand';
4242
import { CustomResourceDefinitionStub, K8sResourceKind } from './webview/common/createServiceTypes';
4343
import { OpenShiftTerminalManager } from './webview/openshift-terminal/openShiftTerminal';
@@ -105,7 +105,7 @@ export class OpenShiftExplorer implements TreeDataProvider<ExplorerItem>, Dispos
105105

106106
private treeView: TreeView<ExplorerItem>;
107107

108-
private kubeConfigWatchers: FileContentChangeNotifier[];
108+
private kubeConfigWatchers: Disposable[];
109109
private kubeContext: Context;
110110
private kubeConfigInfo: KubeConfigInfo;
111111

@@ -129,29 +129,28 @@ export class OpenShiftExplorer implements TreeDataProvider<ExplorerItem>, Dispos
129129
try {
130130
const kubeconfigFiles = getKubeConfigFiles();
131131
this.kubeConfigWatchers = (!kubeconfigFiles || kubeconfigFiles.length === 0) ? [] :
132-
kubeconfigFiles.map(kubeconfigFile => WatchUtil.watchFileForContextChange(path.dirname(kubeconfigFile), path.basename(kubeconfigFile)));
132+
kubeconfigFiles.map(kubeconfigFile =>
133+
watchFile(kubeconfigFile, () => {
134+
const kci2 = new KubeConfigInfo();
135+
const kc2 = kci2.getEffectiveKubeConfig();
136+
const newCtx = kc2.getContextObject(kc2.currentContext);
137+
if (Boolean(this.kubeContext) !== Boolean(newCtx)
138+
|| (this.kubeContext?.cluster !== newCtx?.cluster
139+
|| this.kubeContext?.user !== newCtx?.user
140+
|| this.kubeContext?.namespace !== newCtx?.namespace)) {
141+
this.refresh();
142+
this.onDidChangeContextEmitter.fire(newCtx?.name); // newCtx can be 'null'
143+
}
144+
this.kubeContext = newCtx;
145+
this.kubeConfigInfo = kci2;
146+
})
147+
);
133148
if (this.kubeConfigWatchers.length === 0) {
134149
void window.showWarningMessage('Kubernetes configuration file(s) not found. Make sure that "${HOME}/.kube/config" file exists or "KUBECONFIG" environment variable is properly configured');
135150
}
136151
} catch {
137152
void window.showWarningMessage('Couldn\'t install watcher for Kubernetes configuration file. OpenShift Application Explorer view won\'t be updated automatically.');
138153
}
139-
for (const fsw of this.kubeConfigWatchers) {
140-
fsw.emitter?.on('file-changed', () => {
141-
const kci2 = new KubeConfigInfo();
142-
const kc2 = kci2.getEffectiveKubeConfig();
143-
const newCtx = kc2.getContextObject(kc2.currentContext);
144-
if (Boolean(this.kubeContext) !== Boolean(newCtx)
145-
|| (this.kubeContext?.cluster !== newCtx?.cluster
146-
|| this.kubeContext?.user !== newCtx?.user
147-
|| this.kubeContext?.namespace !== newCtx?.namespace)) {
148-
this.refresh();
149-
this.onDidChangeContextEmitter.fire(newCtx?.name); // newCtx can be 'null'
150-
}
151-
this.kubeContext = newCtx;
152-
this.kubeConfigInfo = kci2;
153-
});
154-
}
155154
this.treeView = window.createTreeView<ExplorerItem>('openshiftProjectExplorer', {
156155
treeDataProvider: this,
157156
});
@@ -804,8 +803,8 @@ export class OpenShiftExplorer implements TreeDataProvider<ExplorerItem>, Dispos
804803
}
805804

806805
dispose(): void {
807-
for (const fsw of this.kubeConfigWatchers) {
808-
fsw?.watcher?.close();
806+
for (const watcher of this.kubeConfigWatchers) {
807+
watcher?.dispose();
809808
}
810809
this.treeView.dispose();
811810
}

src/serverlessFunction/functionModel.ts

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as fs from 'fs-extra';
77
import * as path from 'path';
8-
import { Disposable, Uri, window, workspace } from 'vscode';
8+
import { Disposable, FileSystemWatcher, Uri, workspace } from 'vscode';
99
import { CommandText } from '../base/command';
1010
import { CliChannel } from '../cli';
1111
import { DeploymentConfig } from '../k8s/deploymentConfig';
@@ -15,19 +15,17 @@ import { DeployedFunction, FunctionContent, FunctionObject, FunctionStatus, Func
1515

1616
export class ServerlessFunctionModel implements Disposable {
1717

18-
private watchers: fs.FSWatcher[] = [];
18+
private funcYamlWatcher: FileSystemWatcher;
1919
private workspaceWatcher: Disposable;
2020
private view: FunctionView;
2121

2222
public constructor(view: FunctionView) {
2323
this.view = view;
24-
this.addWatchers();
24+
this.setupWatchers();
2525
this.workspaceWatcher = workspace.onDidChangeWorkspaceFolders((_e) => {
26-
for (const watcher of this.watchers) {
27-
watcher.close();
28-
}
26+
this.funcYamlWatcher?.dispose();
2927
this.view.refresh();
30-
this.addWatchers();
28+
this.setupWatchers();
3129
});
3230
}
3331

@@ -101,10 +99,8 @@ export class ServerlessFunctionModel implements Disposable {
10199
}
102100

103101
public dispose() {
104-
for (const watcher of this.watchers) {
105-
watcher.close();
106-
}
107-
this.workspaceWatcher.dispose();
102+
this.funcYamlWatcher?.dispose();
103+
this.workspaceWatcher?.dispose();
108104
}
109105

110106
private getDeployFunction(
@@ -132,28 +128,13 @@ export class ServerlessFunctionModel implements Disposable {
132128
return Functions.imageRegex.test(yamlContent?.image);
133129
}
134130

135-
private addWatchers() {
136-
if (workspace.workspaceFolders) {
137-
for (const workspaceFolder of workspace.workspaceFolders) {
138-
let folderExists = false;
139-
try {
140-
fs.accessSync(workspaceFolder.uri.fsPath);
141-
folderExists = true;
142-
} catch {
143-
// folder doesn't exist
144-
void window.showErrorMessage(`Can't keep track of if '${path.basename(workspaceFolder.uri.fsPath)}' contains a serverless function, since it's been deleted.`);
145-
}
146-
if (folderExists) {
147-
this.watchers.push(
148-
fs.watch(workspaceFolder.uri.fsPath, (_event, filename) => {
149-
if (filename === 'func.yaml') {
150-
this.view.refresh();
151-
}
152-
}),
153-
);
154-
}
155-
}
156-
}
131+
private setupWatchers() {
132+
// Watch for func.yaml files across all workspace folders using VS Code's FileSystemWatcher
133+
// This is more reliable than fs.watch, especially on Windows
134+
this.funcYamlWatcher = workspace.createFileSystemWatcher('**/func.yaml');
135+
this.funcYamlWatcher.onDidChange(() => this.view.refresh());
136+
this.funcYamlWatcher.onDidCreate(() => this.view.refresh());
137+
this.funcYamlWatcher.onDidDelete(() => this.view.refresh());
157138
}
158139

159140
private async getListItems(command: CommandText, fail = false) {

src/util/watch.ts

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,30 @@
33
* Licensed under the MIT License. See LICENSE file in the project root for license information.
44
*-----------------------------------------------------------------------------------------------*/
55

6-
import { EventEmitter } from 'events';
7-
import * as fs from 'fs';
8-
import { FSWatcher } from 'fs-extra';
9-
import { Util as fsxt } from '../util/utils';
6+
import * as path from 'path';
7+
import { Disposable, FileSystemWatcher, RelativePattern, workspace } from 'vscode';
108

11-
export class WatchUtil {
12-
static watchFileForContextChange(
13-
location: string,
14-
filename: string,
15-
): FileContentChangeNotifier {
16-
const emitter: EventEmitter = new EventEmitter();
17-
let timer: ReturnType<typeof setInterval> | undefined;
18-
fsxt.ensureDirSync(location);
19-
const watcher: FSWatcher = fsxt.watch(location, (eventType, changedFile) => {
20-
if (filename === changedFile) {
21-
if (timer) {
22-
clearTimeout(timer);
23-
}
24-
timer = setTimeout(() => {
25-
timer = undefined;
26-
emitter.emit('file-changed');
27-
}, 500);
28-
}
29-
});
30-
return { watcher, emitter };
31-
}
9+
/**
10+
* Watch a specific file for changes using VS Code's FileSystemWatcher API.
11+
* This uses VS Code's parcel-watcher implementation which is more reliable than fs.watch,
12+
* especially on Windows where fs.watch has known issues with endless events.
13+
*
14+
* @param filePath Absolute path to the file to watch
15+
* @param onChange Callback invoked when the file changes
16+
* @returns Disposable to stop watching
17+
*/
18+
export function watchFile(filePath: string, onChange: () => void): Disposable {
19+
const dir = path.dirname(filePath);
20+
const filename = path.basename(filePath);
3221

33-
}
22+
const watcher: FileSystemWatcher = workspace.createFileSystemWatcher(
23+
new RelativePattern(dir, filename),
24+
true, // ignoreCreateEvents
25+
false, // ignoreChangeEvents (we want these!)
26+
true // ignoreDeleteEvents
27+
);
28+
29+
watcher.onDidChange(onChange);
3430

35-
export interface FileContentChangeNotifier {
36-
readonly watcher: fs.FSWatcher;
37-
readonly emitter: EventEmitter;
31+
return watcher;
3832
}

src/util/workspace.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ export const AddWorkspaceFolder: QuickPickItem = {
1717
description: 'Folder which does not have an OpenShift context',
1818
};
1919

20+
// Extract devfile patterns as constants
21+
const DEVFILE_NAMES = ['devfile', '.devfile'] as const;
22+
const DEVFILE_EXTENSIONS = ['yaml', 'yml', 'json'] as const;
23+
const DEVFILE_GLOB_PATTERN = `**/{${DEVFILE_NAMES.join(',')}}.{${DEVFILE_EXTENSIONS.join(',')}}`;
24+
25+
// Pre-calculate devfile candidates once at module load
26+
const DEVFILE_CANDIDATES = DEVFILE_NAMES.flatMap(name =>
27+
DEVFILE_EXTENSIONS.map(ext => `${name}.${ext}`)
28+
);
29+
2030
function isFile(path: string) {
2131
try {
2232
return fs.statSync(path).isFile()
@@ -26,9 +36,9 @@ function isFile(path: string) {
2636
}
2737

2838
function isComponentorFunction(folder: WorkspaceFolder) {
29-
return !isFile(path.join(folder.uri.fsPath, 'devfile.yaml'))
30-
&& !isFile(path.join(folder.uri.fsPath, '.devfile.yaml'))
31-
&& !isFile(path.join(folder.uri.fsPath, 'func.yaml'));
39+
return !DEVFILE_CANDIDATES.some(candidate =>
40+
isFile(path.join(folder.uri.fsPath, candidate))
41+
) && !isFile(path.join(folder.uri.fsPath, 'func.yaml'));
3242
}
3343

3444
function componentAndFuctionFilter(wsFolder: WorkspaceFolder) {
@@ -158,7 +168,7 @@ async function updateDevfileContext(_: unknown) {
158168
*/
159169
export function setupWorkspaceDevfileContext(): Disposable {
160170
void updateDevfileContext(undefined);
161-
const devfileWatcher = workspace.createFileSystemWatcher('**/{devfile,.devfile}.yaml');
171+
const devfileWatcher = workspace.createFileSystemWatcher(DEVFILE_GLOB_PATTERN);
162172
devfileWatcher.onDidCreate(updateDevfileContext);
163173
devfileWatcher.onDidDelete(updateDevfileContext);
164174
workspace.onDidChangeWorkspaceFolders(updateDevfileContext);

test/integration/watch.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*-----------------------------------------------------------------------------------------------
2+
* Copyright (c) Red Hat, Inc. All rights reserved.
3+
* Licensed under the MIT License. See LICENSE file in the project root for license information.
4+
*-----------------------------------------------------------------------------------------------*/
5+
6+
import * as chai from 'chai';
7+
import * as path from 'path';
8+
import * as tmp from 'tmp';
9+
import { Util as fs } from '../../src/util/utils';
10+
import { watchFile } from '../../src/util/watch';
11+
12+
const {expect} = chai;
13+
14+
suite('File Watch Utility', () => {
15+
16+
test('watchFile calls the callback when file changes', async () => {
17+
// Create a temp file
18+
const tmpFile = tmp.fileSync({dir:`${path.basename(tmp.dirSync().name)}`});
19+
const fileToWatch = fs.realpathSync(tmpFile.name);
20+
fs.ensureFileSync(fileToWatch);
21+
22+
// Set up the file watcher
23+
let changeDetected = false;
24+
const watcher = watchFile(fileToWatch, () => {
25+
changeDetected = true;
26+
});
27+
28+
// Write to the file after a delay
29+
setTimeout(() => {
30+
fs.writeFileSync(fileToWatch, 'current-context:test2');
31+
}, 500);
32+
33+
// Wait for the change to be detected
34+
return new Promise<void>((resolve) => {
35+
const checkInterval = setInterval(() => {
36+
if (changeDetected) {
37+
clearInterval(checkInterval);
38+
expect(changeDetected).to.be.true;
39+
resolve();
40+
}
41+
}, 100);
42+
43+
// Timeout after 5 seconds
44+
setTimeout(() => {
45+
clearInterval(checkInterval);
46+
if (!changeDetected) {
47+
throw new Error('File change was not detected within timeout');
48+
}
49+
}, 5000);
50+
}).finally(() => {
51+
watcher?.dispose();
52+
tmpFile.removeCallback();
53+
});
54+
});
55+
56+
test('watchFile returns a Disposable', () => {
57+
const tmpFile = tmp.fileSync();
58+
const fileToWatch = tmpFile.name;
59+
60+
const watcher = watchFile(fileToWatch, () => {
61+
// callback
62+
});
63+
64+
try {
65+
expect(watcher).to.have.property('dispose');
66+
expect(watcher.dispose).to.be.a('function');
67+
} finally {
68+
watcher?.dispose();
69+
tmpFile.removeCallback();
70+
}
71+
});
72+
});

0 commit comments

Comments
 (0)