From 003e6c7dd75ed18f20e4ead0b5688f45f5f4ffd0 Mon Sep 17 00:00:00 2001 From: exascape Date: Mon, 19 Jun 2023 23:18:19 +0100 Subject: [PATCH 1/9] refactor(ContainerInstance): make ServiceIdentifierLocation const --- src/container-instance.class.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index 9223adef..1297a2a7 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -15,8 +15,7 @@ import { isInjectedFactory } from './utils/is-inject-identifier.util'; import { resolveToTypeWrapper } from './utils/resolve-to-type-wrapper.util'; import { Disposable } from './types/disposable.type'; -// todo: make const -export enum ServiceIdentifierLocation { +export const enum ServiceIdentifierLocation { Local = 'local', Parent = 'parent', None = 'none' From 5ca10c4067600daf8751632d032d6d0e5329e8e9 Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 02:54:35 +0100 Subject: [PATCH 2/9] test(*): Make all tests compliant with the new API Due to an error in the typing for ServiceOptions, some tests were still compiling despite providing incorrect options to the Service decorator. This fixes that issue. --- test/Container.spec.ts | 41 +++++++++++++----------- test/decorators/Service.spec.ts | 34 +++++++++++++++----- test/eager-loading-services.spec.ts | 8 ++--- test/github-issues/151/issue-151.spec.ts | 6 ++-- test/github-issues/157/issue-157.spec.ts | 2 +- test/github-issues/41/issue-41.spec.ts | 8 ++--- test/github-issues/48/issue-48.spec.ts | 2 +- 7 files changed, 61 insertions(+), 40 deletions(-) diff --git a/test/Container.spec.ts b/test/Container.spec.ts index 60824d9d..beb8e144 100644 --- a/test/Container.spec.ts +++ b/test/Container.spec.ts @@ -11,8 +11,8 @@ describe('Container', function () { it('should be able to get a boolean', () => { const booleanTrue = 'boolean.true'; const booleanFalse = 'boolean.false'; - Container.set({ id: booleanTrue, value: true }); - Container.set({ id: booleanFalse, value: false }); + Container.set({ id: booleanTrue, value: true, dependencies: [] }); + Container.set({ id: booleanFalse, value: false, dependencies: [] }); expect(Container.get(booleanTrue)).toBe(true); expect(Container.get(booleanFalse)).toBe(false); @@ -20,14 +20,14 @@ describe('Container', function () { it('should be able to get an empty string', () => { const emptyString = 'emptyString'; - Container.set({ id: emptyString, value: '' }); + Container.set({ id: emptyString, value: '', dependencies: [] }); expect(Container.get(emptyString)).toBe(''); }); it('should be able to get the 0 number', () => { const zero = 'zero'; - Container.set({ id: zero, value: 0 }); + Container.set({ id: zero, value: 0, dependencies: [] }); expect(Container.get(zero)).toBe(0); }); @@ -39,7 +39,7 @@ describe('Container', function () { constructor(public name: string) {} } const testService = new TestService('this is test'); - Container.set({ id: TestService, value: testService }); + Container.set({ id: TestService, value: testService, dependencies: [] }); expect(Container.get(TestService)).toBe(testService); expect(Container.get(TestService).name).toBe('this is test'); }); @@ -49,10 +49,10 @@ describe('Container', function () { constructor(public name: string) {} } const firstService = new TestService('first'); - Container.set({ id: 'first.service', value: firstService }); + Container.set({ id: 'first.service', value: firstService, dependencies: [String] }); const secondService = new TestService('second'); - Container.set({ id: 'second.service', value: secondService }); + Container.set({ id: 'second.service', value: secondService, dependencies: [String] }); expect(Container.get('first.service').name).toBe('first'); expect(Container.get('second.service').name).toBe('second'); @@ -66,10 +66,10 @@ describe('Container', function () { const SecondTestToken = new Token(); const firstService = new TestService('first'); - Container.set({ id: FirstTestToken, value: firstService }); + Container.set({ id: FirstTestToken, value: firstService, dependencies: [] }); const secondService = new TestService('second'); - Container.set({ id: SecondTestToken, value: secondService }); + Container.set({ id: SecondTestToken, value: secondService, dependencies: [] }); expect(Container.get(FirstTestToken).name).toBe('first'); expect(Container.get(SecondTestToken).name).toBe('second'); @@ -82,12 +82,12 @@ describe('Container', function () { const TestToken = new Token(); const firstService = new TestService('first'); - Container.set({ id: TestToken, value: firstService }); + Container.set({ id: TestToken, value: firstService, dependencies: [String] }); expect(Container.get(TestToken)).toBe(firstService); expect(Container.get(TestToken).name).toBe('first'); const secondService = new TestService('second'); - Container.set({ id: TestToken, value: secondService }); + Container.set({ id: TestToken, value: secondService, dependencies: [String] }); expect(Container.get(TestToken)).toBe(secondService); expect(Container.get(TestToken).name).toBe('second'); @@ -108,10 +108,10 @@ describe('Container', function () { const test1Service = new TestService(); const test2Service = new TestService(); - Container.set({ id: TestService, value: testService }); - Container.set({ id: 'test1-service', value: test1Service }); - Container.set({ id: 'test2-service', value: test2Service }); - Container.set({ id: 'test3-service', factory: [TestServiceFactory, 'create'] }); + Container.set({ id: TestService, value: testService, dependencies: [] }); + Container.set({ id: 'test1-service', value: test1Service, dependencies: [] }); + Container.set({ id: 'test2-service', value: test2Service, dependencies: [] }); + Container.set({ id: 'test3-service', factory: [TestServiceFactory, 'create'], dependencies: [] }); expect(Container.get(TestService)).toBe(testService); expect(Container.get('test1-service')).toBe(test1Service); @@ -131,9 +131,9 @@ describe('Container', function () { const test1Service = new TestService(); const test2Service = new TestService(); - Container.set({ id: TestService, value: testService }); - Container.set({ id: 'test1-service', value: test1Service }); - Container.set({ id: 'test2-service', value: test2Service }); + Container.set({ id: TestService, value: testService, dependencies: [] }); + Container.set({ id: 'test1-service', value: test1Service, dependencies: [] }); + Container.set({ id: 'test2-service', value: test2Service, dependencies: [] }); expect(Container.get(TestService)).toBe(testService); expect(Container.get('test1-service')).toBe(test1Service); @@ -154,7 +154,7 @@ describe('Container', function () { constructor(public name: string = 'frank') {} } - Container.set({ id: TestService, type: TestService }); + Container.set({ id: TestService, type: TestService, dependencies: [] }); const testService = Container.get(TestService); testService.name = 'john'; @@ -179,6 +179,7 @@ describe('Container', function () { Container.set({ id: Car, factory: () => new Car(new Engine()), + dependencies: [Engine] }); expect(Container.get(Car).engine.serialNumber).toBe('A-123'); @@ -206,6 +207,7 @@ describe('Container', function () { Container.set({ id: Car, factory: [CarFactory, 'createCar'], + dependencies: [Engine] }); expect(Container.get(Car).engine.serialNumber).toBe('A-123'); @@ -233,6 +235,7 @@ describe('Container', function () { Container.set({ id: VehicleService, factory: [VehicleFactory, 'createBus'], + dependencies: [] }); expect(Container.get(VehicleService).getColor()).toBe('yellow'); diff --git a/test/decorators/Service.spec.ts b/test/decorators/Service.spec.ts index 3ccd4664..f23a250f 100644 --- a/test/decorators/Service.spec.ts +++ b/test/decorators/Service.spec.ts @@ -9,7 +9,7 @@ describe('Service Decorator', function () { it('should register class in the container, and its instance should be retrievable', function () { @Service([]) class TestService {} - @Service({ id: 'super.service' }) + @Service({ id: 'super.service' }, []) class NamedService {} expect(Container.get(TestService)).toBeInstanceOf(TestService); expect(Container.get(TestService)).not.toBeInstanceOf(NamedService); @@ -18,7 +18,7 @@ describe('Service Decorator', function () { it('should register class in the container with given name, and its instance should be retrievable', function () { @Service([]) class TestService {} - @Service({ id: 'super.service' }) + @Service({ id: 'super.service' }, []) class NamedService {} expect(Container.get('super.service')).toBeInstanceOf(NamedService); expect(Container.get('super.service')).not.toBeInstanceOf(TestService); @@ -72,7 +72,7 @@ describe('Service Decorator', function () { } } - @Service({ factory: [CarFactory, 'createCar'] }) + @Service({ factory: [CarFactory, 'createCar'] }, [String, Engine]) class Car { name: string; constructor(name: string, public engine: Engine) { @@ -90,7 +90,7 @@ describe('Service Decorator', function () { public type = 'V8'; } - @Service([]) + @Service([Engine]) class CarFactory { createCar(engine: Engine) { engine.type = 'V6'; @@ -98,7 +98,7 @@ describe('Service Decorator', function () { } } - @Service({ factory: [CarFactory, 'createCar'] }) + @Service({ factory: [CarFactory, 'createCar'] }, [Engine]) class Car { constructor(public engine: Engine) {} } @@ -112,7 +112,7 @@ describe('Service Decorator', function () { public serial = Math.random(); } - @Service({ scope: 'transient' }) + @Service({ scope: 'transient' }, []) class Engine { public serial = Math.random(); } @@ -139,7 +139,7 @@ describe('Service Decorator', function () { public name = 'sporty'; } - @Service({ scope: 'singleton' }) + @Service({ scope: 'singleton' }, []) class Car { public name = 'SportCar'; } @@ -166,12 +166,13 @@ describe('Service Decorator', function () { it('should support function injection with Token dependencies', function () { const myToken: Token = new Token('myToken'); - Container.set({ id: myToken, value: 'test_string' }); + Container.set({ id: myToken, value: 'test_string', dependencies: [] }); Container.set({ id: 'my-service-A', factory: function myServiceFactory(container): string { return container.get(myToken).toUpperCase(); }, + dependencies: [] }); /** @@ -180,6 +181,7 @@ describe('Service Decorator', function () { */ Service({ id: 'my-service-B', + dependencies: [], factory: function myServiceFactory(container): string { return container.get(myToken).toUpperCase(); }, @@ -188,4 +190,20 @@ describe('Service Decorator', function () { expect(Container.get('my-service-A')).toBe('TEST_STRING'); expect(Container.get('my-service-B')).toBe('TEST_STRING'); }); + + it('should support an object with dependencies with no other arguments', () => { + @Service([]) + class AnotherService { + getMeaningOfLife () { + return 42; + } + } + + @Service({ dependencies: [AnotherService] }) + class MyService { + constructor (public anotherService: AnotherService) {} + } + + expect(Container.get(MyService).anotherService.getMeaningOfLife()).toStrictEqual(42); + }); }); diff --git a/test/eager-loading-services.spec.ts b/test/eager-loading-services.spec.ts index aff062f6..d9d24dc0 100644 --- a/test/eager-loading-services.spec.ts +++ b/test/eager-loading-services.spec.ts @@ -13,8 +13,8 @@ describe('Eager loading of services', function () { public createdAt = callOrder++; } - Container.set({ id: 'eager-service', type: MyService, eager: true }); - Container.set({ id: 'lazy-service', type: MyService, eager: false }); + Container.set({ id: 'eager-service', type: MyService, eager: true, dependencies: [] }); + Container.set({ id: 'lazy-service', type: MyService, eager: false, dependencies: [] }); const timeStampBeforeRequests = callOrder++; @@ -37,12 +37,12 @@ describe('Eager loading of services', function () { it('should be able to set eager and lazy service with @Service decorator', () => { let callOrder = 1; - @Service({ eager: true }) + @Service({ eager: true }, []) class MyEagerService { public createdAt = callOrder++; } - @Service({ eager: false }) + @Service({ eager: false }, []) class MyLazyService { public createdAt = callOrder++; } diff --git a/test/github-issues/151/issue-151.spec.ts b/test/github-issues/151/issue-151.spec.ts index 92af044a..5b698e67 100644 --- a/test/github-issues/151/issue-151.spec.ts +++ b/test/github-issues/151/issue-151.spec.ts @@ -1,6 +1,5 @@ import 'reflect-metadata'; -import { Container } from '../../index'; -import { Service } from '../../decorators/service.decorator'; +import { Container, Service } from '../../../src/index'; describe('Github Issues', function () { beforeEach(() => Container.reset({ strategy: 'resetValue' })); @@ -21,12 +20,13 @@ describe('Github Issues', function () { class DataService { constructor(public authService: AuthService) {} } + @Service([AuthService]) class FakeDataService { constructor(public authService: AuthService) {} } - Container.set({ id: DataService, type: FakeDataService }); + Container.set({ id: DataService, type: FakeDataService, dependencies: [AuthService] }); const instance = Container.get(DataService as any); diff --git a/test/github-issues/157/issue-157.spec.ts b/test/github-issues/157/issue-157.spec.ts index 8f22fd85..f18fc40b 100644 --- a/test/github-issues/157/issue-157.spec.ts +++ b/test/github-issues/157/issue-157.spec.ts @@ -8,7 +8,7 @@ describe('Github Issues', function () { it('#157 - reset should not break transient services', () => { let creationCounter = 0; - @Service({ scope: 'transient' }) + @Service({ scope: 'transient' }, []) class TransientService { public constructor() { creationCounter++; diff --git a/test/github-issues/41/issue-41.spec.ts b/test/github-issues/41/issue-41.spec.ts index 866e530c..31ca2cf6 100644 --- a/test/github-issues/41/issue-41.spec.ts +++ b/test/github-issues/41/issue-41.spec.ts @@ -22,16 +22,16 @@ describe('github issues > #41 Token as service id in combination with factory', @Service({ id: SomeInterfaceToken, factory: [SomeInterfaceFactory, 'create'], - }) + }, []) class SomeImplementation implements SomeInterface { foo() { return 'hello implementation'; } } - Container.set({ id: 'moment', value: 'A' }); - Container.set({ id: 'jsonwebtoken', value: 'B' }); - Container.set({ id: 'cfg.auth.jwt', value: 'C' }); + Container.set({ id: 'moment', value: 'A', dependencies: [] }); + Container.set({ id: 'jsonwebtoken', value: 'B', dependencies: [] }); + Container.set({ id: 'cfg.auth.jwt', value: 'C', dependencies: [] }); const someInterfaceImpl = Container.get(SomeInterfaceToken); expect(someInterfaceImpl.foo()).toBe('hello implementation'); }); diff --git a/test/github-issues/48/issue-48.spec.ts b/test/github-issues/48/issue-48.spec.ts index aedff092..c9700a49 100644 --- a/test/github-issues/48/issue-48.spec.ts +++ b/test/github-issues/48/issue-48.spec.ts @@ -11,7 +11,7 @@ describe("github issues > #48 Token service iDs in global container aren't inher const FooServiceToken = new Token(); - @Service({ id: FooServiceToken }) + @Service({ id: FooServiceToken }, []) class FooService implements FooService { public marco() { poloCounter++; From 4baa53fff90842b4e20367cfd4ca286cbca722cb Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 02:57:20 +0100 Subject: [PATCH 3/9] feat(*): Prevent built-in dependencies with no factory BREAKING CHANGE: Prevent services from depending on built-in types such as Number, Symbol, Object without providing a factory. This prevents the injector from calling new on a built-in, leading to confusing errors and values being passed to newly-created services. --- src/builtins.const.ts | 1 + src/container-instance.class.ts | 24 ++++++++++++------- src/error/cannot-instantiate-builtin-error.ts | 10 ++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 src/builtins.const.ts create mode 100644 src/error/cannot-instantiate-builtin-error.ts diff --git a/src/builtins.const.ts b/src/builtins.const.ts new file mode 100644 index 00000000..3642e541 --- /dev/null +++ b/src/builtins.const.ts @@ -0,0 +1 @@ +export const BUILT_INS = [String, Object, Symbol, Array, Number]; diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index 1297a2a7..b42b5e3f 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -14,6 +14,9 @@ import { InjectedFactory } from './types/inject-identifier.type'; import { isInjectedFactory } from './utils/is-inject-identifier.util'; import { resolveToTypeWrapper } from './utils/resolve-to-type-wrapper.util'; import { Disposable } from './types/disposable.type'; +import { Service } from './decorators/service.decorator'; +import { BUILT_INS } from './builtins.const'; +import { CannotInstantiateBuiltInError } from './error/cannot-instantiate-builtin-error'; export const enum ServiceIdentifierLocation { Local = 'local', @@ -284,7 +287,8 @@ export class ContainerInstance implements Disposable { * Add a service to the container using the provided options, containing * all information about the new service including its dependencies. */ - public set(serviceOptions: ServiceOptions): ServiceIdentifier; + public set(serviceOptions: ServiceOptions & { dependencies: AnyInjectIdentifier[] }): ServiceIdentifier; + public set(serviceOptions: ServiceOptions | Omit, 'dependencies'>, precompiledDependencies?: TypeWrapper[]): ServiceIdentifier { this.throwIfDisposed(); @@ -312,14 +316,14 @@ export class ContainerInstance implements Disposable { * or from the dependencies list of the service options object, which must then be compiled to a list of type wrappers. */ const dependencies: TypeWrapper[] = - precompiledDependencies ?? (serviceOptions as ServiceOptions)?.dependencies?.map(resolveToTypeWrapper) ?? []; + precompiledDependencies ?? (serviceOptions as ServiceOptions & { dependencies: AnyInjectIdentifier[] }).dependencies.map(resolveToTypeWrapper) ?? []; const newMetadata: ServiceMetadata = { /** * Typescript cannot understand that if ID doesn't exists then type must exists based on the * typing so we need to explicitly cast this to a `ServiceIdentifier` */ - id: ((serviceOptions as any).id || (serviceOptions as any).type) as ServiceIdentifier, + id: ((serviceOptions as any).id ?? (serviceOptions as any).type) as ServiceIdentifier, type: null, factory: undefined, value: EMPTY_VALUE, @@ -531,7 +535,7 @@ export class ContainerInstance implements Disposable { */ if (!factoryMeta && serviceMetadata.type) { const constructableTargetType: Constructable = serviceMetadata.type; - const parameters = this.getConstructorParameters(serviceMetadata); + const parameters = this.getConstructorParameters(serviceMetadata, true); value = new constructableTargetType(...parameters); } @@ -548,7 +552,7 @@ export class ContainerInstance implements Disposable { return value; } - private getConstructorParameters ({ dependencies }: ServiceMetadata): unknown[] { + private getConstructorParameters ({ dependencies }: ServiceMetadata, guardBuiltIns?: boolean): unknown[] { /** * Firstly, check if the metadata declares any dependencies. */ @@ -562,7 +566,7 @@ export class ContainerInstance implements Disposable { * Therefore, it can be safely assumed that if the key is present, the correct * data will also be present. */ - return dependencies.map(wrapper => this.resolveTypeWrapper(wrapper)); + return dependencies.map(wrapper => this.resolveTypeWrapper(wrapper, guardBuiltIns)); } private runInjectedFactory (factory: InjectedFactory): unknown { @@ -575,7 +579,7 @@ export class ContainerInstance implements Disposable { return returnedValue; } - private resolveTypeWrapper (wrapper: TypeWrapper) { + private resolveTypeWrapper (wrapper: TypeWrapper, guardBuiltIns = false) { if (wrapper.isFactory) { return this.runInjectedFactory(wrapper.factory); } @@ -590,7 +594,11 @@ export class ContainerInstance implements Disposable { const resolved = wrapper.eagerType ?? (wrapper as GenericTypeWrapper).lazyType?.(); if (resolved == null) { - throw new Error('The type could not be resolved.'); + throw new Error(`A value of type "${resolved}" could not be resolved.`); + } + + if (guardBuiltIns && BUILT_INS.includes(resolved as any)) { + throw new CannotInstantiateBuiltInError((resolved as any)?.name ?? resolved); } /** diff --git a/src/error/cannot-instantiate-builtin-error.ts b/src/error/cannot-instantiate-builtin-error.ts new file mode 100644 index 00000000..f45333de --- /dev/null +++ b/src/error/cannot-instantiate-builtin-error.ts @@ -0,0 +1,10 @@ +import { CannotInstantiateValueError } from "./cannot-instantiate-value.error"; + +/** + * Thrown when DI encounters a service depending on a built-in type (Number, String) with no factory. + */ +export class CannotInstantiateBuiltInError extends CannotInstantiateValueError { + get message () { + return super.message + ` If your service requires built-in or unresolvable types, please use a factory.`; + } +} \ No newline at end of file From 67f286bb2488fab9f554c09a7fe0c3bc44fde1ad Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 02:58:35 +0100 Subject: [PATCH 4/9] docs(ContainerInstance): improve TSDoc Improve the documentation of methods and properties on ContainerInstance. This aids VSCode's TSDoc support, as consumers of the API can now see examples and documentation of each operation without resorting to online docs. --- src/container-instance.class.ts | 114 ++++++++++++++++++++++++++++++-- 1 file changed, 108 insertions(+), 6 deletions(-) diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index b42b5e3f..33dc9a9e 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -34,7 +34,7 @@ interface ManyServicesMetadata { * One container is ContainerInstance. */ export class ContainerInstance implements Disposable { - /** Container instance id. */ + /** The ID of this container. This will always be unique. */ public readonly id!: ContainerIdentifier; /** Metadata for all registered services in this container. */ @@ -51,8 +51,6 @@ export class ContainerInstance implements Disposable { /** * Indicates if the container has been disposed or not. * Any function call should fail when called after being disposed. - * - * NOTE: Currently not in use */ public disposed: boolean = false; @@ -62,6 +60,14 @@ export class ContainerInstance implements Disposable { */ public static defaultContainer = new ContainerInstance("default"); + /** + * Create a ContainerInstance. + * + * @param id The ID of the container to create. + * @param parent The parent of the container to create. + * The parent is used for resolving identifiers which are + * not present in this container. + */ protected constructor (id: ContainerIdentifier, public parent?: ContainerInstance) { this.id = id; } @@ -73,6 +79,10 @@ export class ContainerInstance implements Disposable { * If recursive mode is enabled, the symbol is not available locally, and we have a parent, * we tell the parent to search its tree recursively too. * If the container tree is substantial, this operation may affect performance. + * + * @param identifier The identifier of the service to look up. + * + * @returns Whether the identifier is present in the current container, or its parent. */ public has(identifier: ServiceIdentifier, recursive = true): boolean { this.throwIfDisposed(); @@ -86,6 +96,13 @@ export class ContainerInstance implements Disposable { * If recursive mode is enabled, the symbol is not available locally, and we have a parent, * we tell the parent to search its tree recursively too. * If the container tree is substantial, this operation may affect performance. + * + * @param identifier The identifier of the service to look up. + * + * @returns A ServiceIdentifierLocation. + * - If the identifier cannot be found, `ServiceIdentifierLocation.None`. + * - If the identifier is found locally, `ServiceIdentifierLocation.Local`. + * - If the identifier is found upstream, `ServiceIdentifierLocation.Parent.` */ protected getIdentifierLocation(identifier: ServiceIdentifier): ServiceIdentifierLocation { this.throwIfDisposed(); @@ -103,6 +120,19 @@ export class ContainerInstance implements Disposable { return ServiceIdentifierLocation.None; } + /** + * Get the value for the identifier in the current container. + * If the identifier cannot be resolved locally, the parent tree + * (if present) is recursively searched until a match is found + * (or the tree is exhausted). + * + * @param identifier The identifier to get the value of. + * + * @returns The value of the identifier in the current scope. + * + * @throws ServiceNotFoundError + * This exception is thrown if the identifier cannot be found in the tree. + */ public get(identifier: ServiceIdentifier, recursive?: boolean): T { const response = this.getOrNull(identifier, recursive); @@ -113,7 +143,20 @@ export class ContainerInstance implements Disposable { return response as T; } - /** Resolve the metadata for the given identifier. Returns null if no metadata could be found. */ + /** + * Resolve the metadata for the given identifier. Returns null if no metadata could be found. + * + * @param identifier The identifier to resolve metadata for. + * @param recursive Whether the lookup operation is recursive. + * + * @returns + * If the identifier is found, a tuple is returned consisting of the following: + * 1. The metadata for the given identifier, if found. + * 2. The location from where the metadata was returned. + * `ServiceIdentifierLocation.Parent` is returned if the identifier was found upstream. + * + * If an identifier cannot be found, `null` is returned. + */ protected resolveMetadata (identifier: ServiceIdentifier, recursive: boolean): readonly [ServiceMetadata, ServiceIdentifierLocation] | null { this.throwIfDisposed(); @@ -150,6 +193,10 @@ export class ContainerInstance implements Disposable { * Optionally, parameters can be passed in case if instance is initialized in the container for the first time. * * To preserve compatibility with TypeDI, recursive is set to false by default. + * + * @param identifier The identifier to get the value of. + * + * @returns The resolved value for the given metadata, or `null` if it could not be found. */ public getOrNull(identifier: ServiceIdentifier, recursive = false): T | null { this.throwIfDisposed(); @@ -229,7 +276,14 @@ export class ContainerInstance implements Disposable { /** * Gets all instances registered in the container of the given service identifier. * Used when service defined with multiple: true flag. - * If none can be found, an error is thrown. + * + * @param identifier The identifier to resolve. + * + * @returns An array containing the service instances for the given + * identifier. + * + * @throws ServiceNotFoundError + * This exception is thrown if a value could not be found for the given identifier. */ public getMany(identifier: ServiceIdentifier, recursive?: boolean): T[] { const response = this.getManyOrNull(identifier, recursive); @@ -244,6 +298,10 @@ export class ContainerInstance implements Disposable { /** * Gets all instances registered in the container of the given service identifier. * Used when service defined with multiple: true flag. + * + * @param identifier The identifier to resolve. + * + * @returns An array containing the service instances for the given identifier. * If none can be found, `null` is returned. */ public getManyOrNull(identifier: ServiceIdentifier, recursive = true): T[] | null { @@ -280,12 +338,25 @@ export class ContainerInstance implements Disposable { /** * Add a service to the container using the provided options, along with * a pre-wrapped list of dependencies. + * + * _This is mainly for internal use._ + * + * @param serviceOptions The options for the service to add to the container. + * @param precompiledDependencies A precompiled list of dependencies in `TypeWrapper` form for the given service. + * + * @returns The identifier of the given service in the container. + * This can then be passed to `.get` to resolve the identifier. */ public set(serviceOptions: Omit, 'dependencies'>, precompiledDependencies: TypeWrapper[]): ServiceIdentifier; /** * Add a service to the container using the provided options, containing * all information about the new service including its dependencies. + * + * @param serviceOptions The options for the service to add to the container. + * + * @returns The identifier of the given service in the container. + * This can then be passed to `.get` to resolve the identifier. */ public set(serviceOptions: ServiceOptions & { dependencies: AnyInjectIdentifier[] }): ServiceIdentifier; @@ -383,7 +454,11 @@ export class ContainerInstance implements Disposable { } /** - * Removes services with a given service identifiers. + * Removes services with the given list of service identifiers. + * + * @param identifierOrIdentifierArray The list of service identifiers to remove from the container. + * + * @returns The current `ContainerInstance` instance. */ public remove(identifierOrIdentifierArray: ServiceIdentifier | ServiceIdentifier[]): this { this.throwIfDisposed(); @@ -404,6 +479,12 @@ export class ContainerInstance implements Disposable { /** * Gets a separate container instance for the given instance id. * Optionally, a parent can be passed, which will act as an upstream resolver for the container. + * + * @param containerId The ID of the container to resolve or create. Defaults to "default". + * @param parent The parent of the container. + * + * @returns The newly-created ContainerInstance, or the pre-existing container with the same name + * if one already exists. */ public static of(containerId: ContainerIdentifier = 'default', parent = ContainerInstance.defaultContainer): ContainerInstance { if (containerId === 'default') { @@ -430,6 +511,22 @@ export class ContainerInstance implements Disposable { return container; } + /** + * Gets a separate container instance for the given instance id. + * + * @param containerId The ID of the container to resolve or create. Defaults to "default". + * + * @example + * ``` + * const newContainer = Container.of('foo'); + * + * @Service({ container: newContainer }, []) + * class MyService {} + * ``` + * + * @returns The newly-created ContainerInstance, or the pre-existing container with the same name + * if one already exists. + */ public of(containerId?: ContainerIdentifier): ContainerInstance { this.throwIfDisposed(); @@ -440,6 +537,11 @@ export class ContainerInstance implements Disposable { /** * Create a registry with the specified ID, with this instance as its parent. + * + * @param containerId The ID of the container to resolve or create. Defaults to "default". + * + * @returns The newly-created ContainerInstance, or the pre-existing container with the same name + * if one already exists. */ public ofChild (containerId?: ContainerIdentifier) { return ContainerInstance.of(containerId, this); From 6871079d47fcc7d37e606610725a49435143a09a Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 03:00:03 +0100 Subject: [PATCH 5/9] feat(ContainerInstance): force override dependencies BREAKING CHANGE: Forcibly override the dependencies property of newly-injected services in the container. This prevents the type-wrapped dependencies being useless. Also fixes test for #151. --- src/container-instance.class.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index 33dc9a9e..d1142c79 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -402,12 +402,17 @@ export class ContainerInstance implements Disposable { eager: false, scope: 'container', + /** We allow overriding the above options via the received config object. */ + ...serviceOptions, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + /** + * We override the service options with the provided dependencies here, as if we have type wrapped + * dependencies only, we'll obviously want to overwrite anything in the options with those. + * Additionally, this fixes test cases such as #151. + */ /** @ts-ignore TypeScript is actually broken here. We've told it dependencies is of type TypeWrapper[], but it still doesn't like it? */ dependencies: dependencies as unknown as TypeWrapper[], - - /** We allow overriding the above options via the received config object. */ - ...serviceOptions, }; /** If the incoming metadata is marked as multiple we mask the ID and continue saving as single value. */ From 0a975aaf0a680f7e50cb3f7d2839cb9776a18b72 Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 03:01:10 +0100 Subject: [PATCH 6/9] fix(ContainerInstance): add relevant imports These imports were present in prior patches, but a short-sighted git add --patch led me to miss them. --- src/container-instance.class.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index d1142c79..aa17b771 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -10,7 +10,7 @@ import { EMPTY_VALUE } from './empty.const'; import { ContainerIdentifier } from './types/container-identifier.type'; import { ContainerScope } from './types/container-scope.type'; import { GenericTypeWrapper, TypeWrapper } from './types/type-wrapper.type'; -import { InjectedFactory } from './types/inject-identifier.type'; +import { AnyInjectIdentifier, InjectedFactory } from './types/inject-identifier.type'; import { isInjectedFactory } from './utils/is-inject-identifier.util'; import { resolveToTypeWrapper } from './utils/resolve-to-type-wrapper.util'; import { Disposable } from './types/disposable.type'; From 7233dca8fc37196f998d86f40df29ba1bbfd3b87 Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 03:01:47 +0100 Subject: [PATCH 7/9] fix(ContainerInstance): remove dead code Remove dead code from ContainerInstance.set that looks up the identifier in the map. --- src/container-instance.class.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/container-instance.class.ts b/src/container-instance.class.ts index aa17b771..996da184 100644 --- a/src/container-instance.class.ts +++ b/src/container-instance.class.ts @@ -439,12 +439,10 @@ export class ContainerInstance implements Disposable { // todo: sort this out // I've removed the legacy "extend service if it already exists" // behaviour for now. - const existingMetadata = this.metadataMap.get(newMetadata.id); + // const existingMetadata = this.metadataMap.get(newMetadata.id); - { - /** This service hasn't been registered yet, so we register it. */ - this.metadataMap.set(newMetadata.id, newMetadata); - } + /** This service hasn't been registered yet, so we register it. */ + this.metadataMap.set(newMetadata.id, newMetadata); /** * If the service is eager, we need to create an instance immediately except From 27c8ae4e4c57fd36c21a5ff4a5f32fd68bcb2405 Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 03:02:34 +0100 Subject: [PATCH 8/9] feat(JSService): add JSService decorator w/ tests Add a new JSService decorator to make using TypeDI from pure JavaScript easier. --- src/decorators/js-service.decorator.ts | 32 ++++++++++ src/types/js-service.type.ts | 3 + test/decorators/JSService.spec.ts | 83 ++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 src/decorators/js-service.decorator.ts create mode 100644 src/types/js-service.type.ts create mode 100644 test/decorators/JSService.spec.ts diff --git a/src/decorators/js-service.decorator.ts b/src/decorators/js-service.decorator.ts new file mode 100644 index 00000000..3cfc57bd --- /dev/null +++ b/src/decorators/js-service.decorator.ts @@ -0,0 +1,32 @@ +import { ServiceOptions } from "../interfaces/service-options.interface"; +import { AnyConstructable } from "../types/any-constructable.type"; +import { AnyInjectIdentifier } from "../types/inject-identifier.type"; +import { Service } from "./service.decorator"; + +export function JSService(dependencies: AnyInjectIdentifier[], constructor: T): T; +export function JSService(options: Omit, 'dependencies'>, dependencies: AnyInjectIdentifier[], constructor: T): T; +export function JSService(options: ServiceOptions, constructor: T): T; + +export function JSService( + optionsOrDependencies: Omit, 'dependencies'> | ServiceOptions | AnyInjectIdentifier[], + dependenciesOrConstructor: AnyInjectIdentifier[] | T, + maybeConstructor?: T +): T { + let constructor!: T; + + if (typeof dependenciesOrConstructor === 'function') { + constructor = dependenciesOrConstructor as T; + Service(optionsOrDependencies as ServiceOptions & { dependencies: AnyInjectIdentifier[] })(constructor); + } else if (maybeConstructor) { + constructor = maybeConstructor; + Service(optionsOrDependencies, dependenciesOrConstructor)(constructor); + } + + if (!constructor) { + throw new Error('The JSService overload was not used correctly.'); + } + + return constructor; +} + +export type JSService = T extends AnyConstructable ? U : never; diff --git a/src/types/js-service.type.ts b/src/types/js-service.type.ts new file mode 100644 index 00000000..da182656 --- /dev/null +++ b/src/types/js-service.type.ts @@ -0,0 +1,3 @@ +import { AnyConstructable } from "./any-constructable.type"; + +export type JSService = T; \ No newline at end of file diff --git a/test/decorators/JSService.spec.ts b/test/decorators/JSService.spec.ts new file mode 100644 index 00000000..97a184cc --- /dev/null +++ b/test/decorators/JSService.spec.ts @@ -0,0 +1,83 @@ +import Container from '../../src/index'; +import { JSService } from '../../src/decorators/js-service.decorator'; +import { AnyConstructable } from '../types/any-constructable.type'; + +describe('JSService decorator', () => { + beforeEach(() => Container.reset({ strategy: 'resetValue' })); + + type InstanceOf = T extends AnyConstructable ? U : never; + + it('should take dependencies and constructor', () => { + type AnotherService = InstanceOf; + const AnotherService = JSService([], class AnotherService { + getMeaningOfLife () { + return 42; + } + }); + + type MyService = InstanceOf; + const MyService = JSService([AnotherService], class MyService { + constructor (public anotherService: AnotherService) { } + }); + + expect(Container.has(MyService)).toBe(true); + expect(Container.has(AnotherService)).toBe(true); + expect(Container.get(MyService).anotherService.getMeaningOfLife()).toStrictEqual(42); + }); + + it('should take options without dependencies, list of dependencies and constructor', () => { + type AnotherService = InstanceOf; + const AnotherService = JSService([], class AnotherService { + getMeaningOfLife () { + return 42; + } + }); + + type MyService = InstanceOf; + const MyService = JSService({ }, [AnotherService], class MyService { + public anotherService: AnotherService; + + constructor (anotherService: AnotherService) { + this.anotherService = anotherService; + } + }); + + expect(Container.has(MyService)).toBe(true); + expect(Container.has(AnotherService)).toBe(true); + expect(Container.get(MyService).anotherService.getMeaningOfLife()).toStrictEqual(42); + }); + + it('should take options with dependencies and constructor', () => { + type AnotherService = InstanceOf; + const AnotherService = JSService([], class AnotherService { + getMeaningOfLife () { + return 42; + } + }); + + type MyService = InstanceOf; + const MyService = JSService({ dependencies: [AnotherService] }, class MyService { + public anotherService: AnotherService; + + constructor (anotherService: AnotherService) { + this.anotherService = anotherService; + } + }); + + expect(Container.has(MyService)).toBe(true); + expect(Container.has(AnotherService)).toBe(true); + expect(Container.get(MyService).anotherService.getMeaningOfLife()).toStrictEqual(42); + }); + + it('should provide a functioning JSService type', () => { + const MyService = JSService([], class MyService { + getOne () { + return 1; + } + }); + + type MyService = JSService; + + const myService: MyService = Container.get(MyService); + }); +}); \ No newline at end of file From 0ef9740f52a26a283dc73b6484e393f4bf27f2bb Mon Sep 17 00:00:00 2001 From: exascape Date: Tue, 20 Jun 2023 03:08:31 +0100 Subject: [PATCH 9/9] docs(JSService): add TSDoc documentation Add TSDoc documentation to the JSService function. --- src/decorators/js-service.decorator.ts | 52 +++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/decorators/js-service.decorator.ts b/src/decorators/js-service.decorator.ts index 3cfc57bd..12d73b72 100644 --- a/src/decorators/js-service.decorator.ts +++ b/src/decorators/js-service.decorator.ts @@ -3,9 +3,59 @@ import { AnyConstructable } from "../types/any-constructable.type"; import { AnyInjectIdentifier } from "../types/inject-identifier.type"; import { Service } from "./service.decorator"; +/** + * Marks class as a service that can be injected using Container. + * Uses the default options, wherein the class can be passed to `.get` and an instance of it will be returned. + * By default, the service shall be registered upon the `defaultContainer` container. + * @experimental + * + * @param dependencies The dependencies to provide upon initialisation of this service. + * These will be provided to the service as arguments to its constructor. + * They must be valid identifiers in the container the service shall be executed under. + * + * @param constructor The constructor of the service to inject. + * + * @returns The constructor. + */ export function JSService(dependencies: AnyInjectIdentifier[], constructor: T): T; + +/** + * Marks class as a service that can be injected using Container. + * The options allow customization of how the service is injected. + * By default, the service shall be registered upon the `defaultContainer` container. + * @experimental + * + * @param options The options to use for initialisation of the service. + * Documentation for the options can be found in ServiceOptions. + * + * @param dependencies The dependencies to provide upon initialisation of this service. + * These will be provided to the service as arguments to its constructor. + * They must be valid identifiers in the container the service shall be executed under. + * + * @param constructor The constructor of the service to inject. + * + * @returns The constructor. + */ export function JSService(options: Omit, 'dependencies'>, dependencies: AnyInjectIdentifier[], constructor: T): T; -export function JSService(options: ServiceOptions, constructor: T): T; + +/** + * Marks class as a service that can be injected using Container. + * The options allow customization of how the service is injected. + * By default, the service shall be registered upon the `defaultContainer` container. + * + * @param options The options to use for initialisation of the service. + * Documentation for the options can be found in ServiceOptions. + * The options must also contain the dependencies that the service requires. + * + * If found, the specified dependencies to provide upon initialisation of this service. + * These will be provided to the service as arguments to its constructor. + * They must be valid identifiers in the container the service shall be executed under. + * + * @param constructor The constructor of the service to inject. + * + * @returns The constructor. + */ +export function JSService(options: ServiceOptions & { dependencies: AnyInjectIdentifier[] }, constructor: T): T; export function JSService( optionsOrDependencies: Omit, 'dependencies'> | ServiceOptions | AnyInjectIdentifier[],