Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move text changes handling from react component to the wrapper #849

Merged
merged 2 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions packages/examples/src/appPlayground/reactMain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@

import React, { StrictMode } from 'react';
import ReactDOM from 'react-dom/client';
import { MonacoEditorLanguageClientWrapper, type TextChanges } from 'monaco-editor-wrapper';
import { MonacoEditorLanguageClientWrapper } from 'monaco-editor-wrapper';
import { MonacoEditorReactComp } from '@typefox/monaco-editor-react';
import { configure } from './config.js';
import { configurePostStart } from './common.js';
import { disableElement } from '../common/client/utils.js';

export const runApplicationPlaygroundReact = async () => {

const onTextChanged = (textChanges: TextChanges) => {
console.log(`Dirty? ${textChanges.isDirty}\ntext: ${textChanges.modified}\ntextOriginal: ${textChanges.original}`);
};
const configResult = configure();
const root = ReactDOM.createRoot(document.getElementById('react-root')!);

Expand All @@ -26,7 +23,6 @@ export const runApplicationPlaygroundReact = async () => {
<div style={{ 'backgroundColor': '#1f1f1f' }}>
<MonacoEditorReactComp
wrapperConfig={configResult.wrapperConfig}
onTextChanged={onTextChanged}
onLoad={async (wrapper: MonacoEditorLanguageClientWrapper) => {
await configurePostStart(wrapper, configResult);
}}
Expand Down
6 changes: 3 additions & 3 deletions packages/examples/src/python/client/reactPython.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { RegisteredFileSystemProvider, registerFileSystemOverlay, RegisteredMemo
import React, { StrictMode } from 'react';
import ReactDOM from 'react-dom/client';
import { MonacoEditorReactComp } from '@typefox/monaco-editor-react';
import { MonacoEditorLanguageClientWrapper, type TextChanges } from 'monaco-editor-wrapper';
import { MonacoEditorLanguageClientWrapper, type TextContents } from 'monaco-editor-wrapper';
import { createWrapperConfig } from './config.js';
import badPyCode from '../../../resources/python/bad.py?raw';
import { disableElement } from '../../common/client/utils.js';
Expand All @@ -19,8 +19,8 @@ export const runPythonReact = async () => {
fileSystemProvider.registerFile(new RegisteredMemoryFile(badPyUri, badPyCode));
registerFileSystemOverlay(1, fileSystemProvider);

const onTextChanged = (textChanges: TextChanges) => {
console.log(`Dirty? ${textChanges.isDirty}\ntext: ${textChanges.modified}\ntextOriginal: ${textChanges.original}`);
const onTextChanged = (textChanges: TextContents) => {
console.log(`text: ${textChanges.modified}\ntextOriginal: ${textChanges.original}`);
};
const wrapperConfig = createWrapperConfig('/workspace', badPyCode, '/workspace/bad.py');
const root = ReactDOM.createRoot(document.getElementById('react-root')!);
Expand Down
39 changes: 3 additions & 36 deletions packages/wrapper-react/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
* Licensed under the MIT License. See LICENSE in the package root for license information.
* ------------------------------------------------------------------------------------------ */

import * as monaco from '@codingame/monaco-vscode-editor-api';
import React, { type CSSProperties, useCallback, useEffect, useRef } from 'react';
import { didModelContentChange, MonacoEditorLanguageClientWrapper, type TextChanges, type TextModels, type WrapperConfig } from 'monaco-editor-wrapper';
import { MonacoEditorLanguageClientWrapper, type TextContents, type WrapperConfig } from 'monaco-editor-wrapper';

export type MonacoEditorProps = {
style?: CSSProperties;
className?: string;
wrapperConfig: WrapperConfig,
onTextChanged?: (textChanges: TextChanges) => void;
onTextChanged?: (textChanges: TextContents) => void;
onLoad?: (wrapper: MonacoEditorLanguageClientWrapper) => void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
onError?: (e: any) => void;
Expand All @@ -29,7 +28,6 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {

const wrapperRef = useRef<MonacoEditorLanguageClientWrapper>(new MonacoEditorLanguageClientWrapper());
const containerRef = useRef<HTMLDivElement>(null);
const onTextChangedSubscriptions = useRef<monaco.IDisposable[]>([]);

useEffect(() => {
return () => {
Expand Down Expand Up @@ -80,27 +78,7 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
const startMonaco = useCallback(async () => {
if (containerRef.current) {
try {
wrapperRef.current.registerModelUpdate((textModels: TextModels) => {
if (textModels.modified !== undefined || textModels.original !== undefined) {
const newSubscriptions: monaco.IDisposable[] = [];

if (textModels.modified !== undefined) {
newSubscriptions.push(textModels.modified.onDidChangeContent(() => {
didModelContentChange(textModels, wrapperConfig.editorAppConfig?.codeResources, onTextChanged);
}));
}

if (textModels.original !== undefined) {
newSubscriptions.push(textModels.original.onDidChangeContent(() => {
didModelContentChange(textModels, wrapperConfig.editorAppConfig?.codeResources, onTextChanged);
}));
}
onTextChangedSubscriptions.current = newSubscriptions;
// do it initially
didModelContentChange(textModels, wrapperConfig.editorAppConfig?.codeResources, onTextChanged);
}
});

wrapperRef.current.registerTextChangeCallback(onTextChanged);
await wrapperRef.current.start();
onLoad?.(wrapperRef.current);
handleOnTextChanged();
Expand All @@ -117,10 +95,7 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
}, [onError, onLoad, onTextChanged]);

const handleOnTextChanged = useCallback(() => {
disposeOnTextChanged();

if (!onTextChanged) return;

}, [onTextChanged, wrapperConfig]);

const destroyMonaco = useCallback(async () => {
Expand All @@ -130,14 +105,6 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
// The language client may throw an error during disposal.
// This should not prevent us from continue working.
}
disposeOnTextChanged();
}, []);

const disposeOnTextChanged = useCallback(() => {
for (const subscription of onTextChangedSubscriptions.current) {
subscription.dispose();
}
onTextChangedSubscriptions.current = [];
}, []);

return (
Expand Down
6 changes: 3 additions & 3 deletions packages/wrapper-react/test/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { describe, expect, test } from 'vitest';
import { render, type RenderResult } from '@testing-library/react';
import React from 'react';
import { LogLevel } from '@codingame/monaco-vscode-api';
import { MonacoEditorLanguageClientWrapper, type TextChanges, type WrapperConfig } from 'monaco-editor-wrapper';
import { MonacoEditorLanguageClientWrapper, type TextContents, type WrapperConfig } from 'monaco-editor-wrapper';
import { MonacoEditorReactComp } from '@typefox/monaco-editor-react';
import { configureMonacoWorkers } from './helper.js';

Expand Down Expand Up @@ -60,7 +60,7 @@ describe('Test MonacoEditorReactComp', () => {
}
};

const textReceiverHello = (textChanges: TextChanges) => {
const textReceiverHello = (textChanges: TextContents) => {
expect(textChanges.modified).toEqual('hello world');
};

Expand Down Expand Up @@ -89,7 +89,7 @@ describe('Test MonacoEditorReactComp', () => {
};

let count = 0;
const textReceiver = (textChanges: TextChanges) => {
const textReceiver = (textChanges: TextContents) => {
// initial call
if (count === 0) {
expect(textChanges.modified).toBe('hello world');
Expand Down
13 changes: 2 additions & 11 deletions packages/wrapper/src/editorApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ export interface TextContents {
original?: string;
}

export type TextChanges = TextContents & {
isDirty: boolean;
isDirtyOriginal: boolean;
}

export interface CodeContent {
text: string;
enforceLanguageId?: string;
Expand Down Expand Up @@ -315,15 +310,11 @@ export const getEditorUri = (id: string, original: boolean, code: CodePlusUri |
}
};

export const didModelContentChange = (textModels: TextModels, codeResources?: CodeResources, onTextChanged?: (textChanges: TextChanges) => void) => {
export const didModelContentChange = (textModels: TextModels, onTextChanged?: (textChanges: TextContents) => void) => {
const modified = textModels.modified?.getValue() ?? '';
const original = textModels.original?.getValue() ?? '';
const isDirty = modified !== codeResources?.modified?.text;
const isDirtyOriginal = original !== codeResources?.original?.text;
onTextChanged?.({
modified,
original,
isDirty,
isDirtyOriginal
original
});
};
37 changes: 30 additions & 7 deletions packages/wrapper/src/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { MonacoLanguageClient } from 'monaco-languageclient';
import { initServices, type InitServicesInstructions, type VscodeApiConfig } from 'monaco-languageclient/vscode/services';
import { type Logger, ConsoleLogger } from 'monaco-languageclient/tools';
import { augmentVscodeApiConfig, checkServiceConsistency, type OverallConfigType, type VscodeServicesConfig } from './vscode/services.js';
import { type CodeResources, EditorApp, type EditorAppConfig, type ModelRefs, type TextContents, type TextModels, verifyUrlOrCreateDataUrl } from './editorApp.js';
import { type CodeResources, didModelContentChange, EditorApp, type EditorAppConfig, type ModelRefs, type TextContents, type TextModels, verifyUrlOrCreateDataUrl } from './editorApp.js';
import { type LanguageClientConfig, LanguageClientWrapper } from './languageClientWrapper.js';

export interface ExtensionConfig {
Expand Down Expand Up @@ -40,7 +40,8 @@ export class MonacoEditorLanguageClientWrapper {
private id: string;
private editorApp?: EditorApp;
private extensionRegisterResults: Map<string, | RegisterExtensionResult> = new Map();
private disposableStore?: DisposableStore;
private disposableStoreExtensions?: DisposableStore;
private disposableStoreMonaco?: DisposableStore;
private languageClientWrappers: Map<string, LanguageClientWrapper> = new Map();
private wrapperConfig?: WrapperConfig;
private logger: Logger = new ConsoleLogger();
Expand Down Expand Up @@ -74,7 +75,8 @@ export class MonacoEditorLanguageClientWrapper {
// Always dispose old instances before start and
// ensure disposable store is available again after dispose
this.dispose(false);
this.disposableStore = new DisposableStore();
this.disposableStoreExtensions = new DisposableStore();
this.disposableStoreMonaco = new DisposableStore();

this.id = wrapperConfig.id ?? Math.floor(Math.random() * 101).toString();

Expand Down Expand Up @@ -133,7 +135,7 @@ export class MonacoEditorLanguageClientWrapper {
this.extensionRegisterResults.set(manifest.name, extRegResult);
if (extensionConfig.filesOrContents && Object.hasOwn(extRegResult, 'registerFileUrl')) {
for (const entry of extensionConfig.filesOrContents) {
this.disposableStore?.add(extRegResult.registerFileUrl(entry[0], verifyUrlOrCreateDataUrl(entry[1])));
this.disposableStoreExtensions?.add(extRegResult.registerFileUrl(entry[0], verifyUrlOrCreateDataUrl(entry[1])));
}
}
allPromises.push(extRegResult.whenReady());
Expand Down Expand Up @@ -276,8 +278,28 @@ export class MonacoEditorLanguageClientWrapper {
return this.editorApp?.updateCodeResources(codeResources);
}

registerModelUpdate(modelUpdateCallback: (textModels: TextModels) => void) {
this.editorApp?.registerModelUpdate(modelUpdateCallback);
registerTextChangeCallback(onTextChanged?: (textChanges: TextContents) => void) {
this.editorApp?.registerModelUpdate((textModels: TextModels) => {
// clear on new registration
this.disposableStoreMonaco?.clear();

if (textModels.modified !== undefined || textModels.original !== undefined) {

if (textModels.modified !== undefined) {
this.disposableStoreMonaco?.add(textModels.modified.onDidChangeContent(() => {
didModelContentChange(textModels, onTextChanged);
}));
}

if (textModels.original !== undefined) {
this.disposableStoreMonaco?.add(textModels.original.onDidChangeContent(() => {
didModelContentChange(textModels, onTextChanged);
}));
}
// do it initially
didModelContentChange(textModels, onTextChanged);
}
});
}

updateEditorModels(modelRefs: ModelRefs) {
Expand All @@ -302,7 +324,8 @@ export class MonacoEditorLanguageClientWrapper {
this.editorApp = undefined;

this.extensionRegisterResults.forEach((k) => k.dispose());
this.disposableStore?.dispose();
this.disposableStoreExtensions?.dispose();
this.disposableStoreMonaco?.dispose();

if (doDisposeLanguageClients) {
await disposeLanguageClients(this.languageClientWrappers.values(), false);
Expand Down
57 changes: 47 additions & 10 deletions packages/wrapper/test/wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@

import * as vscode from 'vscode';
import { createModelReference } from '@codingame/monaco-vscode-api/monaco';
import { describe, expect, test } from 'vitest';
import { MonacoEditorLanguageClientWrapper } from 'monaco-editor-wrapper';
import { describe, expect, test, vi } from 'vitest';
import { MonacoEditorLanguageClientWrapper, type TextContents } from 'monaco-editor-wrapper';
import { createMonacoEditorDiv, createWrapperConfigClassicApp, createWrapperConfigExtendedApp } from './helper.js';
import { IConfigurationService, StandaloneServices } from '@codingame/monaco-vscode-api';

const createMewModelReference = async () => {
const uri = vscode.Uri.parse('/workspace/statemachineUri.statemachine');
return await createModelReference(uri, 'text');
};

describe('Test MonacoEditorLanguageClientWrapper', () => {

test('New wrapper has undefined editor', () => {
Expand Down Expand Up @@ -53,7 +58,7 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
}).rejects.toThrowError('init was already performed. Please call dispose first if you want to re-start.');
});

test('code resources main', async () => {
test('Code resources main', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigClassicApp();
Expand All @@ -66,7 +71,7 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
app?.disposeApp();
});

test('code resources original', async () => {
test('Code resources original', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigClassicApp();
Expand All @@ -85,7 +90,7 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
app?.disposeApp();
});

test('code resources main and original', async () => {
test('Code resources main and original', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigClassicApp();
Expand All @@ -110,7 +115,7 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
app?.disposeApp();
});

test('code resources empty', async () => {
test('Code resources empty', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigClassicApp();
Expand All @@ -123,7 +128,7 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
expect(modelRefs?.modelRefOriginal).toBeUndefined();
});

test('code resources model direct', async () => {
test('Code resources model direct', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigClassicApp();
Expand All @@ -133,10 +138,8 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {
const app = wrapper.getMonacoEditorApp();

// here the modelReference is created manually and given to the updateEditorModels of the wrapper
const uri = vscode.Uri.parse('/workspace/statemachineUri.statemachine');
const modelRefModified = await createModelReference(uri, 'text');
wrapper.updateEditorModels({
modelRefModified
modelRefModified: await createMewModelReference()
});

const modelRefs = app?.getModelRefs();
Expand Down Expand Up @@ -274,4 +277,38 @@ describe('Test MonacoEditorLanguageClientWrapper', () => {

expect(wrapper.getEditor()?.getModel()?.getValue()).toEqual('console.log("Goodbye World");');
});

test('Verify registerTextChangeCallback', async () => {
createMonacoEditorDiv();
const wrapper = new MonacoEditorLanguageClientWrapper();
const wrapperConfig = createWrapperConfigExtendedApp();

const onTextChanged = (textChanges: TextContents) => {
console.log(`text: ${textChanges.modified}\ntextOriginal: ${textChanges.original}`);
};

await wrapper.init(wrapperConfig);

// eslint-disable-next-line dot-notation
const disposableStoreMonaco = wrapper['disposableStoreMonaco'];
expect(disposableStoreMonaco).toBeDefined();

wrapper.registerTextChangeCallback(onTextChanged);

// eslint-disable-next-line dot-notation
const spyModelUpdateCallback = vi.spyOn(wrapper['editorApp'], 'modelUpdateCallback');
const spyDisposableStoreMonaco = vi.spyOn(disposableStoreMonaco, 'clear');

await wrapper.start();

expect(spyModelUpdateCallback).toHaveBeenCalledTimes(1);
expect(spyDisposableStoreMonaco).toHaveBeenCalledTimes(1);

wrapper.updateEditorModels({
modelRefModified: await createMewModelReference()
});

expect(spyModelUpdateCallback).toHaveBeenCalledTimes(2);
expect(spyDisposableStoreMonaco).toHaveBeenCalledTimes(2);
});
});