Skip to content
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
40 changes: 40 additions & 0 deletions src/vs/platform/instantiation/common/instantiationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
*--------------------------------------------------------------------------------------------*/

import { IdleValue } from 'vs/base/common/async';
import { Event } from 'vs/base/common/event';
import { illegalState } from 'vs/base/common/errors';
import { toDisposable } from 'vs/base/common/lifecycle';
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { Graph } from 'vs/platform/instantiation/common/graph';
import { IInstantiationService, ServiceIdentifier, ServicesAccessor, _util } from 'vs/platform/instantiation/common/instantiation';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
import { LinkedList } from 'vs/base/common/linkedList';

// TRACING
const _enableAllTracing = false
Expand Down Expand Up @@ -246,15 +249,52 @@ export class InstantiationService implements IInstantiationService {
// Return a proxy object that's backed by an idle value. That
// strategy is to instantiate services in our idle time or when actually
// needed but not when injected into a consumer

// return "empty events" when the service isn't instantiated yet
const earlyListeners = new Map<string, LinkedList<Parameters<Event<any>>>>();

const idle = new IdleValue<any>(() => {
const result = child._createInstance<T>(ctor, args, _trace);

// early listeners that we kept are now being subscribed to
// the real service
for (const [key, values] of earlyListeners) {
const candidate = <Event<any>>(<any>result)[key];
if (typeof candidate === 'function') {
for (const listener of values) {
candidate.apply(result, listener);
}
}
}
earlyListeners.clear();

return result;
});
return <T>new Proxy(Object.create(null), {
get(target: any, key: PropertyKey): any {

if (!idle.isInitialized) {
// looks like an event
if (typeof key === 'string' && (key.startsWith('onDid') || key.startsWith('onWill'))) {
Comment on lines +277 to +278
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a tslint rule making sure we don't call non-event properties something which might trigger this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have such a rule for vscode.dts-files but for core it is much harder. In neither case (core and API) we have type information but for the API there is only a single type called Event so we make a very qualified guess. Also, in the API we always have type annotations which we don't always have in core. Last, not all events in core are properly named. They should be but I don't want to block this effort on that. For now it is opt-in: you want a lean and fast service, play by the book of conventions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think it's going to come up in some standup in the future after a 2 hour "why is my service not running" investigation. 😄

But yeah I do see the hassle of getting this right. And you're right, let's not block on that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the future after a 2 hour "why is my service not running" investigation

That's shouldn't happen. Worst case is today's case were listing forces service creation

let list = earlyListeners.get(key);
if (!list) {
list = new LinkedList();
earlyListeners.set(key, list);
}
const event: Event<any> = (callback, thisArg, disposables) => {
const rm = list!.push([callback, thisArg, disposables]);
return toDisposable(rm);
};
return event;
}
}

// value already exists
if (key in target) {
return target[key];
}

// create value
const obj = idle.value;
let prop = obj[key];
if (typeof prop !== 'function') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { Emitter, Event } from 'vs/base/common/event';
import { dispose } from 'vs/base/common/lifecycle';
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { createDecorator, IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
Expand Down Expand Up @@ -459,4 +461,67 @@ suite('Instantiation Service', () => {
assert.strictEqual(cycle, 'A -> B -> A');
}
});

test('Delayed and events', function () {
const A = createDecorator<A>('A');
interface A {
_serviceBrand: undefined;
onDidDoIt: Event<any>;
doIt(): void;
}

let created = false;
class AImpl implements A {
_serviceBrand: undefined;
_doIt = 0;

_onDidDoIt = new Emitter<this>();
onDidDoIt: Event<this> = this._onDidDoIt.event;

constructor() {
created = true;
}

doIt(): void {
this._doIt += 1;
this._onDidDoIt.fire(this);
}
}

const insta = new InstantiationService(new ServiceCollection(
[A, new SyncDescriptor(AImpl, undefined, true)],
), true, undefined, true);

class Consumer {
constructor(@A readonly a: A) {
// eager subscribe -> NO service instance
}
}

const c: Consumer = insta.createInstance(Consumer);
let eventCount = 0;

// subscribing to event doesn't trigger instantiation
const listener = (e: any) => {
assert.ok(e instanceof AImpl);
eventCount++;
};
const d1 = c.a.onDidDoIt(listener);
const d2 = c.a.onDidDoIt(listener);
assert.strictEqual(created, false);
assert.strictEqual(eventCount, 0);
d2.dispose();

// instantiation happens on first call
c.a.doIt();
assert.strictEqual(created, true);
assert.strictEqual(eventCount, 1);


const d3 = c.a.onDidDoIt(listener);
c.a.doIt();
assert.strictEqual(eventCount, 3);

dispose([d1, d3]);
});
});