From bd50fb560dfacb5d3e60855d2fae1db8c6829bc2 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 5 Sep 2024 13:00:30 +1000 Subject: [PATCH 1/3] Correct types for overriden services --- src/Container.ts | 10 ++++++---- src/__tests__/Container.spec.ts | 23 +++++++++++++++++++++++ src/types.ts | 25 ++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/Container.ts b/src/Container.ts index 0694c34..a75811b 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -1,7 +1,7 @@ import { isMemoized, memoize } from "./memoize"; import type { Memoized } from "./memoize"; import { PartialContainer } from "./PartialContainer"; -import type { AddService, InjectableClass, InjectableFunction, TokenType, ValidTokens } from "./types"; +import type { AddService, AddServices, InjectableClass, InjectableFunction, TokenType, ValidTokens } from "./types"; import { ConcatInjectable } from "./Injectable"; import { entries } from "./entries"; @@ -344,7 +344,7 @@ export class Container { // `this` type, we ensure this Container can provide all the Dependencies required by the PartialContainer. this: Container, container: PartialContainer - ): Container; + ): Container>; /** * Merges services from another `Container` into this container, creating a new `Container` instance. @@ -362,7 +362,9 @@ export class Container { * @returns A new `Container` instance that combines services from this container with those from the * provided container, with services from the provided container taking precedence in case of conflicts. */ - provides(container: Container): Container; + provides( + container: Container + ): Container>; /** * Registers a new service in this Container using an `InjectableFunction`. This function defines how the service @@ -400,7 +402,7 @@ export class Container { return new Container({ ...this.factories, ...factories, - } as unknown as MaybeMemoizedFactories); + } as unknown as MaybeMemoizedFactories>); } return this.providesService(fnOrContainer); } diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index 825fbca..f583096 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -305,6 +305,29 @@ describe("Container", () => { let childContainerWithOverride = parentContainer.providesValue("value", 2); expect(childContainerWithOverride.get("service")).toBe(1); }); + + test("overriding with a different type changes resulting container's type", () => { + const parentContainer = Container.providesValue("value", 1); + let childContainerWithOverride = parentContainer.providesValue("value", "two"); + + // @ts-expect-error should be failing to compile as the type of the container has changed + let numberValue: number = childContainerWithOverride.get("value"); + + let value: string = childContainerWithOverride.get("value"); + expect(value).toBe("two"); + + const partialContainer = new PartialContainer({ + value: Injectable("value", () => "three"), + }); + childContainerWithOverride = parentContainer.provides(partialContainer); + value = childContainerWithOverride.get("value"); + expect(value).toBe("three"); + + let extraContainer = Container.fromObject({ value: "four" }); + childContainerWithOverride = parentContainer.provides(extraContainer); + value = childContainerWithOverride.get("value"); + expect(value).toBe("four"); + }); }); describe("when making a copy of the Container", () => { diff --git a/src/types.ts b/src/types.ts index e590849..b9dc936 100644 --- a/src/types.ts +++ b/src/types.ts @@ -71,7 +71,30 @@ export type ServicesFromInjectables = ParentServices extends any ? // A mapped type produces better, more concise type hints than an intersection type. - { [K in keyof ParentServices | Token]: K extends keyof ParentServices ? ParentServices[K] : Service } + { + [K in keyof ParentServices | Token]: K extends keyof ParentServices + ? K extends Token + ? Service + : ParentServices[K] + : Service; + } + : never; + +/** + * Same as AddService above, but is merging multiple services at once. Services types override those of the parent. + */ +// Using a conditional type forces TS language services to evaluate the type -- so when showing e.g. type hints, we +// will see the mapped type instead of the AddService type alias. This produces better hints. +export type AddServices = ParentServices extends any + ? Services extends any + ? { + [K in keyof Services | keyof ParentServices]: K extends keyof Services + ? Services[K] + : K extends keyof ParentServices + ? ParentServices[K] + : never; + } + : never : never; /** From 35c6405ebe9b96d6a10872ac92c5b856a1ac2db5 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Fri, 6 Sep 2024 09:01:14 +1000 Subject: [PATCH 2/3] PR feedback --- src/__tests__/Container.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index f583096..10523f1 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -316,9 +316,7 @@ describe("Container", () => { let value: string = childContainerWithOverride.get("value"); expect(value).toBe("two"); - const partialContainer = new PartialContainer({ - value: Injectable("value", () => "three"), - }); + const partialContainer = new PartialContainer({}).provides(Injectable("value", () => "three")); childContainerWithOverride = parentContainer.provides(partialContainer); value = childContainerWithOverride.get("value"); expect(value).toBe("three"); From 4cc24c37e74d3c30ccdfa005324cd508345b9dde Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Fri, 6 Sep 2024 10:31:03 +1000 Subject: [PATCH 3/3] Bump package version. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e56c928..fb8f52f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@snap/ts-inject", - "version": "0.1.0", + "version": "0.1.1", "description": "100% typesafe dependency injection framework for TypeScript projects", "license": "MIT", "author": "Snap Inc.",