Skip to content

Commit

Permalink
fix: update service bindings to avoid caching (#1644)
Browse files Browse the repository at this point in the history
  • Loading branch information
notaphplover authored Nov 21, 2024
1 parent 64aa399 commit 6b4cc69
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed
- Fixed container to properly resolve async `.toService` bindings.
- Fixed `.toService` binding to properly disable caching any values.

## [6.1.4]

Expand Down
18 changes: 16 additions & 2 deletions src/syntax/binding_to_syntax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,28 @@ class BindingToSyntax<T> implements interfaces.BindingToSyntax<T> {
}

public toService(service: interfaces.ServiceIdentifier<T>): void {
this.toDynamicValue((context: interfaces.Context): T | Promise<T> => {
this._binding.type = BindingTypeEnum.DynamicValue;

// Service bindings should never ever be cached. This is just a workaround to achieve that. A better design should replace this approach.
Object.defineProperty(this._binding, 'cache', {
configurable: true,
enumerable: true,
get() {
return null;
},
set(_value: unknown) {},
});
this._binding.dynamicValue = (
context: interfaces.Context,
): T | Promise<T> => {
try {
return context.container.get<T>(service);
} catch (_error: unknown) {
// This is a performance degradation in this edge case, we do need to improve the internal resolution architecture in order to solve this properly.
return context.container.getAsync<T>(service);
}
});
};
this._binding.implementationType = null;
}
}

Expand Down
42 changes: 42 additions & 0 deletions test/bugs/issue_1416.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { describe, it } from 'mocha';
import sinon from 'sinon';

import { Container, injectable, preDestroy } from '../../src/inversify';

describe('Issue 1416', () => {
it('should allow providing default values on optional bindings', async () => {
@injectable()
class Test1 {
public stub: sinon.SinonStub<unknown[], void> = sinon.stub();

@preDestroy()
public destroy() {
this.stub();
}
}

@injectable()
class Test2 {
public destroy(): void {}
}

@injectable()
class Test3 {
public destroy(): void {}
}

const container: Container = new Container({ defaultScope: 'Singleton' });

container.bind(Test1).toSelf();
container.bind(Test2).toService(Test1);
container.bind(Test3).toService(Test1);

const test1: Test1 = container.get(Test1);
container.get(Test2);
container.get(Test3);

container.unbindAll();

sinon.assert.calledOnce(test1.stub);
});
});
2 changes: 1 addition & 1 deletion wiki/tagged_bindings.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Tagged bindings

We can use tagged bindings to fix `AMBIGUOUS_MATCH` errors when two or more
concretions have been bound to an abstraction. Notice how the constructor
concretions have been bound to an abstraction. Notice how the constructor
arguments of the `Ninja` class have been annotated using the `@tagged` decorator:

```ts
Expand Down

0 comments on commit 6b4cc69

Please sign in to comment.