From 8d8d07c6a6af50e060a0157a3d90dc16a13e55ec Mon Sep 17 00:00:00 2001 From: Kai Salmen Date: Sat, 7 Oct 2023 22:07:55 +0200 Subject: [PATCH] EditorAppBase no longer inherits code from derived classes --- packages/monaco-editor-react/src/index.tsx | 33 ++++---- .../src/editorAppBase.ts | 75 +++++-------------- .../src/editorAppClassic.ts | 30 +++++--- .../src/editorAppExtended.ts | 22 ++++-- packages/monaco-editor-wrapper/src/index.ts | 10 +-- packages/monaco-editor-wrapper/src/wrapper.ts | 8 +- .../test/editorAppBase.test.ts | 30 ++++---- .../test/wrapper.test.ts | 1 - 8 files changed, 94 insertions(+), 115 deletions(-) diff --git a/packages/monaco-editor-react/src/index.tsx b/packages/monaco-editor-react/src/index.tsx index a15b674..fe5dc6a 100644 --- a/packages/monaco-editor-react/src/index.tsx +++ b/packages/monaco-editor-react/src/index.tsx @@ -1,4 +1,4 @@ -import { EditorAppClassic, MonacoEditorLanguageClientWrapper, UserConfig, WorkerConfigDirect, WorkerConfigOptions, isAppConfigDifferent } from 'monaco-editor-wrapper'; +import { EditorAppClassic, EditorAppExtended, MonacoEditorLanguageClientWrapper, UserConfig, WorkerConfigDirect, WorkerConfigOptions } from 'monaco-editor-wrapper'; import { IDisposable } from 'monaco-editor'; import * as vscode from 'vscode'; import React, { CSSProperties } from 'react'; @@ -41,6 +41,8 @@ export class MonacoEditorReactComp extends React.Component { } let mustReInit = false; + const prevConfig = prevProps.userConfig.wrapperConfig.editorAppConfig; + const config = userConfig.wrapperConfig.editorAppConfig; const prevWorkerOptions = prevProps.userConfig.languageClientConfig?.options; const currentWorkerOptions = userConfig.languageClientConfig?.options; const prevIsWorker = (prevWorkerOptions?.$type === 'WorkerDirect'); @@ -59,26 +61,21 @@ export class MonacoEditorReactComp extends React.Component { mustReInit = true; } + if (prevConfig.$type === 'classic' && config.$type === 'classic') { + mustReInit = (wrapper?.getMonacoEditorApp() as EditorAppClassic).isAppConfigDifferent(prevConfig, config, false) === true; + } else if (prevConfig.$type === 'extended' && config.$type === 'extended') { + mustReInit = (wrapper?.getMonacoEditorApp() as EditorAppExtended).isAppConfigDifferent(prevConfig, config, false) === true; + } + if (mustReInit) { await this.handleReinit(); } else { - if (wrapper !== null) { - const prevConfig = prevProps.userConfig.wrapperConfig.editorAppConfig; - const config = userConfig.wrapperConfig.editorAppConfig; - const appConfigDifferent = isAppConfigDifferent(prevConfig, config, false, false); - - // we need to restart if the editor wrapper config changed - if (appConfigDifferent) { - await this.handleReinit(); - } else { - // the function now ensure a model update is only required if something else than the code changed - this.wrapper.updateModel(userConfig.wrapperConfig.editorAppConfig); - - if (prevConfig.$type === 'classic' && config.$type === 'classic') { - if (prevConfig.editorOptions !== config.editorOptions) { - (wrapper.getMonacoEditorApp() as EditorAppClassic).updateMonacoEditorOptions(config.editorOptions ?? {}); - } - } + // the function now ensure a model update is only required if something else than the code changed + this.wrapper.updateModel(userConfig.wrapperConfig.editorAppConfig); + + if (prevConfig.$type === 'classic' && config.$type === 'classic') { + if (prevConfig.editorOptions !== config.editorOptions) { + (wrapper.getMonacoEditorApp() as EditorAppClassic).updateMonacoEditorOptions(config.editorOptions ?? {}); } } } diff --git a/packages/monaco-editor-wrapper/src/editorAppBase.ts b/packages/monaco-editor-wrapper/src/editorAppBase.ts index 03af636..9ab48a0 100644 --- a/packages/monaco-editor-wrapper/src/editorAppBase.ts +++ b/packages/monaco-editor-wrapper/src/editorAppBase.ts @@ -1,10 +1,7 @@ import { editor, Uri } from 'monaco-editor'; import { createConfiguredEditor, createConfiguredDiffEditor, createModelReference, ITextFileEditorModel } from 'vscode/monaco'; import { IReference } from 'vscode/service-override/editor'; -import { WrapperConfig } from './wrapper.js'; import { updateUserConfiguration as vscodeUpdateUserConfiguration } from 'vscode/service-override/configuration'; -import { EditorAppConfigClassic } from './editorAppClassic.js'; -import { EditorAppConfigExtended } from './editorAppExtended.js'; export type ModelUpdate = { languageId: string; @@ -14,14 +11,21 @@ export type ModelUpdate = { codeOriginalUri?: string; } -export type EditorAppBaseConfig = ModelUpdate & { +export type EditorAppType = 'extended' | 'classic'; + +export type EditorAppConfigBase = ModelUpdate & { + $type: EditorAppType; useDiffEditor: boolean; domReadOnly?: boolean; readOnly?: boolean; awaitExtensionReadiness?: Array<() => Promise>; } -export type EditorAppType = 'extended' | 'classic'; +export enum ModelUpdateType { + none, + code, + model +} /** * This is the base class for both Monaco Ediotor Apps: @@ -44,8 +48,9 @@ export abstract class EditorAppBase { this.id = id; } - protected buildConfig(userAppConfig: EditorAppConfigExtended | EditorAppConfigClassic): EditorAppBaseConfig { + protected buildConfig(userAppConfig: EditorAppConfigBase): EditorAppConfigBase { return { + $type: userAppConfig.$type, languageId: userAppConfig.languageId, code: userAppConfig.code ?? '', codeOriginal: userAppConfig.codeOriginal ?? '', @@ -214,31 +219,31 @@ export abstract class EditorAppBase { return Promise.resolve(); } - protected async updateUserConfiguration(json?: string) { + updateMonacoEditorOptions(options: editor.IEditorOptions & editor.IGlobalEditorOptions) { + this.getEditor()?.updateOptions(options); + } + + async updateUserConfiguration(json?: string) { if (json) { return vscodeUpdateUserConfiguration(json); } return Promise.resolve(); } - abstract getAppType(): string; abstract init(): Promise; abstract specifyService(): editor.IEditorOverrideServices; abstract createEditors(container: HTMLElement): Promise; - abstract getConfig(): EditorAppConfigClassic | EditorAppConfigExtended; + abstract getConfig(): EditorAppConfigBase; abstract disposeApp(): void; + abstract isAppConfigDifferent(orgConfig: EditorAppConfigBase, config: EditorAppConfigBase, includeModelData: boolean): boolean; } -export const isExtendedEditorApp = (wrapperConfig: WrapperConfig) => { - return wrapperConfig.editorAppConfig?.$type === 'extended'; -}; - -export const isCodeUpdateRequired = (config: EditorAppBaseConfig, modelUpdate: ModelUpdate) => { +export const isCodeUpdateRequired = (config: EditorAppConfigBase, modelUpdate: ModelUpdate) => { const updateRequired = (modelUpdate.code !== undefined && modelUpdate.code !== config.code) || modelUpdate.codeOriginal !== config.codeOriginal; return updateRequired ? ModelUpdateType.code : ModelUpdateType.none; }; -export const isModelUpdateRequired = (config: EditorAppBaseConfig, modelUpdate: ModelUpdate): ModelUpdateType => { +export const isModelUpdateRequired = (config: EditorAppConfigBase, modelUpdate: ModelUpdate): ModelUpdateType => { const codeUpdate = isCodeUpdateRequired(config, modelUpdate); type ModelUpdateKeys = keyof typeof modelUpdate; @@ -249,43 +254,3 @@ export const isModelUpdateRequired = (config: EditorAppBaseConfig, modelUpdate: const updateRequired = propsModelUpdate.some(propCompare); return updateRequired ? ModelUpdateType.model : codeUpdate; }; - -export enum ModelUpdateType { - none, - code, - model -} - -export const isAppConfigDifferent = (orgConfig: EditorAppConfigClassic | EditorAppConfigExtended, - config: EditorAppConfigClassic | EditorAppConfigExtended, includeModelData: boolean, includeEditorOptions: boolean): boolean => { - - let different = includeModelData ? isModelUpdateRequired(orgConfig, config) !== ModelUpdateType.none : false; - if (orgConfig.$type === config.$type) { - - type ClassicKeys = keyof typeof orgConfig; - const propsClassic = ['useDiffEditor', 'readOnly', 'domReadOnly', 'awaitExtensionReadiness', 'automaticLayout', 'languageDef', 'languageExtensionConfig', 'theme', 'themeData']; - const propsClassicEditorOptions = ['editorOptions', 'diffEditorOptions']; - - const propCompareClassic = (name: string) => { - return orgConfig[name as ClassicKeys] !== config[name as ClassicKeys]; - }; - - const propsVscode = ['useDiffEditor', 'readOnly', 'domReadOnly', 'awaitExtensionReadiness', 'userConfiguration', 'extensions']; - type ExtendedKeys = keyof typeof orgConfig; - const propCompareExtended = (name: string) => { - return orgConfig[name as ExtendedKeys] !== config[name as ExtendedKeys]; - }; - - if (orgConfig.$type === 'classic' && config.$type === 'classic') { - different = different || propsClassic.some(propCompareClassic); - if (includeEditorOptions) { - different = different || propsClassicEditorOptions.some(propCompareClassic); - } - } else if (orgConfig.$type === 'extended' && config.$type === 'extended') { - different = different || propsVscode.some(propCompareExtended); - } - } else { - throw new Error('Provided configurations are not of the same type.'); - } - return different; -}; diff --git a/packages/monaco-editor-wrapper/src/editorAppClassic.ts b/packages/monaco-editor-wrapper/src/editorAppClassic.ts index 18c9d22..8e7d4e1 100644 --- a/packages/monaco-editor-wrapper/src/editorAppClassic.ts +++ b/packages/monaco-editor-wrapper/src/editorAppClassic.ts @@ -1,9 +1,9 @@ import { editor, languages } from 'monaco-editor'; -import { EditorAppBase, EditorAppBaseConfig, EditorAppType } from './editorAppBase.js'; +import { EditorAppBase, EditorAppConfigBase, ModelUpdateType, isModelUpdateRequired } from './editorAppBase.js'; import { UserConfig } from './wrapper.js'; import { Logger } from './logger.js'; -export type EditorAppConfigClassic = EditorAppBaseConfig & { +export type EditorAppConfigClassic = EditorAppConfigBase & { $type: 'classic'; automaticLayout?: boolean; theme?: editor.BuiltinTheme | string; @@ -43,10 +43,6 @@ export class EditorAppClassic extends EditorAppBase { this.config.themeData = userAppConfig.themeData ?? undefined; } - getAppType(): EditorAppType { - return 'classic'; - } - getConfig(): EditorAppConfigClassic { return this.config; } @@ -102,12 +98,26 @@ export class EditorAppClassic extends EditorAppBase { this.logger?.info('Init of Classic App was completed.'); } - updateMonacoEditorOptions(options: editor.IEditorOptions & editor.IGlobalEditorOptions) { - this.getEditor()?.updateOptions(options); - } - disposeApp(): void { this.disposeEditor(); this.disposeDiffEditor(); } + + isAppConfigDifferent(orgConfig: EditorAppConfigClassic, config: EditorAppConfigClassic, includeModelData: boolean): boolean { + let different = false; + if (includeModelData) { + different = isModelUpdateRequired(orgConfig, config) !== ModelUpdateType.none; + } + type ClassicKeys = keyof typeof orgConfig; + const propsClassic = ['useDiffEditor', 'readOnly', 'domReadOnly', 'awaitExtensionReadiness', 'automaticLayout', 'languageDef', 'languageExtensionConfig', 'theme', 'themeData']; + const propsClassicEditorOptions = ['editorOptions', 'diffEditorOptions']; + + const propCompareClassic = (name: string) => { + return orgConfig[name as ClassicKeys] !== config[name as ClassicKeys]; + }; + + different = different || propsClassic.some(propCompareClassic); + different = different || propsClassicEditorOptions.some(propCompareClassic); + return different; + } } diff --git a/packages/monaco-editor-wrapper/src/editorAppExtended.ts b/packages/monaco-editor-wrapper/src/editorAppExtended.ts index edbe37f..22654bc 100644 --- a/packages/monaco-editor-wrapper/src/editorAppExtended.ts +++ b/packages/monaco-editor-wrapper/src/editorAppExtended.ts @@ -3,7 +3,7 @@ import { IDisposable, editor } from 'monaco-editor'; import getThemeServiceOverride from '@codingame/monaco-vscode-theme-service-override'; import getTextmateServiceOverride from '@codingame/monaco-vscode-textmate-service-override'; import { whenReady as whenReadyTheme } from '@codingame/monaco-vscode-theme-defaults-default-extension'; -import { EditorAppBase, EditorAppBaseConfig, EditorAppType } from './editorAppBase.js'; +import { EditorAppBase, EditorAppConfigBase, ModelUpdateType, isModelUpdateRequired } from './editorAppBase.js'; import { registerExtension, IExtensionManifest, ExtensionHostKind } from 'vscode/extensions'; import { UserConfig } from './wrapper.js'; import { verifyUrlorCreateDataUrl } from './utils.js'; @@ -18,7 +18,7 @@ export type UserConfiguration = { json?: string; } -export type EditorAppConfigExtended = EditorAppBaseConfig & { +export type EditorAppConfigExtended = EditorAppConfigBase & { $type: 'extended'; extensions?: ExtensionConfig[]; userConfiguration?: UserConfiguration; @@ -57,10 +57,6 @@ export class EditorAppExtended extends EditorAppBase { this.config.userConfiguration = userAppConfig.userConfiguration ?? undefined; } - getAppType(): EditorAppType { - return 'extended'; - } - getConfig(): EditorAppConfigExtended { return this.config; } @@ -116,4 +112,18 @@ export class EditorAppExtended extends EditorAppBase { this.disposeDiffEditor(); this.extensionRegisterResults.forEach((k) => k?.dispose()); } + + isAppConfigDifferent(orgConfig: EditorAppConfigExtended, config: EditorAppConfigExtended, includeModelData: boolean): boolean { + let different = false; + if (includeModelData) { + different = isModelUpdateRequired(orgConfig, config) !== ModelUpdateType.none; + } + const propsExtended = ['useDiffEditor', 'readOnly', 'domReadOnly', 'awaitExtensionReadiness', 'userConfiguration', 'extensions']; + type ExtendedKeys = keyof typeof orgConfig; + const propCompareExtended = (name: string) => { + return orgConfig[name as ExtendedKeys] !== config[name as ExtendedKeys]; + }; + different = different || propsExtended.some(propCompareExtended); + return different; + } } diff --git a/packages/monaco-editor-wrapper/src/index.ts b/packages/monaco-editor-wrapper/src/index.ts index 0297234..6efc341 100644 --- a/packages/monaco-editor-wrapper/src/index.ts +++ b/packages/monaco-editor-wrapper/src/index.ts @@ -1,14 +1,12 @@ import { EditorAppBase, - isExtendedEditorApp, isCodeUpdateRequired, isModelUpdateRequired, - isAppConfigDifferent, ModelUpdateType } from './editorAppBase.js'; import type { - EditorAppBaseConfig, + EditorAppConfigBase, EditorAppType, ModelUpdate } from './editorAppBase.js'; @@ -18,7 +16,7 @@ import type { } from './editorAppClassic.js'; import { - EditorAppClassic, + EditorAppClassic } from './editorAppClassic.js'; import type { @@ -67,7 +65,7 @@ import { export type { WrapperConfig, - EditorAppBaseConfig, + EditorAppConfigBase, EditorAppType, EditorAppConfigClassic, ExtensionConfig, @@ -92,10 +90,8 @@ export { MonacoEditorLanguageClientWrapper, LanguageClientWrapper, EditorAppBase, - isExtendedEditorApp, isCodeUpdateRequired, isModelUpdateRequired, - isAppConfigDifferent, ModelUpdateType, EditorAppClassic, EditorAppExtended, diff --git a/packages/monaco-editor-wrapper/src/wrapper.ts b/packages/monaco-editor-wrapper/src/wrapper.ts index 8e1a3d1..5bb130d 100644 --- a/packages/monaco-editor-wrapper/src/wrapper.ts +++ b/packages/monaco-editor-wrapper/src/wrapper.ts @@ -3,7 +3,7 @@ import getConfigurationServiceOverride from '@codingame/monaco-vscode-configurat import { initServices, wasVscodeApiInitialized, InitializeServiceConfig, MonacoLanguageClient, mergeServices } from 'monaco-languageclient'; import { EditorAppExtended, EditorAppConfigExtended } from './editorAppExtended.js'; import { EditorAppClassic, EditorAppConfigClassic } from './editorAppClassic.js'; -import { ModelUpdate, isExtendedEditorApp } from './editorAppBase.js'; +import { ModelUpdate } from './editorAppBase.js'; import { LanguageClientConfig, LanguageClientWrapper } from './languageClientWrapper.js'; import { Logger, LoggerConfig } from './logger.js'; @@ -76,10 +76,10 @@ export class MonacoEditorLanguageClientWrapper { this.init(userConfig); - if (isExtendedEditorApp(userConfig.wrapperConfig)) { - this.editorApp = new EditorAppExtended(this.id, userConfig, this.logger); - } else { + if (userConfig.wrapperConfig.editorAppConfig.$type === 'classic') { this.editorApp = new EditorAppClassic(this.id, userConfig, this.logger); + } else { + this.editorApp = new EditorAppExtended(this.id, userConfig, this.logger); } await this.initServices(); diff --git a/packages/monaco-editor-wrapper/test/editorAppBase.test.ts b/packages/monaco-editor-wrapper/test/editorAppBase.test.ts index 128f43e..66e5673 100644 --- a/packages/monaco-editor-wrapper/test/editorAppBase.test.ts +++ b/packages/monaco-editor-wrapper/test/editorAppBase.test.ts @@ -1,17 +1,17 @@ import { describe, expect, test } from 'vitest'; -import { isAppConfigDifferent, isExtendedEditorApp, isModelUpdateRequired, EditorAppClassic, ModelUpdateType, EditorAppConfigExtended, EditorAppExtended } from 'monaco-editor-wrapper'; +import { isModelUpdateRequired, EditorAppClassic, ModelUpdateType, EditorAppConfigExtended, EditorAppExtended, EditorAppConfigClassic } from 'monaco-editor-wrapper'; import { createBaseConfig, createEditorAppConfig, createWrapperConfig } from './helper.js'; describe('Test EditorAppBase', () => { - test('isExtendedEditorApp: empty EditorAppConfigClassic', () => { + test('classic type: empty EditorAppConfigClassic', () => { const wrapperConfig = createWrapperConfig('classic'); - expect(isExtendedEditorApp(wrapperConfig)).toBeFalsy(); + expect(wrapperConfig.editorAppConfig.$type).toBe('classic'); }); - test('isExtendedEditorApp: empty EditorAppConfigExtended', () => { + test('extended type: empty EditorAppConfigExtended', () => { const wrapperConfig = createWrapperConfig('extended'); - expect(isExtendedEditorApp(wrapperConfig)).toBeTruthy(); + expect(wrapperConfig.editorAppConfig.$type).toBe('extended'); }); test('config defaults', () => { @@ -53,26 +53,28 @@ describe('Test EditorAppBase', () => { }); test('isAppConfigDifferent: classic', () => { - const orgConfig = createEditorAppConfig('classic'); - const config = createEditorAppConfig('classic'); - expect(isAppConfigDifferent(orgConfig, config, false, false)).toBeFalsy(); + const orgConfig = createEditorAppConfig('classic') as EditorAppConfigClassic; + const config = createEditorAppConfig('classic') as EditorAppConfigClassic; + const app = new EditorAppClassic('test', createBaseConfig('classic')); + expect(app.isAppConfigDifferent(orgConfig, config, false)).toBeFalsy(); config.code = 'test'; - expect(isAppConfigDifferent(orgConfig, config, false, false)).toBeFalsy(); - expect(isAppConfigDifferent(orgConfig, config, true, false)).toBeTruthy(); + expect(app.isAppConfigDifferent(orgConfig, config, false)).toBeFalsy(); + expect(app.isAppConfigDifferent(orgConfig, config, true)).toBeTruthy(); config.code = ''; config.useDiffEditor = true; - expect(isAppConfigDifferent(orgConfig, config, false, false)).toBeTruthy(); + expect(app.isAppConfigDifferent(orgConfig, config, false)).toBeTruthy(); }); test('isAppConfigDifferent: vscode', () => { const orgConfig = createEditorAppConfig('extended') as EditorAppConfigExtended; const config = createEditorAppConfig('extended') as EditorAppConfigExtended; - expect(isAppConfigDifferent(orgConfig, config, false, true)).toBeFalsy(); + const app = new EditorAppExtended('test', createBaseConfig('extended')); + expect(app.isAppConfigDifferent(orgConfig, config, false)).toBeFalsy(); config.code = 'test'; - expect(isAppConfigDifferent(orgConfig, config, true, false)).toBeTruthy(); + expect(app.isAppConfigDifferent(orgConfig, config, true)).toBeTruthy(); config.code = ''; config.extensions = [{ @@ -85,7 +87,7 @@ describe('Test EditorAppBase', () => { } } }]; - expect(isAppConfigDifferent(orgConfig, config, false, false)).toBeTruthy(); + expect(app.isAppConfigDifferent(orgConfig, config, false)).toBeTruthy(); }); }); diff --git a/packages/monaco-editor-wrapper/test/wrapper.test.ts b/packages/monaco-editor-wrapper/test/wrapper.test.ts index 4703a81..d3efe63 100644 --- a/packages/monaco-editor-wrapper/test/wrapper.test.ts +++ b/packages/monaco-editor-wrapper/test/wrapper.test.ts @@ -24,7 +24,6 @@ describe('Test MonacoEditorLanguageClientWrapper', () => { const app = wrapper.getMonacoEditorApp() as EditorAppClassic; expect(app).toBeDefined(); - expect(app.getAppType()).toBe('classic'); const appConfig = app.getConfig(); expect(appConfig.automaticLayout).toBeTruthy();