Move Theme Loading into Inversify Lifecycle (#11213)

CHANGELOG.md

@@ -10,6 +10,7 @@
 - [plugin] Add support for property `color` of `ThemeIcon`. [#11243](https://github.com/eclipse-theia/theia/pull/11243) - Contributed on behalf of STMicroelectronics
 - [plugin] Add support for `TreeItemLabel` in `TreeItem`. [#11288](https://github.com/eclipse-theia/theia/pull/11288) - Contributed on behalf of STMicroelectronics
 - [plugin] Support `TextEditor#show()` and `TextEditor#hide()` [#11168](https://github.com/eclipse-theia/theia/pull/11168) - Contributed on behalf of STMicroelectronics
+- [core, monaco] refactored theme initialization to occur within application lifecycle rather than at import time. [#11213](https://github.com/eclipse-theia/theia/pull/11213)
 
 <a name="breaking_changes_1.27.0">[Breaking Changes:](#breaking_changes_1.27.0)</a>
 
@@ -54,6 +55,8 @@
 - [workspace] removed deprecated `getDefaultWorkspacePath` [#11185](https://github.com/eclipse-theia/theia/pull/11185)
 - [vsx-registry] removed deprecated `VSXExtensionsCommands` re-export [#11185](https://github.com/eclipse-theia/theia/pull/11185)
 - [plugin] Remove `TreeItem2` from proposed plugin API. `TreeItem` can be used instead. [#11288](https://github.com/eclipse-theia/theia/pull/11288) - Contributed on behalf of STMicroelectronics
+- [core] removed `ThemeService.get()`; inject the `ThemeService` instead. Removed `ColorApplicationContribution.initBackground()`; by default the `editor.background` color variable will be initialized through the normal theme initialization process. It is now expected that the `ThemeService` will call `this.deferredInitializer.resolve()` when the `ThemeService` finishes its initialization. Failure to do so in any overrides may cause failures to apply default themes.  [#11213](https://github.com/eclipse-theia/theia/pull/11213)
+- [monaco] removed static methods `init()`, `register()`, `restore(), `updateBodyUiTheme()` from `MonacoThemingService`; use instance methods `initialize()`, `registerParsedTheme()`, `restore()`, `updateBodyUiTheme()` instead. Removed `MonacoThemeRegistry.SINGLETON`, inject `MonacoThemeRegistry` instead. [#11213](https://github.com/eclipse-theia/theia/pull/11213)
 
 ## v1.26.0 - 5/26/2022
 

dev-packages/application-manager/src/generator/frontend-generator.ts

@@ -89,9 +89,6 @@ self.MonacoEnvironment = {
 }
 `)}
 
-const { ThemeService } = require('@theia/core/lib/browser/theming');
-ThemeService.get().loadUserTheme();
-
 const preloader = require('@theia/core/lib/browser/preloader');
 
 // We need to fetch some data from the backend before the frontend starts (nls, os)

packages/core/src/browser/color-application-contribution.ts

@@ -21,6 +21,7 @@ import { ThemeService } from './theming';
 import { FrontendApplicationContribution } from './frontend-application';
 import { ContributionProvider } from '../common/contribution-provider';
 import { Disposable, DisposableCollection } from '../common/disposable';
+import { DEFAULT_BACKGROUND_COLOR_STORAGE_KEY } from './frontend-application-config-provider';
 
 export const ColorContribution = Symbol('ColorContribution');
 export interface ColorContribution {
@@ -39,18 +40,17 @@ export class ColorApplicationContribution implements FrontendApplicationContribu
     @inject(ContributionProvider) @named(ColorContribution)
     protected readonly colorContributions: ContributionProvider<ColorContribution>;
 
-    private static themeBackgroundId = 'theme.background';
+    @inject(ThemeService) protected readonly themeService: ThemeService;
 
     onStart(): void {
         for (const contribution of this.colorContributions.getContributions()) {
             contribution.registerColors(this.colors);
         }
-
-        this.updateThemeBackground();
-        ThemeService.get().onDidColorThemeChange(() => this.updateThemeBackground());
-
-        this.update();
-        ThemeService.get().onDidColorThemeChange(() => this.update());
+        this.themeService.initialized.then(() => this.update());
+        this.themeService.onDidColorThemeChange(() => {
+            this.update();
+            this.updateThemeBackground();
+        });
         this.colors.onDidChange(() => this.update());
     }
 
@@ -60,7 +60,7 @@ export class ColorApplicationContribution implements FrontendApplicationContribu
             return;
         }
         this.toUpdate.dispose();
-        const theme = 'theia-' + ThemeService.get().getCurrentTheme().type;
+        const theme = 'theia-' + this.themeService.getCurrentTheme().type;
         document.body.classList.add(theme);
         this.toUpdate.push(Disposable.create(() => document.body.classList.remove(theme)));
 
@@ -81,16 +81,9 @@ export class ColorApplicationContribution implements FrontendApplicationContribu
     protected updateThemeBackground(): void {
         const color = this.colors.getCurrentColor('editor.background');
         if (color) {
-            window.localStorage.setItem(ColorApplicationContribution.themeBackgroundId, color);
+            window.localStorage.setItem(DEFAULT_BACKGROUND_COLOR_STORAGE_KEY, color);
         } else {
-            window.localStorage.removeItem(ColorApplicationContribution.themeBackgroundId);
+            window.localStorage.removeItem(DEFAULT_BACKGROUND_COLOR_STORAGE_KEY);
         }
     }
-
-    static initBackground(): void {
-        const value = window.localStorage.getItem(this.themeBackgroundId) || '#1d1d1d';
-        const documentElement = document.documentElement;
-        documentElement.style.setProperty('--theia-editor-background', value);
-    }
-
 }

packages/core/src/browser/frontend-application-config-provider.ts

@@ -16,6 +16,8 @@
 
 import { FrontendApplicationConfig, deepmerge } from '@theia/application-package/lib/application-props';
 
+export const DEFAULT_BACKGROUND_COLOR_STORAGE_KEY = 'theme.background';
+
 export class FrontendApplicationConfigProvider {
 
     private static KEY = Symbol('FrontendApplicationConfigProvider');

packages/core/src/browser/frontend-application-module.ts

@@ -129,8 +129,6 @@ import { MarkdownRenderer, MarkdownRendererFactory, MarkdownRendererImpl } from
 
 export { bindResourceProvider, bindMessageService, bindPreferenceService };
 
-ColorApplicationContribution.initBackground();
-
 export const frontendApplicationModule = new ContainerModule((bind, _unbind, _isBound, _rebind) => {
     bind(NoneIconTheme).toSelf().inSingletonScope();
     bind(LabelProviderContribution).toService(NoneIconTheme);
@@ -338,7 +336,7 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
         return connection.createProxy<EnvVariablesServer>(envVariablesPath);
     }).inSingletonScope();
 
-    bind(ThemeService).toDynamicValue(() => ThemeService.get());
+    bind(ThemeService).toSelf().inSingletonScope();
 
     bindCorePreferences(bind);
 

packages/core/src/browser/preloader.ts

@@ -17,7 +17,7 @@
 import { nls } from '../common/nls';
 import { Endpoint } from './endpoint';
 import { OS } from '../common/os';
-import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
+import { DEFAULT_BACKGROUND_COLOR_STORAGE_KEY, FrontendApplicationConfigProvider } from './frontend-application-config-provider';
 
 function fetchFrom(path: string): Promise<Response> {
     const endpoint = new Endpoint({ path }).getRestUrl().toString();
@@ -47,9 +47,16 @@ async function loadBackendOS(): Promise<void> {
     OS.backend.type = () => osType;
 }
 
+function initBackground(): void {
+    const value = window.localStorage.getItem(DEFAULT_BACKGROUND_COLOR_STORAGE_KEY) || '#1d1d1d';
+    const documentElement = document.documentElement;
+    documentElement.style.setProperty('--theia-editor-background', value);
+}
+
 export async function preload(): Promise<void> {
     await Promise.allSettled([
         loadTranslations(),
-        loadBackendOS()
+        loadBackendOS(),
+        initBackground(),
     ]);
 }

packages/core/src/browser/shell/shell-layout-restorer.ts

@@ -123,11 +123,9 @@ export class ShellLayoutRestorer implements CommandContribution {
     protected storageKey = 'layout';
     protected shouldStoreLayout: boolean = true;
 
-    @inject(ContributionProvider) @named(ApplicationShellLayoutMigration)
-    protected readonly migrations: ContributionProvider<ApplicationShellLayoutMigration>;
-
-    @inject(WindowService)
-    protected readonly windowService: WindowService;
+    @inject(ContributionProvider) @named(ApplicationShellLayoutMigration) protected readonly migrations: ContributionProvider<ApplicationShellLayoutMigration>;
+    @inject(WindowService) protected readonly windowService: WindowService;
+    @inject(ThemeService) protected readonly themeService: ThemeService;
 
     constructor(
         @inject(WidgetManager) protected widgetManager: WidgetManager,
@@ -145,7 +143,7 @@ export class ShellLayoutRestorer implements CommandContribution {
             this.logger.info('>>> Resetting layout...');
             this.shouldStoreLayout = false;
             this.storageService.setData(this.storageKey, undefined);
-            ThemeService.get().reset(); // Theme service cannot use DI, so the current theme ID is stored elsewhere. Hence the explicit reset.
+            this.themeService.reset();
             this.logger.info('<<< The layout has been successfully reset.');
             this.windowService.reload();
         }

packages/core/src/browser/theming.ts

@@ -19,26 +19,26 @@ import { Disposable } from '../common/disposable';
 import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
 import { ApplicationProps } from '@theia/application-package/lib/application-props';
 import { Theme, ThemeChangeEvent } from '../common/theme';
+import { injectable, postConstruct } from 'inversify';
+import { Deferred } from '../common/promise-util';
 
-export const ThemeServiceSymbol = Symbol('ThemeService');
-
+@injectable()
 export class ThemeService {
 
     protected themes: { [id: string]: Theme } = {};
     protected activeTheme: Theme | undefined;
     protected readonly themeChange = new Emitter<ThemeChangeEvent>();
+    protected readonly deferredInitializer = new Deferred();
+    get initialized(): Promise<void> {
+        return this.deferredInitializer.promise;
+    }
 
     readonly onDidColorThemeChange: Event<ThemeChangeEvent> = this.themeChange.event;
 
-    static get(): ThemeService {
-        const global = window as any; // eslint-disable-line @typescript-eslint/no-explicit-any
-        if (!global[ThemeServiceSymbol]) {
-            const themeService = new ThemeService();
-            themeService.register(...BuiltinThemeProvider.themes);
-            themeService.startupTheme();
-            global[ThemeServiceSymbol] = themeService;
-        }
-        return global[ThemeServiceSymbol];
+    @postConstruct()
+    protected init(): void {
+        this.register(...BuiltinThemeProvider.themes);
+        this.loadUserTheme();
     }
 
     register(...themes: Theme[]): Disposable {
@@ -89,6 +89,7 @@ export class ThemeService {
     loadUserTheme(): void {
         const theme = this.getCurrentTheme();
         this.setCurrentTheme(theme.id);
+        this.deferredInitializer.resolve();
     }
 
     setCurrentTheme(themeId: string): void {
@@ -126,7 +127,6 @@ export class ThemeService {
     reset(): void {
         this.setCurrentTheme(this.defaultTheme.id);
     }
-
 }
 
 export class BuiltinThemeProvider {

packages/monaco/src/browser/monaco-frontend-application-contribution.ts

@@ -14,7 +14,7 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { injectable, inject } from '@theia/core/shared/inversify';
+import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
 import { FrontendApplicationContribution, PreferenceSchemaProvider, QuickAccessRegistry } from '@theia/core/lib/browser';
 import { MonacoSnippetSuggestProvider } from './monaco-snippet-suggest-provider';
 import * as monaco from '@theia/monaco-editor-core';
@@ -29,6 +29,7 @@ import { ITextModelService } from '@theia/monaco-editor-core/esm/vs/editor/commo
 import { IContextKeyService } from '@theia/monaco-editor-core/esm/vs/platform/contextkey/common/contextkey';
 import { IContextMenuService } from '@theia/monaco-editor-core/esm/vs/platform/contextview/browser/contextView';
 import { MonacoContextMenuService } from './monaco-context-menu';
+import { MonacoThemingService } from './monaco-theming-service';
 
 @injectable()
 export class MonacoFrontendApplicationContribution implements FrontendApplicationContribution {
@@ -54,7 +55,10 @@ export class MonacoFrontendApplicationContribution implements FrontendApplicatio
     @inject(MonacoContextMenuService)
     protected readonly contextMenuService: MonacoContextMenuService;
 
-    async initialize(): Promise<void> {
+    @inject(MonacoThemingService) protected readonly monacoThemingService: MonacoThemingService;
+
+    @postConstruct()
+    protected init(): void {
         const { codeEditorService, textModelService, contextKeyService, contextMenuService } = this;
         StandaloneServices.initialize({
             [ICodeEditorService.toString()]: codeEditorService,
@@ -77,6 +81,9 @@ export class MonacoFrontendApplicationContribution implements FrontendApplicatio
             this.preferenceSchema.registerOverrideIdentifier(language.id);
             registerLanguage(language);
         };
+
+        this.monacoThemingService.initialize();
     }
 
+    initialize(): void { }
 }

packages/monaco/src/browser/monaco-frontend-module.ts

@@ -88,11 +88,11 @@ import { StandaloneConfigurationService, StandaloneServices } from '@theia/monac
 import { Configuration } from '@theia/monaco-editor-core/esm/vs/platform/configuration/common/configurationModels';
 import { MarkdownRenderer } from '@theia/core/lib/browser/markdown-rendering/markdown-renderer';
 import { MonacoMarkdownRenderer } from './markdown-renderer/monaco-markdown-renderer';
+import { ThemeService } from '@theia/core/lib/browser/theming';
+import { ThemeServiceWithDB } from './monaco-indexed-db';
 
 decorate(injectable(), VSCodeContextKeyService);
 
-MonacoThemingService.init();
-
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
     bind(MonacoThemingService).toSelf().inSingletonScope();
 
@@ -183,6 +183,9 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
 
     bind(MonacoColorRegistry).toSelf().inSingletonScope();
     rebind(ColorRegistry).toService(MonacoColorRegistry);
+
+    bind(ThemeServiceWithDB).toSelf().inSingletonScope();
+    rebind(ThemeService).toService(ThemeServiceWithDB);
 });
 
 export const MonacoConfigurationService = Symbol('MonacoConfigurationService');

packages/monaco/src/browser/monaco-indexed-db.ts

@@ -16,11 +16,11 @@
 
 import * as idb from 'idb';
 import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable';
-import { BuiltinThemeProvider, ThemeService, ThemeServiceSymbol } from '@theia/core/lib/browser/theming';
-import { Theme } from '@theia/core/lib/common/theme';
+import { BuiltinThemeProvider, ThemeService } from '@theia/core/lib/browser/theming';
 import * as monaco from '@theia/monaco-editor-core';
-
-type ThemeMix = import('./textmate/monaco-theme-registry').ThemeMix;
+import { injectable } from '@theia/core/shared/inversify';
+import type { ThemeMix } from './textmate/monaco-theme-registry';
+import { Theme } from '@theia/core/lib/common/theme';
 
 let _monacoDB: Promise<idb.IDBPDatabase> | undefined;
 if ('indexedDB' in window) {
@@ -111,27 +111,20 @@ async function getThemeFromDB(id: string): Promise<Theme | undefined> {
     return matchingState && stateToTheme(matchingState);
 }
 
+@injectable()
 export class ThemeServiceWithDB extends ThemeService {
-    static override get(): ThemeService {
-        const global = window as any; // eslint-disable-line @typescript-eslint/no-explicit-any
-        if (!global[ThemeServiceSymbol]) {
-            const themeService = new ThemeServiceWithDB();
-            themeService.register(...BuiltinThemeProvider.themes);
-            themeService.startupTheme();
-            global[ThemeServiceSymbol] = themeService;
-        }
-        return global[ThemeServiceSymbol];
-    }
-
     override loadUserTheme(): void {
         this.loadUserThemeWithDB();
     }
 
     protected async loadUserThemeWithDB(): Promise<void> {
         const themeId = window.localStorage.getItem('theme') || this.defaultTheme.id;
         const theme = this.themes[themeId] ?? await getThemeFromDB(themeId) ?? this.defaultTheme;
+        // In case the theme comes from the DB.
+        if (!this.themes[theme.id]) {
+            this.themes[theme.id] = theme;
+        }
         this.setCurrentTheme(theme.id);
+        this.deferredInitializer.resolve();
     }
 }
-
-ThemeService.get = ThemeServiceWithDB.get;