Skip to content

Commit 06aab29

Browse files
kaisalmenkaisalmen
committed
Fixed htmlContainter check was made too early
1 parent deb7996 commit 06aab29

File tree

9 files changed

+85
-66
lines changed

9 files changed

+85
-66
lines changed

‎packages/client/src/vscode/apiWrapper.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,6 @@ export class MonacoVscodeApiWrapper {
3636
private apiConfig: MonacoVscodeApiConfigRuntime;
3737

3838
constructor(apiConfig: MonacoVscodeApiConfig) {
39-
const viewsConfigType = apiConfig.viewsConfig.$type;
40-
if ((viewsConfigType === 'ViewsService' || viewsConfigType === 'WorkbenchService') &&
41-
apiConfig.viewsConfig.htmlContainer === undefined) {
42-
43-
throw new Error(`View Service Type "${viewsConfigType}" requires a HTMLElement.`);
44-
}
45-
4639
this.apiConfig = {
4740
...apiConfig,
4841
serviceOverrides: apiConfig.serviceOverrides ?? {},
@@ -93,8 +86,13 @@ export class MonacoVscodeApiWrapper {
9386

9487
protected async configureViewsServices() {
9588
const viewsConfigType = this.apiConfig.viewsConfig.$type;
96-
if (this.apiConfig.$type === 'classic' && (viewsConfigType === 'ViewsService' || viewsConfigType === 'WorkbenchService')) {
97-
throw new Error(`View Service Type "${viewsConfigType}" cannot be used with classic configuration.`);
89+
if (viewsConfigType === 'ViewsService' || viewsConfigType === 'WorkbenchService') {
90+
if (this.apiConfig.$type === 'classic') {
91+
throw new Error(`View Service Type "${viewsConfigType}" cannot be used with classic configuration.`);
92+
}
93+
if (this.apiConfig.viewsConfig.htmlContainer === undefined) {
94+
throw new Error(`View Service Type "${viewsConfigType}" requires a HTMLElement.`);
95+
}
9896
}
9997

10098
const envEnhanced = getEnhancedMonacoEnvironment();

‎packages/client/test/support/helper.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export const createEditorAppConfig = (codeResources: CodeResources): EditorAppCo
7171
};
7272
};
7373

74-
export const createDefaultMonacoVscodeApiConfig = (overallConfigType: OverallConfigType, htmlContainer: HTMLElement, viewsConfigType: ViewsConfigTypes): MonacoVscodeApiConfig => {
74+
export const createDefaultMonacoVscodeApiConfig = (overallConfigType: OverallConfigType, htmlContainer: HTMLElement | undefined, viewsConfigType: ViewsConfigTypes): MonacoVscodeApiConfig => {
7575
return {
7676
$type: overallConfigType,
7777
advanced: {

‎packages/client/test/vscode/manager.editorservice.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/* --------------------------------------------------------------------------------------------
2+
* Copyright (c) 2025 TypeFox and others.
3+
* Licensed under the MIT License. See LICENSE in the package root for license information.
4+
* ------------------------------------------------------------------------------------------ */
5+
import { MonacoVscodeApiWrapper } from 'monaco-languageclient/vscodeApiWrapper';
6+
import { describe, expect, test } from 'vitest';
7+
import { createDefaultMonacoVscodeApiConfig } from '../support/helper.js';
8+
9+
describe('MonacoVscodeApiWrapper Tests: Different config', () => {
10+
11+
test('Start MonacoVscodeApiWrapper with EditorService but no htmlContainer', async () => {
12+
const apiConfig = createDefaultMonacoVscodeApiConfig('extended', undefined, 'EditorService');
13+
14+
const apiWrapper = new MonacoVscodeApiWrapper(apiConfig!);
15+
const awaited = await apiWrapper.start();
16+
expect(awaited).toBeUndefined();
17+
});
18+
19+
});

‎packages/client/test/vscode/manager.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,5 @@ describe('MonacoVscodeApiWrapper Tests', () => {
123123
expect(() => apiWrapper.dispose()).not.toThrowError();
124124
await expect(await apiWrapper.initExtensions()).toBeUndefined();
125125
});
126+
126127
});

‎packages/client/test/vscode/manager.viewsserivce.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* --------------------------------------------------------------------------------------------
2+
* Copyright (c) 2025 TypeFox and others.
3+
* Licensed under the MIT License. See LICENSE in the package root for license information.
4+
* ------------------------------------------------------------------------------------------ */
5+
import { MonacoVscodeApiWrapper } from 'monaco-languageclient/vscodeApiWrapper';
6+
import { describe, expect, test } from 'vitest';
7+
import { createDefaultMonacoVscodeApiConfig } from '../support/helper.js';
8+
9+
describe('MonacoVscodeApiWrapper Tests: Different config', () => {
10+
11+
test('Start MonacoVscodeApiWrapper with ViewsService but no htmlContainer', async () => {
12+
const apiConfig = createDefaultMonacoVscodeApiConfig('extended', undefined, 'ViewsService');
13+
14+
const apiWrapper = new MonacoVscodeApiWrapper(apiConfig);
15+
await expect(async () => {
16+
await apiWrapper.start();
17+
}).rejects.toThrowError('View Service Type "ViewsService" requires a HTMLElement.');
18+
});
19+
20+
});

‎packages/client/test/vscode/manager.workbenchserivce.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* --------------------------------------------------------------------------------------------
2+
* Copyright (c) 2025 TypeFox and others.
3+
* Licensed under the MIT License. See LICENSE in the package root for license information.
4+
* ------------------------------------------------------------------------------------------ */
5+
import { MonacoVscodeApiWrapper } from 'monaco-languageclient/vscodeApiWrapper';
6+
import { describe, expect, test } from 'vitest';
7+
import { createDefaultMonacoVscodeApiConfig } from '../support/helper.js';
8+
9+
describe('MonacoVscodeApiWrapper Tests: Different config', () => {
10+
11+
test('Start MonacoVscodeApiWrapper with WorkbenchService but no htmlContainer', async () => {
12+
const apiConfig = createDefaultMonacoVscodeApiConfig('extended', undefined, 'WorkbenchService');
13+
14+
const apiWrapper = new MonacoVscodeApiWrapper(apiConfig);
15+
await expect(async () => {
16+
await apiWrapper.start();
17+
}).rejects.toThrowError('View Service Type "WorkbenchService" requires a HTMLElement.');
18+
});
19+
20+
});

‎packages/client/test/vscode/manager.wrongHtmlContainer.test.ts‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎packages/wrapper-react/src/index.tsx‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@ export type MonacoEditorProps = {
2828
originalTextValue?: string;
2929
}
3030

31-
// this must be outside of the component as this is valid across multiple instances
31+
// All must be outside of the component as they ars valid across all instances and should not be re-created
32+
let apiWrapperRef: MonacoVscodeApiWrapper | undefined;
3233
const lcsManager = new LanguageClientManager();
33-
3434
const runQueue: Array<{id: string, func: () => Promise<void>}> = [];
3535
let queueAwait: Promise<void> | undefined = undefined;
3636
let queueResolve: ((value: void | PromiseLike<void>) => void) | undefined = undefined;
3737

38+
3839
export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
3940
const {
4041
style,
@@ -54,7 +55,6 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
5455
originalTextValue
5556
} = props;
5657

57-
const apiWrapperRef = useRef<MonacoVscodeApiWrapper>(new MonacoVscodeApiWrapper(vscodeApiConfig));
5858
const haveEditorService = useRef(true);
5959
const currentEditorConfig = useRef<EditorAppConfig | undefined>(undefined);
6060
const editorAppRef = useRef<EditorApp>(null);
@@ -98,9 +98,9 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
9898

9999
const debugLogging = (id: string, useTime?: boolean) => {
100100
if (useTime === true) {
101-
apiWrapperRef.current.getLogger().debug(`${id}: ${Date.now()}`);
101+
apiWrapperRef?.getLogger().debug(`${id}: ${Date.now()}`);
102102
} else {
103-
apiWrapperRef.current.getLogger().debug(id);
103+
apiWrapperRef?.getLogger().debug(id);
104104
}
105105
};
106106

@@ -114,15 +114,15 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
114114

115115
useEffect(() => {
116116
// this is only available if EditorService is configured
117-
if (modifiedTextValue !== undefined && haveEditorService.current) {
117+
if (haveEditorService.current && modifiedTextValue !== undefined) {
118118
modifiedCode.current = modifiedTextValue;
119119
editorAppRef.current?.updateCode({modified: modifiedTextValue});
120120
}
121121
}, [modifiedTextValue]);
122122

123123
useEffect(() => {
124124
// this is only available if EditorService is configured
125-
if (originalTextValue !== undefined && haveEditorService.current) {
125+
if ( haveEditorService.current && originalTextValue !== undefined) {
126126
originalCode.current = originalTextValue;
127127
editorAppRef.current?.updateCode({original: originalTextValue});
128128
}
@@ -137,20 +137,22 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
137137
// init will only performed once
138138
if (envEnhanced.vscodeApiInitialising !== true) {
139139

140+
apiWrapperRef = new MonacoVscodeApiWrapper(vscodeApiConfig);
140141
const globalInitFunc = async () => {
141142
debugLogging('GLOBAL INIT', true);
143+
if (apiWrapperRef === undefined) throw new Error('Unexpected error occurred: apiWrapper is not available! Aborting...');
142144

143-
apiWrapperRef.current.overrideViewsConfig({
144-
$type: apiWrapperRef.current.getMonacoVscodeApiConfig().viewsConfig.$type,
145+
apiWrapperRef.overrideViewsConfig({
146+
$type: apiWrapperRef.getMonacoVscodeApiConfig().viewsConfig.$type,
145147
htmlContainer: containerRef.current!
146148
});
147-
await apiWrapperRef.current.start();
149+
await apiWrapperRef.start();
148150

149151
// set if editor mode is available, otherwise text bindings will not work
150152
haveEditorService.current = envEnhanced.viewServiceType === 'EditorService';
151-
lcsManager.setLogger(apiWrapperRef.current.getLogger());
153+
lcsManager.setLogger(apiWrapperRef.getLogger());
152154

153-
onVscodeApiInitDone?.(apiWrapperRef.current);
155+
onVscodeApiInitDone?.(apiWrapperRef);
154156
triggerQueue();
155157
debugLogging('GLOBAL INIT DONE', true);
156158
};

‎vitest.config.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ export const vitestConfig = {
4141
// '**/client/test/common/utils.test.ts',
4242
// '**/client/test/fs/endpoints/emptyEndpoint.test.ts',
4343
// '**/client/test/vscode/manager.test.ts',
44-
// '**/client/test/vscode/manager.wrongHtmlContainer.test.ts',
44+
// '**/client/test/vscode/manager.editorservice.test.ts',
45+
// '**/client/test/vscode/manager.viewsserivce.test.ts',
46+
// '**/client/test/vscode/manager.workbenchserivce.test.ts',
4547
// '**/client/test/wrapper/lcmanager.test.ts',
4648
// '**/client/test/wrapper/lcwrapper.test.ts',
4749
// '**/client/test/worker/workerFactory.test.ts',

0 commit comments

Comments
 (0)