Skip to content

Commit

Permalink
fix: handle property the watch mode for extension in development mode (
Browse files Browse the repository at this point in the history
…podman-desktop#10623)

* fix: handle property the watch mode for extension in development mode

fixes podman-desktop#10619
Signed-off-by: Florent Benoit <[email protected]>
  • Loading branch information
benoitf authored Jan 15, 2025
1 parent 4149966 commit deed092
Show file tree
Hide file tree
Showing 9 changed files with 565 additions and 18 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
"eslint-plugin-sonarjs": "^3.0.1",
"eslint-plugin-unicorn": "^56.0.1",
"fzstd": "^0.1.1",
"get-tsconfig": "^4.8.1",
"globals": "^15.14.0",
"husky": "^9.1.7",
"js-yaml": "^4.1.0",
Expand Down
59 changes: 59 additions & 0 deletions packages/main/src/plugin/extension-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import type { DialogRegistry } from './dialog-registry.js';
import type { Directories } from './directories.js';
import type { ActivatedExtension, AnalyzedExtension, RequireCacheDict } from './extension-loader.js';
import { ExtensionLoader } from './extension-loader.js';
import type { ExtensionWatcher } from './extension-watcher.js';
import type { FilesystemMonitoring } from './filesystem-monitoring.js';
import type { IconRegistry } from './icon-registry.js';
import type { ImageCheckerImpl } from './image-checker.js';
Expand Down Expand Up @@ -113,6 +114,10 @@ class TestExtensionLoader extends ExtensionLoader {
setAnalyzedExtension(extensionId: string, analyzedExtension: AnalyzedExtension): void {
this.analyzedExtensions.set(extensionId, analyzedExtension);
}

override reloadExtension(extension: AnalyzedExtension, removable: boolean): Promise<void> {
return super.reloadExtension(extension, removable);
}
}

let extensionLoader: TestExtensionLoader;
Expand Down Expand Up @@ -210,6 +215,7 @@ const exec = new Exec(proxy);

const notificationRegistry: NotificationRegistry = {
registerExtension: vi.fn(),
addNotification: vi.fn(),
} as unknown as NotificationRegistry;

const imageCheckerImpl: ImageCheckerImpl = {
Expand Down Expand Up @@ -252,6 +258,13 @@ const dialogRegistry: DialogRegistry = {

const certificates: Certificates = {} as unknown as Certificates;

const extensionWatcher = {
monitor: vi.fn(),
untrack: vi.fn(),
stop: vi.fn(),
reloadExtension: vi.fn(),
} as unknown as ExtensionWatcher;

vi.mock('electron', () => {
return {
app: {
Expand Down Expand Up @@ -304,6 +317,7 @@ beforeAll(() => {
dialogRegistry,
safeStorageRegistry,
certificates,
extensionWatcher,
);
});

Expand Down Expand Up @@ -1193,6 +1207,8 @@ test('check dispose when deactivating', async () => {
await extensionLoader.deactivateExtension(extensionId);
expect(analyzedExtension.dispose).toBeCalled();

expect(extensionWatcher.untrack).toBeCalled();

expect(telemetry.track).toBeCalledWith('deactivateExtension', { extensionId });
});

Expand Down Expand Up @@ -2667,3 +2683,46 @@ describe('loading extension folders', () => {
});
});
});

test('reload extensions', async () => {
const extension = {
path: 'fakePath',
manifest: {
displayName: 'My Extension Display Name',
},
id: 'my.extensionId',
} as unknown as AnalyzedExtension;

// override deactivateExtension
const deactivateSpy = vi.spyOn(extensionLoader, 'deactivateExtension');
const analyzeExtensionSpy = vi.spyOn(extensionLoader, 'analyzeExtension');
const loadExtensionSpy = vi.spyOn(extensionLoader, 'loadExtension');
const analyzedExtension = {} as unknown as AnalyzedExtension;
analyzeExtensionSpy.mockResolvedValue(analyzedExtension);

const fakeDisposableObject = {
dispose: vi.fn(),
} as unknown as Disposable;
vi.mocked(notificationRegistry.addNotification).mockReturnValue(fakeDisposableObject);

// reload the extension
await extensionLoader.reloadExtension(extension, false);

expect(deactivateSpy).toBeCalledWith(extension.id);
expect(analyzeExtensionSpy).toBeCalledWith(extension.path, false);
expect(loadExtensionSpy).toBeCalledWith(analyzedExtension, true);

expect(vi.mocked(notificationRegistry.addNotification)).toBeCalledWith({
extensionId: extension.id,
title: 'Extension My Extension Display Name has been updated',
type: 'info',
});

// restore the spy
deactivateSpy.mockRestore();
analyzeExtensionSpy.mockRestore();
loadExtensionSpy.mockRestore();

// wait the notification is disposed
await vi.waitFor(() => expect(fakeDisposableObject.dispose).toBeCalled(), { timeout: 5_000 });
});
47 changes: 29 additions & 18 deletions packages/main/src/plugin/extension-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import type { Directories } from './directories.js';
import type { Event } from './events/emitter.js';
import { Emitter } from './events/emitter.js';
import { DEFAULT_TIMEOUT, ExtensionLoaderSettings } from './extension-loader-settings.js';
import type { ExtensionWatcher } from './extension-watcher.js';
import type { FilesystemMonitoring } from './filesystem-monitoring.js';
import type { IconRegistry } from './icon-registry.js';
import type { ImageCheckerImpl } from './image-checker.js';
Expand Down Expand Up @@ -141,7 +142,6 @@ export class ExtensionLoader {

protected activatedExtensions = new Map<string, ActivatedExtension>();
protected analyzedExtensions = new Map<string, AnalyzedExtension>();
private watcherExtensions = new Map<string, containerDesktopAPI.FileSystemWatcher>();
private reloadInProgressExtensions = new Map<string, boolean>();
protected extensionState = new Map<string, string>();
protected extensionStateErrors = new Map<string, unknown>();
Expand Down Expand Up @@ -194,6 +194,7 @@ export class ExtensionLoader {
private dialogRegistry: DialogRegistry,
private safeStorageRegistry: SafeStorageRegistry,
private certificates: Certificates,
private extensionWatcher: ExtensionWatcher,
) {
this.pluginsDirectory = directories.getPluginsDirectory();
this.pluginsScanDirectory = directories.getPluginsScanDirectory();
Expand Down Expand Up @@ -431,6 +432,13 @@ export class ExtensionLoader {

// now we have all extensions, we can load them
await this.loadExtensions(analyzedExtensions);

// handle the reload extensions callback
this.extensionWatcher.onNeedToReloadExtension(extension => {
this.reloadExtension(extension, false).catch((error: unknown) => {
console.error('error while reloading extension', error);
});
});
}

async analyzeExtension(extensionPath: string, removable: boolean): Promise<AnalyzedExtension> {
Expand Down Expand Up @@ -652,8 +660,7 @@ export class ExtensionLoader {
}

protected async reloadExtension(extension: AnalyzedExtension, removable: boolean): Promise<void> {
const inProgress = this.reloadInProgressExtensions.get(extension.id);
if (inProgress) {
if (this.reloadInProgressExtensions.has(extension.id)) {
return;
}

Expand All @@ -673,8 +680,18 @@ export class ExtensionLoader {
} catch (error) {
console.error('error while reloading extension', error);
} finally {
this.reloadInProgressExtensions.set(extension.id, false);
this.reloadInProgressExtensions.delete(extension.id);
}

const notification = this.notificationRegistry.addNotification({
extensionId: extension.id,
type: 'info',
title: `Extension ${extension.manifest.displayName} has been updated`,
});
// remove the notification after few seconds
setTimeout(() => {
notification.dispose();
}, 2_000);
}

async loadExtension(extension: AnalyzedExtension, checkForMissingDependencies = false): Promise<void> {
Expand Down Expand Up @@ -754,19 +771,10 @@ export class ExtensionLoader {

try {
// in development mode, watch if the extension is updated and reload it
if (import.meta.env.DEV && !this.watcherExtensions.has(extension.id)) {
const extensionWatcher = this.fileSystemMonitoring.createFileSystemWatcher(extension.path);
extensionWatcher.onDidChange(async () => {
// wait 1 second before trying to reload the extension
// this is to avoid reloading the extension while it is still being updated
setTimeout(() => {
this.reloadExtension(extension, extension.removable).catch((error: unknown) =>
console.error('error while reloading extension', error),
);
}, 1000);
});
this.watcherExtensions.set(extension.id, extensionWatcher);
if (import.meta.env.DEV) {
await this.extensionWatcher.monitor(extension);
}

if (!this.getDisabledExtensionIds().includes(extension.id)) {
const beforeLoadingRuntime = performance.now();
const runtime = this.loadRuntime(extension);
Expand Down Expand Up @@ -1697,8 +1705,10 @@ export class ExtensionLoader {
}
}

const watcher = this.watcherExtensions.get(extensionId);
watcher?.dispose();
// dispose resources for a given extension only if not in a reload
if (!this.reloadInProgressExtensions.has(extensionId)) {
this.extensionWatcher.untrack(extension);
}

const analyzedExtension = this.analyzedExtensions.get(extensionId);
// dispose subscriptions if any
Expand All @@ -1715,6 +1725,7 @@ export class ExtensionLoader {
Array.from(this.activatedExtensions.keys()).map(extensionId => this.deactivateExtension(extensionId)),
);
this.kubernetesClient.dispose();
this.extensionWatcher.dispose();
}

async startExtension(extensionId: string): Promise<void> {
Expand Down
68 changes: 68 additions & 0 deletions packages/main/src/plugin/extension-tsconfig-parser.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**********************************************************************
* Copyright (C) 2025 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/

import { existsSync } from 'node:fs';
import { sep } from 'node:path';

import type { FileMatcher, TsConfigResult } from 'get-tsconfig';
import { createFilesMatcher, getTsconfig } from 'get-tsconfig';
import { beforeEach, expect, test, vi } from 'vitest';

import { ExtensionTypeScriptConfigParser } from './extension-tsconfig-parser.js';

vi.mock('node:fs');
vi.mock('get-tsconfig');

beforeEach(() => {
vi.restoreAllMocks();
vi.resetAllMocks();
});

test('no tsconfig.json file means it returns undefined', async () => {
const fakePath = 'fakePath';
const extensionTypeScriptConfigParser = new ExtensionTypeScriptConfigParser(fakePath);

const result = await extensionTypeScriptConfigParser.analyze();

expect(existsSync).toHaveBeenCalledWith(expect.stringContaining(`fakePath${sep}tsconfig.json`));
expect(vi.mocked(getTsconfig)).not.toHaveBeenCalled();
expect(result).toBe(undefined);
});

test('with a tsconfig.json file means it returns undefined', async () => {
vi.mocked(existsSync).mockReturnValue(true);
const fakePath = 'fakePath';
const extensionTypeScriptConfigParser = new ExtensionTypeScriptConfigParser(fakePath);

const fakeTsConfig = {} as unknown as TsConfigResult;
const fakeFilesMatcher = {} as unknown as FileMatcher;
vi.mocked(getTsconfig).mockReturnValue(fakeTsConfig);
vi.mocked(createFilesMatcher).mockReturnValue(fakeFilesMatcher);

const result = await extensionTypeScriptConfigParser.analyze();
expect(existsSync).toHaveBeenCalledWith(expect.stringContaining(`fakePath${sep}tsconfig.json`));

// check we called the getTsconfig function
expect(vi.mocked(getTsconfig)).toHaveBeenCalled();

// check we called the createFilesMatcher function
expect(vi.mocked(createFilesMatcher)).toHaveBeenCalledWith(fakeTsConfig);

// check we have a valid file matcher in that case
expect(result).toBe(fakeFilesMatcher);
});
52 changes: 52 additions & 0 deletions packages/main/src/plugin/extension-tsconfig-parser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**********************************************************************
* Copyright (C) 2025 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/

import { existsSync } from 'node:fs';
import { resolve } from 'node:path';

import type { FileMatcher } from 'get-tsconfig';
import { createFilesMatcher, getTsconfig } from 'get-tsconfig';

/**
* Responsible to parse all the tsconfig.json files from an extension
* and provide a list of all pattern having source files
*/
export class ExtensionTypeScriptConfigParser {
#rootPath: string;

constructor(rootPath: string) {
this.#rootPath = rootPath;
}

async analyze(): Promise<FileMatcher | undefined> {
// read all tsconfig.json files not being in node_modules
// and parse them to extract all the patterns

// find all tsconfig.json files from rootPath folder
// do we have a tsconfig.json file in the rootPath ?
const tsConfigJsonPath = resolve(this.#rootPath, 'tsconfig.json');

if (existsSync(tsConfigJsonPath)) {
const tsConfig = getTsconfig(tsConfigJsonPath);
if (tsConfig) {
return createFilesMatcher(tsConfig);
}
}
return undefined;
}
}
Loading

0 comments on commit deed092

Please sign in to comment.