Skip to content

Commit

Permalink
Correct types for overriden services (#4)
Browse files Browse the repository at this point in the history
Fixing an issue where override's type wasn't correctly carried over onto
the resulting container, e.g.

```
const value: number =  Container.providesValue("value", 1).providesValue("value", "two").get("value");
```

would, incorrectly, compile and cause issues at runtime. With the fix
`.get("value")` return type is correctly resolved to `string`.

Also fixed `provides*` implementations that take containers as
arguments.
  • Loading branch information
kburov-sc authored Sep 6, 2024
1 parent 818bb29 commit d85d8f1
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 6 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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.",
Expand Down
10 changes: 6 additions & 4 deletions src/Container.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -344,7 +344,7 @@ export class Container<Services = {}> {
// `this` type, we ensure this Container can provide all the Dependencies required by the PartialContainer.
this: Container<FulfilledDependencies>,
container: PartialContainer<AdditionalServices, Dependencies>
): Container<Services & AdditionalServices>;
): Container<AddServices<Services, AdditionalServices>>;

/**
* Merges services from another `Container` into this container, creating a new `Container` instance.
Expand All @@ -362,7 +362,9 @@ export class Container<Services = {}> {
* @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<AdditionalServices>(container: Container<AdditionalServices>): Container<Services & AdditionalServices>;
provides<AdditionalServices>(
container: Container<AdditionalServices>
): Container<AddServices<Services, AdditionalServices>>;

/**
* Registers a new service in this Container using an `InjectableFunction`. This function defines how the service
Expand Down Expand Up @@ -400,7 +402,7 @@ export class Container<Services = {}> {
return new Container({
...this.factories,
...factories,
} as unknown as MaybeMemoizedFactories<Services & AdditionalServices>);
} as unknown as MaybeMemoizedFactories<AddServices<Services, AdditionalServices>>);
}
return this.providesService(fnOrContainer);
}
Expand Down
21 changes: 21 additions & 0 deletions src/__tests__/Container.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,27 @@ 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({}).provides(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", () => {
Expand Down
25 changes: 24 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,30 @@ export type ServicesFromInjectables<Injectables extends readonly AnyInjectable[]
// will see the mapped type instead of the AddService type alias. This produces better hints.
export type AddService<ParentServices, Token extends TokenType, Service> = 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, Services> = 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;

/**
Expand Down

0 comments on commit d85d8f1

Please sign in to comment.