Skip to content

Conversation

@wh4everest
Copy link
Contributor

@wh4everest wh4everest commented Jun 22, 2019

Introduces the concept of "providing singletons".

Modules will be expected to provide stuff to other modules, and doing app#getSingleton will throw if stuff wasn't previously provided.

Still WIP.

@wh4everest wh4everest requested a review from spion June 22, 2019 11:38
if (this.isSingletonProvided(Klass)) {
throw new Error(`The singleton ${Klass.name} is already provided.`);
}
this.providedSingletons.set(Klass, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we refactor so the same map is used for provides and overrides?

Copy link
Contributor

Choose a reason for hiding this comment

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

on second thought maybe not, you might want to override before providing.

if (this.hasSingleton(Klass)) {
return this.getExistingSingleton(Klass);
}
if (!this.isSingletonProvided(Klass)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe at locator level.

Copy link
Contributor

@spion spion left a comment

Choose a reason for hiding this comment

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

see comments

load<T>(Klass: ConstructorOrFactory<App, T>): void {
this.getSingleton(Klass); // force initialization;
return;
if (this.isSingletonProvided(Klass)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case it should be just getSingleton (multiple loads should be ok)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to re-do this with a bit of thinking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redo this PR or the loading-once-or-twice mechanics? Fine with both really.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to think whether we want to allow loading plugins within plugins, because thats what implies multiple load calls are OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants