remove support for @optional service dependencies, microsoft#119440 (comment)

Author: jrieken

src/vs/platform/instantiation/common/instantiation.ts

@@ -62,7 +62,6 @@ export interface IConstructorSignature8<A1, A2, A3, A4, A5, A6, A7, A8, T> {
 
 export interface ServicesAccessor {
 	get<T>(id: ServiceIdentifier<T>): T;
-	get<T>(id: ServiceIdentifier<T>, isOptional: typeof optional): T | undefined;
 }
 
 export const IInstantiationService = createDecorator<IInstantiationService>('instantiationService');
@@ -158,17 +157,3 @@ export function createDecorator<T>(serviceId: string): ServiceIdentifier<T> {
 export function refineServiceDecorator<T1, T extends T1>(serviceIdentifier: ServiceIdentifier<T1>): ServiceIdentifier<T> {
 	return <ServiceIdentifier<T>>serviceIdentifier;
 }
-
-/**
- * Mark a service dependency as optional.
- * @deprecated Avoid, see https://github.com/microsoft/vscode/issues/119440
- */
-export function optional<T>(serviceIdentifier: ServiceIdentifier<T>) {
-
-	return function (target: Function, key: string, index: number) {
-		if (arguments.length !== 3) {
-			throw new Error('@optional-decorator can only be used to decorate a parameter');
-		}
-		storeServiceDependency(serviceIdentifier, target, index, true);
-	};
-}

src/vs/platform/instantiation/common/instantiationService.ts

@@ -7,7 +7,7 @@ import { IdleValue } from 'vs/base/common/async';
 import { illegalState } from 'vs/base/common/errors';
 import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
 import { Graph } from 'vs/platform/instantiation/common/graph';
-import { IInstantiationService, optional, ServiceIdentifier, ServicesAccessor, _util } from 'vs/platform/instantiation/common/instantiation';
+import { IInstantiationService, ServiceIdentifier, ServicesAccessor, _util } from 'vs/platform/instantiation/common/instantiation';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 
 // TRACING
@@ -45,14 +45,14 @@ export class InstantiationService implements IInstantiationService {
 		let _done = false;
 		try {
 			const accessor: ServicesAccessor = {
-				get: <T>(id: ServiceIdentifier<T>, isOptional?: typeof optional) => {
+				get: <T>(id: ServiceIdentifier<T>) => {
 
 					if (_done) {
 						throw illegalState('service accessor is only valid during the invocation of its target method');
 					}
 
 					const result = this._getOrCreateServiceInstance(id, _trace);
-					if (!result && isOptional !== optional) {
+					if (!result) {
 						throw new Error(`[invokeFunction] unknown service '${id}'`);
 					}
 					return result;

src/vs/platform/instantiation/test/common/instantiationService.test.ts

@@ -5,7 +5,7 @@
 
 import * as assert from 'assert';
 import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
-import { createDecorator, IInstantiationService, optional, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
+import { createDecorator, IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
 import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 
@@ -85,18 +85,7 @@ class TargetWithStaticParam {
 	}
 }
 
-class TargetNotOptional {
-	constructor(@IService1 service1: IService1, @IService2 service2: IService2) {
 
-	}
-}
-class TargetOptional {
-	constructor(@IService1 service1: IService1, @optional(IService2) service2: IService2) {
-		assert.ok(service1);
-		assert.strictEqual(service1.c, 1);
-		assert.ok(service2 === undefined);
-	}
-}
 
 class DependentServiceTarget {
 	constructor(@IDependentService d: IDependentService) {
@@ -181,13 +170,6 @@ suite('Instantiation Service', () => {
 		let service = new InstantiationService(collection);
 		service.createInstance(Service1Consumer);
 
-		// no IService2
-		assert.throws(() => service.createInstance(Target2Dep));
-		service.invokeFunction(function (a) {
-			assert.ok(a.get(IService1));
-			assert.ok(!a.get(IService2, optional));
-		});
-
 		collection.set(IService2, new Service2());
 
 		service.createInstance(Target2Dep);
@@ -197,18 +179,6 @@ suite('Instantiation Service', () => {
 		});
 	});
 
-	test('@Param - optional', function () {
-		let collection = new ServiceCollection([IService1, new Service1()]);
-		let service = new InstantiationService(collection, true);
-
-		service.createInstance(TargetOptional);
-		assert.throws(() => service.createInstance(TargetNotOptional));
-
-		service = new InstantiationService(collection, false);
-		service.createInstance(TargetOptional);
-		service.createInstance(TargetNotOptional);
-	});
-
 	// we made this a warning
 	// test('@Param - too many args', function () {
 	// 	let service = instantiationService.create(Object.create(null));
@@ -320,7 +290,6 @@ suite('Instantiation Service', () => {
 		function test(accessor: ServicesAccessor) {
 			assert.ok(accessor.get(IService1) instanceof Service1);
 			assert.throws(() => accessor.get(IService2));
-			assert.strictEqual(accessor.get(IService2, optional), undefined);
 			return true;
 		}
 		assert.strictEqual(service.invokeFunction(test), true);