From 72d5ed50ba2a7d29ab92706c41a4373e7316b907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Vlatkovic=CC=81?= <390700+ivandotv@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:13:22 +0200 Subject: [PATCH 1/3] feat!: fix resolution bugs, add typed tokens, speed up resolve Correctness: - a SINGLETON/REQUEST binding producing `undefined` returned an internal symbol on the first resolve and injected it into dependents afterwards - validate/validateSafe silently aborted when a bind key had already been seen as another binding's dependency - resolving a parent owned SINGLETON started a fresh request context, which split REQUEST scope, reordered postConstruct and hid circular references - setParent accepted cycles, turning every lookup into infinite recursion - unbindAll ignored the lock on an empty container Features: - token() typed bind keys, so resolve infers instead of being told - tryResolve, getKeys, and a `replace` bind option - Symbol.dispose on resolved singletons and on the container itself - every error is a PumpitError carrying an ERROR_CODE Performance (bound value ~2.5x, cached singleton ~5x, transient graph ~2.4x): - allocate request context maps lazily instead of on every resolve - parse injection metadata once at bind time - drop the write-only transient cache and the per lookup wrapper object Co-Authored-By: Claude Opus 5 --- .changeset/brave-hoops-cough.md | 17 + .changeset/olive-donkeys-shave.md | 15 + .changeset/quiet-moons-invite.md | 15 + README.md | 172 +++++++- package.json | 2 +- src/__tests__/container-api.test.ts | 356 ++++++++++++++++ src/__tests__/regressions.test.ts | 381 +++++++++++++++++ src/__tests__/token.test.ts | 137 +++++++ src/__tests__/validate.test.ts | 24 +- src/index.ts | 5 +- src/pumpit-error.ts | 55 ++- src/pumpit.ts | 616 +++++++++++++++++----------- src/types-internal.ts | 33 +- src/types.ts | 31 ++ src/utils.ts | 20 + 15 files changed, 1609 insertions(+), 270 deletions(-) create mode 100644 .changeset/brave-hoops-cough.md create mode 100644 .changeset/olive-donkeys-shave.md create mode 100644 .changeset/quiet-moons-invite.md create mode 100644 src/__tests__/container-api.test.ts create mode 100644 src/__tests__/regressions.test.ts create mode 100644 src/__tests__/token.test.ts diff --git a/.changeset/brave-hoops-cough.md b/.changeset/brave-hoops-cough.md new file mode 100644 index 0000000..7462e80 --- /dev/null +++ b/.changeset/brave-hoops-cough.md @@ -0,0 +1,17 @@ +--- +"pumpit": major +--- + +`PumpitError` is now the base class for every error the container throws and its constructor takes an `ErrorCode` instead of a `ValidationError[]`. Validation failures throw `PumpitValidationError`, a subclass that still carries `result`, but whose message now lists the unresolved keys instead of being the literal string `"Validation"`. + +`validateSafe` always returns a `ValidationResult` and no longer returns `undefined`. + +Injection metadata is read once when a value is bound instead of on every resolve, so `registerInjections` (or assigning `inject` / `INJECT_KEY`) must happen before `bindClass` and `bindFactory`. Later changes are no longer picked up. + +Dependencies marked optional are no longer reported by `validate` and `validateSafe`. + +`resolve` now infers the return type when the key is a class or a typed token, where it previously widened to `unknown`. + +`setParent` accepts `undefined` to detach a parent, and throws when the parent would create a cycle. + +Resolving is substantially faster: bound values ~2.5x, cached singletons ~5x, and a small transient graph ~2.4x, mostly by dropping per resolve allocations and parsing injection metadata at bind time. diff --git a/.changeset/olive-donkeys-shave.md b/.changeset/olive-donkeys-shave.md new file mode 100644 index 0000000..70ff6f1 --- /dev/null +++ b/.changeset/olive-donkeys-shave.md @@ -0,0 +1,15 @@ +--- +"pumpit": patch +--- + +Fix bindings that resolve to `undefined` leaking an internal symbol. A `SINGLETON` or `REQUEST` binding whose class or factory produced `undefined` returned that sentinel on the first resolve, and injected it into every dependent afterwards. + +Fix `validate` and `validateSafe` silently giving up. Reaching a binding that an earlier binding already listed as a dependency aborted the whole check, so `validate` did not throw and `validateSafe` returned `undefined`. Missing dependencies are now reported regardless of bind order, and optional dependencies are no longer reported at all. + +Fix resolution that crosses into a parent container starting a fresh request. A `SINGLETON` owned by a parent used to be resolved through a brand new context, which split `REQUEST` scope into two instances within a single `resolve` call, ran `postConstruct` hooks before the outer graph finished building, and hid circular references behind a stack overflow. + +Fix `setParent` accepting a parent that creates a cycle in the hierarchy, which turned every lookup into infinite recursion. + +Fix `unbindAll` succeeding on a locked container when the container was empty. + +Circular reference errors now report the full resolution path with the bound class names, instead of stringifying an internal wrapper function. diff --git a/.changeset/quiet-moons-invite.md b/.changeset/quiet-moons-invite.md new file mode 100644 index 0000000..fdea5dd --- /dev/null +++ b/.changeset/quiet-moons-invite.md @@ -0,0 +1,15 @@ +--- +"pumpit": minor +--- + +Add typed tokens. `token()` creates a `Symbol` bind key that carries its type, so `resolve` infers the result instead of being told, and bindings made under the token are type checked. `resolve` also infers the instance type when a class is used as the key. + +Add `tryResolve`, which returns `undefined` for an unbound key instead of throwing. A missing required dependency of a bound key still throws. + +Add `getKeys`, which lists the keys bound on the container, optionally including the parent chain. + +Add a `replace` bind option, so a key can be rebound without unbinding it first. The previous binding is unbound, which disposes its cached singleton. + +Support `Symbol.dispose` on resolved singletons, preferred over a `dispose` method. The container itself is now disposable, so `using container = new PumpIt()` unbinds everything on scope exit. + +Every error thrown by the container is now a `PumpitError` carrying a machine readable `code`, exported as `ERROR_CODE`. diff --git a/README.md b/README.md index 83de8c5..5e67197 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Codecov](https://img.shields.io/codecov/c/gh/ivandotv/pumpit)](https://app.codecov.io/gh/ivandotv/pumpit) [![GitHub license](https://img.shields.io/github/license/ivandotv/pumpit)](https://github.com/ivandotv/pumpit/blob/main/LICENSE) -`PumpIt` is a small [(~2KB)](https://bundlephobia.com/package/pumpit) dependency injection container without the decorators and zero dependencies, suitable for the browser. +`PumpIt` is a small [(~2.3KB)](https://bundlephobia.com/package/pumpit) dependency injection container without the decorators and zero dependencies, suitable for the browser. It supports different injection scopes, child containers, hooks etc... @@ -17,7 +17,9 @@ It supports different injection scopes, child containers, hooks etc... * [Registering factories](#registering-factories) * [Registering values](#registering-values) - [Resolving container data](#resolving-container-data) + * [Resolving without throwing](#resolving-without-throwing) - [Injection tokens](#injection-tokens) + * [Typed tokens](#typed-tokens) - [Injection scopes](#injection-scopes) * [Singleton](#singleton) * [Transient](#transient) @@ -30,12 +32,16 @@ It supports different injection scopes, child containers, hooks etc... - [Removing values from the container](#removing-values-from-the-container) * [Calling the dispose method](#calling-the-dispose-method) * [Removing all the values from the container](#removing-all-the-values-from-the-container) + * [Disposing the container](#disposing-the-container) * [Locking the container](#locking-the-container) +- [Replacing bindings](#replacing-bindings) +- [Inspecting the container](#inspecting-the-container) - [Child containers](#child-containers) * [Shadowing values](#shadowing-values) * [Checking for values](#checking-for-values) * [Child singletons](#child-singletons) * [Validating bindings](#validating-bindings) +- [Error handling](#error-handling) - [Helpers](#helpers) * [Register injections](#register-injections) - [API docs](#api-docs) @@ -77,12 +83,16 @@ class TestB {} container.bindClass(TestA, TestA).bindClass(bindKeyB, TestB) //resolve values -const instanceA = container.resolve(TestA) -const instanceB = container.resolve(bindKeyB) +const instanceA = container.resolve(TestA) // TestA, inferred from the class key +const instanceB = container.resolve(bindKeyB) instanceA.b // injected B instance ``` +> When the key **is** the class, `resolve` infers the instance type for you. For +> string and symbol keys there is nothing to infer from, so either pass the type +> explicitly or use a [typed token](#typed-tokens). + There is also alternative syntax that you can use when you don't want to use the static `inject` property, or you are importing a class from third-party packages. ```ts @@ -303,6 +313,22 @@ const container = new PumpIt() container.resolve('key_does_not_exist') // will throw ``` + +### Resolving without throwing + +Use `tryResolve` when a missing key is an acceptable outcome. It behaves exactly +like `resolve`, except an unbound key gives back `undefined` instead of throwing. + +```ts +const container = new PumpIt() + +container.tryResolve('key_does_not_exist') // undefined +``` + +> `tryResolve` only forgives the key you asked for. If the key **is** bound but one +> of its own required dependencies is missing, it still throws - make that +> dependency [optional](#optional-injections) if it should be tolerated. + ## Injection tokens Injection tokens are the values by which the injection container knows how to resolve registered data. They can be `string`, `Symbol`, or any object. @@ -330,6 +356,55 @@ class B { } ``` +### Typed tokens + +A plain `string` or `Symbol` key carries no type information, so `resolve` has no +way to know what comes back and you end up annotating every call by hand - and +nothing stops you from getting it wrong. + +`token()` creates a `Symbol` that remembers its type. Bindings made under it +are type checked, and `resolve` infers the result. + +```ts +import { PumpIt, token } from 'pumpit' + +type Config = { url: string; retries: number } + +const configToken = token('config') +const container = new PumpIt() + +container.bindValue(configToken, { url: 'https://example.com', retries: 3 }) + +const config = container.resolve(configToken) // Config, no type argument needed + +container.bindValue(configToken, 42) // compile error, 42 is not a Config +``` + +It works the same way for classes and factories, where the binding must produce +the token's type: + +```ts +const loggerToken = token('logger') + +container.bindClass(loggerToken, Logger) // ok +container.bindFactory(loggerToken, () => new Logger()) // ok + +container.bindClass(loggerToken, SomethingElse) // compile error +``` + +Tokens are ordinary symbols at runtime, so they can be injected like any other key: + +```ts +class Service { + static inject = [configToken] + + constructor(public config: Config) {} +} +``` + +> The description passed to `token()` is only used to make error messages +> readable. Two tokens created with the same description are still different keys. + ## Injection scopes There are four types of injection scopes: @@ -541,7 +616,9 @@ container.resolve('name') // throws error ### Calling the dispose method -If the class has a method `dispose()` it will automatically be called on the disposed of value, but **only** if the value is a `singleton`. +If the class has a method `dispose()` (or a `[Symbol.dispose]()` method, which +takes precedence) it will automatically be called on the disposed of value, but +**only** if the value is a `singleton`. Internally, the container will remove the value from its internal pool, and if the value was registered with the scope: `singleton` and the value has been resolved before (class has been instantiated or factory function executed). That means that the container holds an instance of the value, and it will try to call the `dispose of` method on that instance, or in the case of the factory, on whatever was returned from the factory. @@ -575,6 +652,21 @@ const callDispose = true container.unbindAll(callDispose) ``` +### Disposing the container + +The container itself implements `Symbol.dispose`, so it can be scoped with +`using` on runtimes that support explicit resource management. Leaving the scope +calls `unbindAll()`, which disposes every cached singleton. + +```ts +{ + using container = new PumpIt() + + container.bindClass(TestA, TestA, { scope: SCOPE.SINGLETON }) + container.resolve(TestA) +} // container.unbindAll() runs here, TestA instance is disposed +``` + ### Locking the container If the container is `locked` that particular container can't accept new bindings or unbind the values already in the container. @@ -597,6 +689,45 @@ container.bindClass(TestB,TestB) //throws error container.unbind(TestA) //throws error ``` + +## Replacing bindings + +Binding a key that is already taken throws. Pass `replace: true` when that is the +point - handy in tests, where a real dependency is swapped for a fake. + +```ts +const container = new PumpIt() + +container.bindClass('mailer', RealMailer, { scope: SCOPE.SINGLETON }) +container.resolve('mailer') + +container.bindClass('mailer', FakeMailer, { + scope: SCOPE.SINGLETON, + replace: true +}) + +container.resolve('mailer') // FakeMailer +``` + +Replacing unbinds the previous entry first, so its cached singleton is dropped and +disposed. A locked container still refuses the change. + +## Inspecting the container + +`getKeys()` lists everything bound on the container. Pass `true` to walk the +parent chain as well, where a shadowed key is reported once. + +```ts +const parent = new PumpIt() +const child = parent.child() + +parent.bindValue('parent_key', 1) +child.bindValue('child_key', 2) + +child.getKeys() // ['child_key'] +child.getKeys(true) // ['child_key', 'parent_key'] +``` + ## Child containers Every container instance can create a **child** container. Or every container can set it's parent. @@ -677,7 +808,9 @@ TestA.count === 2 Calling `validate` or `validateSafe` will validate the bindings in the container. It will check if all the dependencies that are required by other bindings are present in the container. -`validate` method will throw an error, while `validateSafe` will return a validation result. Calling these methods will not instantiate classes or run factory functions, so there is still a possibility that you will not get what you want when dependencies are resolved at runtime. +`validate` method will throw a `PumpitValidationError`, while `validateSafe` will always return a validation result. Calling these methods will not instantiate classes or run factory functions, so there is still a possibility that you will not get what you want when dependencies are resolved at runtime. + +Dependencies declared as [optional](#optional-injections) are allowed to be missing, so they are never reported. Keys bound on a parent container count as present. In the next example `RequestTest` class is not present in the container, but is needed in class `TestB` @@ -707,6 +840,30 @@ expect(result).toEqual({ ``` +## Error handling + +Every error the container throws is a `PumpitError` carrying a `code`, so failures +can be handled without matching on message strings. + +```ts +import { PumpIt, PumpitError, ERROR_CODE } from 'pumpit' + +try { + container.resolve('nope') +} catch (e) { + if (e instanceof PumpitError && e.code === ERROR_CODE.KEY_NOT_FOUND) { + // ... + } +} +``` + +The available codes are `KEY_NOT_FOUND`, `KEY_ALREADY_EXISTS`, `CIRCULAR_REFERENCE`, +`CONTAINER_LOCKED`, `PARENT_CYCLE` and `VALIDATION`. + +Validation failures throw a `PumpitValidationError`, a `PumpitError` subclass that +also carries the full `result` array described in +[validating bindings](#validating-bindings). + ## Helpers ### Register injections @@ -739,6 +896,11 @@ test("use helper to inject in to class", () => { expect(result.b).toBeInstanceOf(TestB) }) ``` + +> Call `registerInjections` **before** binding the class or factory. Injection +> metadata is read once, when the value is bound, so changes made afterwards are +> not picked up. + ## API docs `PumpIt` is written in TypeScript and ships its own type declarations, so the full API documentation is available directly in your editor via autocomplete and hover. No `@types/*` package is required. diff --git a/package.json b/package.json index 33506f8..c07ce92 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "pumpit", "version": "10.1.0", - "description": "Dependency injection container without decorators, supports circular dependencies and arrays of dependencies.", + "description": "Small dependency injection container without decorators. Typed tokens, four injection scopes, child containers and zero dependencies.", "keywords": [ "ioc", "di", diff --git a/src/__tests__/container-api.test.ts b/src/__tests__/container-api.test.ts new file mode 100644 index 0000000..86f4e78 --- /dev/null +++ b/src/__tests__/container-api.test.ts @@ -0,0 +1,356 @@ +import { describe, expect, test, vi } from "vitest" +import { PumpIt, SCOPE } from "../pumpit" +import { ERROR_CODE, PumpitError } from "../pumpit-error" + +describe("Container API", () => { + describe("tryResolve", () => { + test("returns undefined instead of throwing for an unbound key", () => { + const pumpIt = new PumpIt() + + expect(pumpIt.tryResolve("nope")).toBeUndefined() + }) + + test("resolves normally when the key is bound", () => { + const pumpIt = new PumpIt() + class TestA {} + pumpIt.bindClass(TestA, TestA) + + expect(pumpIt.tryResolve(TestA)).toBeInstanceOf(TestA) + }) + + test("still throws when a required dependency is missing", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["missing"] + } + pumpIt.bindClass(TestA, TestA) + + expect(() => pumpIt.tryResolve(TestA)).toThrow("not found") + }) + + test("searches the parent container", () => { + const parent = new PumpIt() + const child = parent.child() + parent.bindValue("key", "value") + + expect(child.tryResolve("key")).toBe("value") + }) + + test("runs post construct hooks like resolve does", () => { + const pumpIt = new PumpIt() + const postConstruct = vi.fn() + + class TestA { + postConstruct() { + postConstruct() + } + } + pumpIt.bindClass(TestA, TestA) + pumpIt.tryResolve(TestA) + + expect(postConstruct).toHaveBeenCalledTimes(1) + }) + }) + + describe("getKeys", () => { + test("lists the keys bound on the container", () => { + const pumpIt = new PumpIt() + const symbolKey = Symbol("sym") + class TestA {} + + pumpIt + .bindValue("value", 1) + .bindClass(TestA, TestA) + .bindFactory(symbolKey, () => 2) + + expect(pumpIt.getKeys()).toEqual(["value", TestA, symbolKey]) + }) + + test("does not include parent keys by default", () => { + const parent = new PumpIt() + const child = parent.child() + + parent.bindValue("parent_key", 1) + child.bindValue("child_key", 2) + + expect(child.getKeys()).toEqual(["child_key"]) + }) + + test("includes parent keys when asked", () => { + const parent = new PumpIt() + const child = parent.child() + + parent.bindValue("parent_key", 1) + child.bindValue("child_key", 2) + + expect(child.getKeys(true)).toEqual(["child_key", "parent_key"]) + }) + + test("a shadowed key is reported once", () => { + const parent = new PumpIt() + const child = parent.child() + + parent.bindValue("shared", 1) + child.bindValue("shared", 2) + + expect(child.getKeys(true)).toEqual(["shared"]) + }) + + test("reflects unbinding", () => { + const pumpIt = new PumpIt() + pumpIt.bindValue("a", 1).bindValue("b", 2) + pumpIt.unbind("a") + + expect(pumpIt.getKeys()).toEqual(["b"]) + }) + }) + + describe("replace", () => { + test("binding an existing key throws without replace", () => { + const pumpIt = new PumpIt() + pumpIt.bindValue("key", 1) + + expect(() => pumpIt.bindValue("key", 2)).toThrow("already exists") + }) + + test("replace swaps a bound value", () => { + const pumpIt = new PumpIt() + pumpIt.bindValue("key", 1) + pumpIt.bindValue("key", 2, { replace: true }) + + expect(pumpIt.resolve("key")).toBe(2) + }) + + test("replace swaps a bound class", () => { + const pumpIt = new PumpIt() + class TestA {} + class TestB {} + + pumpIt.bindClass("key", TestA) + pumpIt.bindClass("key", TestB, { replace: true }) + + expect(pumpIt.resolve("key")).toBeInstanceOf(TestB) + }) + + test("replace swaps a bound factory", () => { + const pumpIt = new PumpIt() + + pumpIt.bindFactory("key", () => "one") + pumpIt.bindFactory("key", () => "two", { replace: true }) + + expect(pumpIt.resolve("key")).toBe("two") + }) + + test("replacing drops the cached singleton and disposes it", () => { + const pumpIt = new PumpIt() + const disposeCall = vi.fn() + + class Old { + dispose() { + disposeCall() + } + } + class New {} + + pumpIt.bindClass("key", Old, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + + pumpIt.bindClass("key", New, { scope: SCOPE.SINGLETON, replace: true }) + + expect(disposeCall).toHaveBeenCalledTimes(1) + expect(pumpIt.resolve("key")).toBeInstanceOf(New) + }) + + test("replace still respects the lock", () => { + const pumpIt = new PumpIt() + pumpIt.bindValue("key", 1) + pumpIt.lock() + + expect(() => pumpIt.bindValue("key", 2, { replace: true })).toThrow( + "Container is locked", + ) + }) + + test("replace works on a key that is not bound yet", () => { + const pumpIt = new PumpIt() + + expect(() => pumpIt.bindValue("key", 1, { replace: true })).not.toThrow() + expect(pumpIt.resolve("key")).toBe(1) + }) + }) + + describe("disposal", () => { + test("Symbol.dispose is preferred over a dispose method", () => { + const pumpIt = new PumpIt() + const symbolDispose = vi.fn() + const methodDispose = vi.fn() + + class TestA { + [Symbol.dispose]() { + symbolDispose() + } + + dispose() { + methodDispose() + } + } + + pumpIt.bindClass("key", TestA, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + pumpIt.unbind("key") + + expect(symbolDispose).toHaveBeenCalledTimes(1) + expect(methodDispose).not.toHaveBeenCalled() + }) + + test("a plain dispose method still works", () => { + const pumpIt = new PumpIt() + const methodDispose = vi.fn() + + class TestA { + dispose() { + methodDispose() + } + } + + pumpIt.bindClass("key", TestA, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + pumpIt.unbind("key") + + expect(methodDispose).toHaveBeenCalledTimes(1) + }) + + test("the container itself is disposable", () => { + const pumpIt = new PumpIt() + const disposeCall = vi.fn() + + class TestA { + dispose() { + disposeCall() + } + } + + pumpIt.bindClass("key", TestA, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + + pumpIt[Symbol.dispose]() + + expect(disposeCall).toHaveBeenCalledTimes(1) + expect(pumpIt.getKeys()).toEqual([]) + }) + + test("`using` unbinds everything on scope exit", () => { + const disposeCall = vi.fn() + + class TestA { + dispose() { + disposeCall() + } + } + + let seen: PumpIt | undefined + { + using pumpIt = new PumpIt() + pumpIt.bindClass("key", TestA, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + seen = pumpIt + + expect(seen.getKeys()).toEqual(["key"]) + } + + expect(disposeCall).toHaveBeenCalledTimes(1) + expect(seen.getKeys()).toEqual([]) + }) + }) + + describe("error codes", () => { + test("every container error is a PumpitError with a code", () => { + const pumpIt = new PumpIt() + + const cases: [() => void, string][] = [ + [() => pumpIt.resolve("nope"), ERROR_CODE.KEY_NOT_FOUND], + [() => pumpIt.unbind("nope"), ERROR_CODE.KEY_NOT_FOUND], + [ + () => { + pumpIt.bindValue("dupe", 1) + pumpIt.bindValue("dupe", 2) + }, + ERROR_CODE.KEY_ALREADY_EXISTS, + ], + [() => pumpIt.setParent(pumpIt), ERROR_CODE.PARENT_CYCLE], + ] + + for (const [fn, code] of cases) { + try { + fn() + throw new Error("expected the call to throw") + } catch (e) { + expect(e).toBeInstanceOf(PumpitError) + expect((e as PumpitError).code).toBe(code) + } + } + + const locked = new PumpIt() + locked.lock() + try { + locked.bindValue("key", 1) + } catch (e) { + expect((e as PumpitError).code).toBe(ERROR_CODE.CONTAINER_LOCKED) + } + }) + + test("errors are named", () => { + const pumpIt = new PumpIt() + + try { + pumpIt.resolve("nope") + } catch (e) { + expect((e as Error).name).toBe("PumpitError") + } + + expect.assertions(1) + }) + + test("a plain object key is described in the message", () => { + const pumpIt = new PumpIt() + const objectKey = {} + + expect(() => pumpIt.resolve(objectKey)).toThrow("[object Object]") + }) + + test("a circular reference between anonymous classes still reports a path", () => { + const pumpIt = new PumpIt() + // a class returned from a function gets no inferred name + const anonymous = () => class {} + + pumpIt.bindClass("a", { value: anonymous(), inject: ["b"] }) + pumpIt.bindClass("b", { value: anonymous(), inject: ["a"] }) + + expect(() => pumpIt.resolve("a")).toThrow( + "Circular reference detected: [ a ] -> [ b ] -> [ a ]", + ) + }) + + test("the validation message is pluralised", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["one", "two"] + } + pumpIt.bindClass(TestA, TestA) + + expect(() => pumpIt.validate()).toThrow("2 unresolved dependencies") + }) + }) + + describe("lock", () => { + test("isLocked reflects the lock", () => { + const pumpIt = new PumpIt() + + expect(pumpIt.isLocked()).toBe(false) + pumpIt.lock() + expect(pumpIt.isLocked()).toBe(true) + }) + }) +}) diff --git a/src/__tests__/regressions.test.ts b/src/__tests__/regressions.test.ts new file mode 100644 index 0000000..6dcb128 --- /dev/null +++ b/src/__tests__/regressions.test.ts @@ -0,0 +1,381 @@ +import { describe, expect, test, vi } from "vitest" +import { PumpIt, SCOPE } from "../pumpit" +import { ERROR_CODE, type PumpitError } from "../pumpit-error" +import { get } from "../utils" + +describe("Regressions", () => { + describe("bindings that resolve to undefined", () => { + test("singleton returns undefined on the first resolve, not an internal sentinel", () => { + const pumpIt = new PumpIt() + + pumpIt.bindFactory("key", () => undefined, { scope: SCOPE.SINGLETON }) + + expect(pumpIt.resolve("key")).toBeUndefined() + expect(pumpIt.resolve("key")).toBeUndefined() + }) + + test("request scope returns undefined on the first resolve", () => { + const pumpIt = new PumpIt() + + pumpIt.bindFactory("key", () => undefined, { scope: SCOPE.REQUEST }) + + expect(pumpIt.resolve("key")).toBeUndefined() + }) + + test("the factory only runs once even though it produced undefined", () => { + const pumpIt = new PumpIt() + const factory = vi.fn(() => undefined) + + pumpIt.bindFactory("key", factory, { scope: SCOPE.SINGLETON }) + + pumpIt.resolve("key") + pumpIt.resolve("key") + + expect(factory).toHaveBeenCalledTimes(1) + }) + + test("an undefined singleton is injected as undefined", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["undefined_key"] + + constructor(public dep: unknown) {} + } + + pumpIt.bindFactory("undefined_key", () => undefined, { + scope: SCOPE.SINGLETON, + }) + pumpIt.bindClass(TestA, TestA) + + // first resolve populates the singleton cache, second one reads it back + expect(pumpIt.resolve(TestA).dep).toBeUndefined() + expect(pumpIt.resolve(TestA).dep).toBeUndefined() + }) + }) + + describe("validation", () => { + test("reports a missing dependency regardless of bind order", () => { + const pumpIt = new PumpIt() + const missing = Symbol("missing") + + class TestA { + static inject = [missing] + } + class TestB { + static inject = [TestA] + } + + // TestA is seen as a dependency of TestB before its own pool entry is + // reached, which used to abort the whole validation + pumpIt.bindClass(TestB, TestB).bindClass(TestA, TestA) + + expect(pumpIt.validateSafe()).toEqual({ + valid: false, + errors: [{ key: missing, wantedBy: [TestA] }], + }) + expect(() => pumpIt.validate()).toThrow("Validation failed") + }) + + test("validateSafe always returns a result", () => { + const pumpIt = new PumpIt() + + class TestA {} + class TestB { + static inject = [TestA] + } + pumpIt.bindClass(TestA, TestA).bindClass(TestB, TestB) + + expect(pumpIt.validateSafe()).toEqual({ valid: true, errors: [] }) + }) + + test("a chain of dependencies is fully walked", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["missing_a"] + } + class TestB { + static inject = [TestA, "missing_b"] + } + class TestC { + static inject = [TestB] + } + + pumpIt + .bindClass(TestC, TestC) + .bindClass(TestB, TestB) + .bindClass(TestA, TestA) + + const result = pumpIt.validateSafe() + + expect(result.valid).toBe(false) + expect(result.errors).toEqual([ + { key: "missing_b", wantedBy: [TestB] }, + { key: "missing_a", wantedBy: [TestA] }, + ]) + }) + + test("a missing optional dependency is not a validation error", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = [get("nope", { optional: true })] + } + + pumpIt.bindClass(TestA, TestA) + + expect(pumpIt.validateSafe()).toEqual({ valid: true, errors: [] }) + expect(() => pumpIt.validate()).not.toThrow() + }) + + test("a missing required dependency is still an error next to an optional one", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = [get("nope", { optional: true }), "required_key"] + } + + pumpIt.bindClass(TestA, TestA) + + expect(pumpIt.validateSafe()).toEqual({ + valid: false, + errors: [{ key: "required_key", wantedBy: [TestA] }], + }) + }) + + test("validation looks into the parent container", () => { + const parent = new PumpIt() + const child = parent.child() + + class Dep {} + class TestA { + static inject = [Dep] + } + + parent.bindClass(Dep, Dep) + child.bindClass(TestA, TestA) + + expect(child.validateSafe()).toEqual({ valid: true, errors: [] }) + }) + }) + + describe("resolving across containers keeps one request context", () => { + test("request scope is shared with a parent owned singleton", () => { + const parent = new PumpIt() + const child = parent.child() + + class Req {} + class OnParent { + static inject = ["req"] + + constructor(public req: Req) {} + } + class OnChild { + static inject = ["req", "on_parent"] + + constructor( + public req: Req, + public onParent: OnParent, + ) {} + } + + parent.bindClass("req", Req, { scope: SCOPE.REQUEST }) + parent.bindClass("on_parent", OnParent, { scope: SCOPE.SINGLETON }) + child.bindClass("on_child", OnChild) + + const result = child.resolve("on_child") + + expect(result.req).toBe(result.onParent.req) + }) + + test("post construct runs after the whole graph is built", () => { + const order: string[] = [] + const parent = new PumpIt() + const child = parent.child() + + class Dep { + postConstruct() { + order.push("dep.postConstruct") + } + } + class Root { + static inject = ["dep"] + + constructor(public dep: Dep) { + order.push("root.constructor") + } + + postConstruct() { + order.push("root.postConstruct") + } + } + + parent.bindClass("dep", Dep, { scope: SCOPE.SINGLETON }) + child.bindClass("root", Root) + + child.resolve("root") + + expect(order).toEqual([ + "root.constructor", + "dep.postConstruct", + "root.postConstruct", + ]) + }) + + test("a circular reference across containers is detected", () => { + const parent = new PumpIt() + const child = parent.child() + + class TestA { + static inject = ["b"] + } + class TestB { + static inject = ["a"] + } + + parent.bindClass("a", TestA, { scope: SCOPE.SINGLETON }) + parent.bindClass("b", TestB, { scope: SCOPE.SINGLETON }) + + expect(() => child.resolve("a")).toThrow("Circular reference detected") + }) + + test("the singleton still lands on the container that owns the key", () => { + const parent = new PumpIt() + const child = parent.child() + + class TestA {} + parent.bindClass("a", TestA, { scope: SCOPE.SINGLETON }) + + expect(child.resolve("a")).toBe(parent.resolve("a")) + }) + }) + + describe("circular reference reporting", () => { + test("the message shows the full path with bound class names", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["b"] + } + class TestB { + static inject = ["c"] + } + class TestC { + static inject = ["a"] + } + + pumpIt.bindClass("a", TestA).bindClass("b", TestB).bindClass("c", TestC) + + try { + pumpIt.resolve("a") + } catch (e) { + expect((e as Error).message).toBe( + "Circular reference detected: [ a: TestA ] -> [ b: TestB ] -> [ c: TestC ] -> [ a: TestA ]", + ) + expect((e as PumpitError).code).toBe(ERROR_CODE.CIRCULAR_REFERENCE) + } + + expect.assertions(2) + }) + + test("a diamond shaped graph is not reported as circular", () => { + const pumpIt = new PumpIt() + + class Leaf {} + class Left { + static inject = ["leaf"] + } + class Right { + static inject = ["leaf"] + } + class Root { + static inject = ["left", "right"] + } + + pumpIt + .bindClass("leaf", Leaf) + .bindClass("left", Left) + .bindClass("right", Right) + .bindClass("root", Root) + + expect(() => pumpIt.resolve("root")).not.toThrow() + }) + + test("the same key can be injected twice into one binding", () => { + const pumpIt = new PumpIt() + + class Leaf {} + class Root { + static inject = ["leaf", "leaf"] + + constructor( + public one: Leaf, + public two: Leaf, + ) {} + } + + pumpIt.bindClass("leaf", Leaf).bindClass("root", Root) + + const root = pumpIt.resolve("root") + + expect(root.one).toBeInstanceOf(Leaf) + expect(root.two).toBeInstanceOf(Leaf) + expect(root.one).not.toBe(root.two) + }) + }) + + describe("container hierarchy", () => { + test("a container cannot become its own parent", () => { + const pumpIt = new PumpIt() + + expect(() => pumpIt.setParent(pumpIt)).toThrow("cycle") + }) + + test("a longer parent cycle is rejected", () => { + const a = new PumpIt("a") + const b = new PumpIt("b") + const c = new PumpIt("c") + + b.setParent(a) + c.setParent(b) + + try { + a.setParent(c) + } catch (e) { + expect((e as PumpitError).code).toBe(ERROR_CODE.PARENT_CYCLE) + } + + expect(a.getParent()).toBeUndefined() + expect.assertions(2) + }) + + test("the parent can be detached", () => { + const parent = new PumpIt() + const child = parent.child() + + child.setParent(undefined) + + expect(child.getParent()).toBeUndefined() + }) + }) + + describe("unbind", () => { + test("unbindAll throws on a locked container even when it is empty", () => { + const pumpIt = new PumpIt() + pumpIt.lock() + + expect(() => pumpIt.unbindAll()).toThrow("Container is locked") + }) + + test("a singleton that resolved to undefined does not break unbind", () => { + const pumpIt = new PumpIt() + + pumpIt.bindFactory("key", () => undefined, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + + expect(() => pumpIt.unbind("key")).not.toThrow() + expect(pumpIt.has("key")).toBe(false) + }) + }) +}) diff --git a/src/__tests__/token.test.ts b/src/__tests__/token.test.ts new file mode 100644 index 0000000..19ecb32 --- /dev/null +++ b/src/__tests__/token.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, expectTypeOf, test } from "vitest" +import { PumpIt } from "../pumpit" +import { token } from "../utils" + +type Config = { url: string; retries: number } + +class Logger { + log(msg: string) { + return msg + } +} + +describe("Typed tokens", () => { + test("a token is a symbol and works as a bind key", () => { + const pumpIt = new PumpIt() + const configToken = token("config") + + expect(typeof configToken).toBe("symbol") + + const config = { url: "https://example.com", retries: 3 } + pumpIt.bindValue(configToken, config) + + expect(pumpIt.resolve(configToken)).toBe(config) + }) + + test("the description shows up in error messages", () => { + const pumpIt = new PumpIt() + const configToken = token("app_config") + + expect(() => pumpIt.resolve(configToken)).toThrow("app_config") + }) + + test("two tokens with the same description are different keys", () => { + const pumpIt = new PumpIt() + const one = token("same") + const two = token("same") + + pumpIt.bindValue(one, "one").bindValue(two, "two") + + expect(pumpIt.resolve(one)).toBe("one") + expect(pumpIt.resolve(two)).toBe("two") + }) + + test("a class bound to a token resolves to the instance", () => { + const pumpIt = new PumpIt() + const loggerToken = token("logger") + + pumpIt.bindClass(loggerToken, Logger) + + expect(pumpIt.resolve(loggerToken)).toBeInstanceOf(Logger) + }) + + test("a factory bound to a token resolves to its return value", () => { + const pumpIt = new PumpIt() + const greetToken = token("greet") + + pumpIt.bindFactory(greetToken, () => "hello") + + expect(pumpIt.resolve(greetToken)).toBe("hello") + }) + + test("tokens can be injected like any other key", () => { + const pumpIt = new PumpIt() + const configToken = token("config") + const config = { url: "https://example.com", retries: 3 } + + class Service { + static inject = [configToken] + + constructor(public config: Config) {} + } + + pumpIt.bindValue(configToken, config) + pumpIt.bindClass(Service, Service) + + expect(pumpIt.resolve(Service).config).toBe(config) + }) + + // `expectTypeOf` still evaluates its argument, so everything asserted on here + // is bound first and the assertions double as runtime checks + describe("types", () => { + test("resolve infers the type carried by the token", () => { + const pumpIt = new PumpIt() + const configToken = token("config") + pumpIt.bindValue(configToken, { url: "https://example.com", retries: 3 }) + + expectTypeOf(pumpIt.resolve(configToken)).toEqualTypeOf() + }) + + test("tryResolve widens the token type with undefined", () => { + const pumpIt = new PumpIt() + const configToken = token("config") + + expectTypeOf(pumpIt.tryResolve(configToken)).toEqualTypeOf< + Config | undefined + >() + }) + + test("resolve infers the instance type when a class is the key", () => { + const pumpIt = new PumpIt() + pumpIt.bindClass(Logger, Logger) + + expectTypeOf(pumpIt.resolve(Logger)).toEqualTypeOf() + }) + + test("an explicit type argument still works with a plain key", () => { + const pumpIt = new PumpIt() + pumpIt.bindClass("logger", Logger) + + expectTypeOf(pumpIt.resolve("logger")).toEqualTypeOf() + }) + + test("binding the wrong value type to a token is a type error", () => { + const pumpIt = new PumpIt() + + // @ts-expect-error a number is not a Config + pumpIt.bindValue(token("a"), 42) + + // @ts-expect-error Logger does not produce a Config + pumpIt.bindClass(token("b"), Logger) + + // @ts-expect-error the factory returns a string, not a Config + pumpIt.bindFactory(token("c"), () => "nope") + + expect(pumpIt.getKeys()).toHaveLength(3) + }) + + test("a plain symbol is not treated as a token", () => { + const pumpIt = new PumpIt() + const plain = Symbol("plain") + + pumpIt.bindValue(plain, 42) + + expectTypeOf(pumpIt.resolve(plain)).toEqualTypeOf() + }) + }) +}) diff --git a/src/__tests__/validate.test.ts b/src/__tests__/validate.test.ts index b20d84c..a93e980 100644 --- a/src/__tests__/validate.test.ts +++ b/src/__tests__/validate.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "vitest" import { PumpIt } from "../pumpit" -import { PumpitError } from "../pumpit-error" +import { + ERROR_CODE, + type PumpitError, + PumpitValidationError, +} from "../pumpit-error" describe("Validation", () => { test("throw if the dependency is not found", () => { @@ -22,13 +26,14 @@ describe("Validation", () => { try { pumpIt.validate() } catch (e) { - expect((e as PumpitError).result).toEqual([ + expect((e as PumpitValidationError).result).toEqual([ { key: RequestTest, wantedBy: [TestB] }, ]) - expect((e as PumpitError).message).toEqual("Validation") - expect(e).toBeInstanceOf(PumpitError) + expect((e as PumpitValidationError).message).toContain("RequestTest") + expect((e as PumpitError).code).toBe(ERROR_CODE.VALIDATION) + expect(e).toBeInstanceOf(PumpitValidationError) } - expect.assertions(3) + expect.assertions(4) }) test("throw error with multiple dependencies missing", () => { @@ -55,14 +60,15 @@ describe("Validation", () => { try { pumpIt.validate() } catch (e) { - expect((e as PumpitError).result).toEqual([ + expect((e as PumpitValidationError).result).toEqual([ { key: requestKey, wantedBy: [TestA, TestB] }, ]) - expect((e as PumpitError).message).toEqual("Validation") - expect(e).toBeInstanceOf(PumpitError) + expect((e as PumpitValidationError).message).toContain("requestKey") + expect((e as PumpitError).code).toBe(ERROR_CODE.VALIDATION) + expect(e).toBeInstanceOf(PumpitValidationError) } - expect.assertions(3) + expect.assertions(4) }) test("return validation result instead of throwing", () => { diff --git a/src/index.ts b/src/index.ts index f2911c5..6230a57 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,9 +3,9 @@ export * from "./pumpit.js" export * from "./types.js" export * from "./pumpit-error.js" -import { INJECT_KEY, get, registerInjections } from "./utils.js" +import { INJECT_KEY, get, registerInjections, token } from "./utils.js" -export { get, INJECT_KEY, registerInjections } +export { get, INJECT_KEY, registerInjections, token } // types that appear in the public signatures above, so consumers can name them export type { @@ -15,4 +15,5 @@ export type { InjectionFn, InjectionOptions, ParsedInjectionData, + Token, } from "./utils.js" diff --git a/src/pumpit-error.ts b/src/pumpit-error.ts index 771204f..d30e1d2 100644 --- a/src/pumpit-error.ts +++ b/src/pumpit-error.ts @@ -1,10 +1,63 @@ import type { ValidationError } from "./types" +import { keyToString } from "./utils" +/** Machine readable reason attached to every error the container throws */ +export const ERROR_CODE = { + /** The key is not bound in the container (or any of its parents) */ + KEY_NOT_FOUND: "KEY_NOT_FOUND", + /** Something is already bound under the key */ + KEY_ALREADY_EXISTS: "KEY_ALREADY_EXISTS", + /** Two or more bindings depend on each other */ + CIRCULAR_REFERENCE: "CIRCULAR_REFERENCE", + /** The container is locked {@link PumpIt.lock | PumpIt.lock()} */ + CONTAINER_LOCKED: "CONTAINER_LOCKED", + /** Setting the parent would create a cycle in the container hierarchy */ + PARENT_CYCLE: "PARENT_CYCLE", + /** {@link PumpIt.validate | PumpIt.validate()} found unresolvable bindings */ + VALIDATION: "VALIDATION", +} as const + +/** Available error codes {@link ERROR_CODE}*/ +export type ErrorCode = keyof typeof ERROR_CODE + +/** + * Every error thrown by the container is a `PumpitError`, so it can be caught + * by type and branched on via {@link PumpitError.code}. + */ export class PumpitError extends Error { constructor( message: string, - public result: ValidationError[], + /** Machine readable reason for the error {@link ERROR_CODE} */ + public readonly code: ErrorCode, ) { super(message) + this.name = "PumpitError" + } +} + +/** + * Thrown by {@link PumpIt.validate | PumpIt.validate()} when the container + * holds bindings whose dependencies cannot be resolved. + */ +export class PumpitValidationError extends PumpitError { + constructor( + /** Every dependency that could not be found, and who wants it */ + public readonly result: ValidationError[], + ) { + super(formatValidationErrors(result), ERROR_CODE.VALIDATION) + this.name = "PumpitValidationError" } } + +function formatValidationErrors(errors: ValidationError[]): string { + const lines = errors.map( + ({ key, wantedBy }) => + ` [ ${keyToString(key)} ] wanted by: ${wantedBy + .map((k) => `[ ${keyToString(k)} ]`) + .join(", ")}`, + ) + + return `Validation failed, ${errors.length} unresolved ${ + errors.length === 1 ? "dependency" : "dependencies" + }:\n${lines.join("\n")}` +} diff --git a/src/pumpit.ts b/src/pumpit.ts index 655925c..90e31f8 100644 --- a/src/pumpit.ts +++ b/src/pumpit.ts @@ -1,22 +1,30 @@ -import { PumpitError } from "./pumpit-error" +import { ERROR_CODE, PumpitError, PumpitValidationError } from "./pumpit-error" import type { AvailableScopes, BindKey, + BindOptions, ClassConstructor, - ClassOptions, ClassValue, - FactoryFn, - FactoryOptions, + ClassValueFor, FactoryValue, + FactoryValueFor, ValidationError, ValidationResult, + ValueBindOptions, } from "./types" import type { RequestCtx } from "./types-internal" -import type { ClassPoolData, FactoryPoolData, PoolData } from "./types-internal" +import type { + ClassPoolData, + FactoryPoolData, + PoolData, + ResolverFn, +} from "./types-internal" import { INJECT_KEY, - type Injection, type InjectionData, + type InjectionOptions, + type ParsedInjectionData, + type Token, keyToString, parseInjectionData, } from "./utils" @@ -25,6 +33,17 @@ import { const UNDEFINED_RESULT = Symbol() const DISPOSE_PROP = "dispose" + +// `Symbol.dispose` only exists on newer runtimes, so it is looked up once and +// treated as optional everywhere else +const DISPOSE_SYMBOL: symbol | undefined = + typeof Symbol.dispose === "symbol" ? Symbol.dispose : undefined + +// shared, never mutated, so the hot path does not allocate an options object +const NO_OPTIONS: InjectionOptions = {} +const OPTIONAL_OPTIONS: InjectionOptions = { optional: true } +const NO_DEPS: any[] = [] + /** Constants that represent the type of values that can be binded*/ export const TYPE = { VALUE: "VALUE", @@ -49,6 +68,7 @@ export const SCOPE = { CONTAINER_SINGLETON: "CONTAINER_SINGLETON", } as const +// biome-ignore lint/suspicious/noUnsafeDeclarationMerging: merged with the `Disposable` interface at the bottom of the file, where the member is installed on the prototype export class PumpIt { protected pool: Map = new Map() @@ -56,8 +76,6 @@ export class PumpIt { protected parent: PumpIt | undefined - protected currentCtx: RequestCtx | null = null - protected name: string | undefined protected locked = false @@ -73,15 +91,21 @@ export class PumpIt { return this.name } - protected add(key: BindKey, info: PoolData): void { + protected add(key: BindKey, info: PoolData, replace: boolean): void { if (this.locked) { - throw new Error("Container is locked") + throw new PumpitError("Container is locked", ERROR_CODE.CONTAINER_LOCKED) } - const dataHit = this.pool.get(key) - if (dataHit) { - throw new Error(`Key: ${keyToString(key)} already exists`) + if (this.pool.has(key)) { + if (!replace) { + throw new PumpitError( + `Key: ${keyToString(key)} already exists`, + ERROR_CODE.KEY_ALREADY_EXISTS, + ) + } + this.unbind(key) } + this.pool.set(key, info) } @@ -89,34 +113,42 @@ export class PumpIt { * Unbinds a dependency from the container. * @param key - The key to unbind. * @param dispose - Optional. Specifies whether to call dispose method if available. Default is `true`. - * @throws {Error} If the container is locked or if the key is not found. + * @throws {PumpitError} If the container is locked or if the key is not found. */ unbind(key: BindKey, dispose = true): void { if (this.locked) { - throw new Error("Container is locked") + throw new PumpitError("Container is locked", ERROR_CODE.CONTAINER_LOCKED) } - const poolData = this.pool.get(key) - if (poolData) { - const singleton = this.singletonCache.get(key) - this.pool.delete(key) - this.singletonCache.delete(key) + if (!this.pool.has(key)) { + throw new PumpitError( + `Key: ${keyToString(key)} not found`, + ERROR_CODE.KEY_NOT_FOUND, + ) + } - if (singleton && dispose) { - this.callDispose(singleton) - } + // `has` rather than a truthiness check, so falsy singletons are disposed too + const hasSingleton = this.singletonCache.has(key) + const singleton = this.singletonCache.get(key) - return + this.pool.delete(key) + this.singletonCache.delete(key) + + if (hasSingleton && dispose) { + this.callDispose(singleton) } - throw new Error(`Key: ${keyToString(key)} not found`) } /** * Unbinds all dependencies from the container. * @param callDispose - Whether to call the `dispose` method on each unbound dependency. Default is `true` - * + * @throws {PumpitError} If the container is locked. */ unbindAll(callDispose = true) { + if (this.locked) { + throw new PumpitError("Container is locked", ERROR_CODE.CONTAINER_LOCKED) + } + for (const key of this.pool.keys()) { this.unbind(key, callDispose) } @@ -127,17 +159,27 @@ export class PumpIt { protected callDispose(value: unknown): void { // `in` throws on primitives, so guard on the object shape first if ( - (typeof value === "object" || typeof value === "function") && - value !== null && - DISPOSE_PROP in value + (typeof value !== "object" && typeof value !== "function") || + value === null ) { - const dispose = (value as Record)[ - DISPOSE_PROP - ] - if (typeof dispose === "function") { - dispose.call(value) + return + } + + const target = value as Record + + if (DISPOSE_SYMBOL !== undefined) { + const symbolDispose = target[DISPOSE_SYMBOL] + if (typeof symbolDispose === "function") { + symbolDispose.call(value) + + return } } + + const dispose = target[DISPOSE_PROP] + if (typeof dispose === "function") { + dispose.call(value) + } } /** @@ -148,24 +190,55 @@ export class PumpIt { * @returns A boolean value indicating whether the key exists in the pool or its parent pool. */ has(key: BindKey, searchParent = true): boolean { - if (searchParent && this.parent) { - return !!this.getInjectable(key) + if (this.pool.has(key)) { + return true + } + + return searchParent && this.parent !== undefined + ? this.parent.has(key, true) + : false + } + + /** + * Lists every key currently bound in the container. + * + * @param includeParent - Optional. Also include keys bound on the parent chain, + * shadowed keys reported once. Default is `false`. + */ + getKeys(includeParent = false): BindKey[] { + if (!includeParent || this.parent === undefined) { + return Array.from(this.pool.keys()) + } + + const keys = new Set(this.pool.keys()) + for (const key of this.parent.getKeys(true)) { + keys.add(key) } - return this.pool.has(key) + return Array.from(keys) } /** * Binds value. Value is treated as a singleton and ti will always resolve to the same data (value) * * @param key - key to resolve binded value {@link BindKey} + * @param options - bind options {@link ValueBindOptions} */ - bindValue(key: BindKey, value: T): this { - this.add(key, { - type: TYPE.VALUE, - scope: SCOPE.SINGLETON, - value, - }) + bindValue( + key: K, + value: K extends Token ? V : T, + options?: ValueBindOptions, + ): this + bindValue(key: BindKey, value: unknown, options?: ValueBindOptions): this { + this.add( + key, + { + type: TYPE.VALUE, + scope: SCOPE.SINGLETON, + value, + }, + options?.replace ?? false, + ) return this } @@ -175,94 +248,128 @@ export class PumpIt { * Number of executions depends on the scope used. * * @param key - key to resolve binded value {@link BindKey} - * @param options - bind options {@link FactoryOptions} + * @param options - bind options {@link BindOptions} */ - bindFactory( - key: BindKey, - value: T, - options?: Omit, "type">, - ): this { + bindFactory( + key: K, + value: K extends Token ? FactoryValueFor : FactoryValue, + options?: BindOptions, + ): this + bindFactory(key: BindKey, value: FactoryValue, options?: BindOptions): this { const { exec, inject } = this.parseValue(value) - const resolve = (...args: any[]) => exec(...args) + const resolve = ((...args: any[]) => exec(...args)) as ResolverFn resolve.inject = inject resolve.original = exec - this.add(key, { - ...options, - type: TYPE.FACTORY, - scope: options?.scope || SCOPE.TRANSIENT, - value: resolve, - }) + this.add( + key, + { + type: TYPE.FACTORY, + scope: options?.scope ?? SCOPE.TRANSIENT, + value: resolve, + }, + options?.replace ?? false, + ) return this } - protected parseValue( - value: T | { value: T; inject: InjectionData }, - ): { exec: T; inject: InjectionData | undefined } { - const exec = typeof value === "function" ? value : value.value - // injections come either from `registerInjections` (symbol keyed) or from - // an `inject` property on the class/factory itself, or on the wrapper - const source = value as { - [INJECT_KEY]?: InjectionData - inject?: InjectionData - } - - return { exec, inject: source[INJECT_KEY] ?? source.inject } - } - /** * Binds class. Class constructor that is binded will be executed with the "new" call when resolved. Number of executions * depends on the scope used. * * @param key - key to resolve binded value {@link BindKey} - * @param options - bind options for the class {@link ClassOptions} + * @param options - bind options for the class {@link BindOptions} */ - bindClass( - key: BindKey, - value: T, - options?: Omit, "type">, - ): this { + bindClass( + key: K, + value: K extends Token ? ClassValueFor : ClassValue, + options?: BindOptions, + ): this + bindClass(key: BindKey, value: ClassValue, options?: BindOptions): this { const { exec, inject } = this.parseValue(value) - const resolve = (...args: any[]) => new exec(...args) + const resolve = ((...args: any[]) => new exec(...args)) as ResolverFn resolve.inject = inject resolve.original = exec - this.add(key, { - ...options, - type: TYPE.CLASS, - scope: options?.scope || SCOPE.TRANSIENT, - value: resolve, - }) + this.add( + key, + { + type: TYPE.CLASS, + scope: options?.scope ?? SCOPE.TRANSIENT, + value: resolve, + }, + options?.replace ?? false, + ) return this } + protected parseValue any)>( + value: T | { value: T; inject: InjectionData }, + ): { exec: T; inject: ParsedInjectionData[] | undefined } { + const exec = typeof value === "function" ? value : value.value + // injections come either from `registerInjections` (symbol keyed) or from + // an `inject` property on the class/factory itself, or on the wrapper + const source = value as { + [INJECT_KEY]?: InjectionData + inject?: InjectionData + } + + const raw = source[INJECT_KEY] ?? source.inject + + return { + exec, + // injection metadata cannot change once bound, so parse it here rather + // than on every single resolve + inject: raw === undefined ? undefined : raw.map(parseInjectionData), + } + } + /** * Resolve value that has previously been binded. * * @typeParam T - value that is going to be resolved * @param key - key to search for {@link BindKey} + * @throws {PumpitError} If the key is not bound. */ - resolve(key: BindKey): T { - const ctx: RequestCtx = this.currentCtx || { - singletonCache: this.singletonCache, - transientCache: new Map(), - requestCache: new Map(), - requestedKeys: new Map(), - postConstruct: [], - } + resolve(key: Token): T + resolve(key: T): InstanceType + resolve(key: BindKey): T + resolve(key: BindKey): any { + return this.runResolve(key, NO_OPTIONS) + } - const result = this._resolve(key, {}, ctx) + /** + * Same as {@link PumpIt.resolve | PumpIt.resolve()}, except that an unbound + * key resolves to `undefined` instead of throwing. + * + * @param key - key to search for {@link BindKey} + */ + tryResolve(key: Token): T | undefined + tryResolve(key: T): InstanceType | undefined + tryResolve(key: BindKey): T | undefined + tryResolve(key: BindKey): any { + return this.runResolve(key, OPTIONAL_OPTIONS) + } - // Execute postConstruct functions - for (const value of ctx.postConstruct) { - value.postConstruct() + protected runResolve(key: BindKey, options: InjectionOptions): any { + const ctx: RequestCtx = { + requestCache: undefined, + stack: undefined, + postConstruct: undefined, } - this.currentCtx = null + const result = this._resolve(key, options, ctx) + + const postConstruct = ctx.postConstruct + if (postConstruct !== undefined) { + for (const value of postConstruct) { + value.postConstruct() + } + } return result } @@ -282,9 +389,22 @@ export class PumpIt { /** * Sets the parent PumpIt instance. * - * @param parent - The parent PumpIt instance to be set. + * @param parent - The parent PumpIt instance to be set, or `undefined` to detach. + * @throws {PumpitError} If the parent would introduce a cycle in the hierarchy. */ - setParent(parent: PumpIt) { + setParent(parent: PumpIt | undefined) { + // without this a cycle turns every lookup into an infinite recursion + let ancestor = parent + while (ancestor !== undefined) { + if (ancestor === this) { + throw new PumpitError( + "Parent would create a cycle in the container hierarchy", + ERROR_CODE.PARENT_CYCLE, + ) + } + ancestor = ancestor.parent + } + this.parent = parent } @@ -295,209 +415,213 @@ export class PumpIt { return this.parent } - protected getInjectable( - key: BindKey, - ): { value: PoolData; fromParent: boolean } | undefined { + protected getInjectable(key: BindKey): PoolData | undefined { const value = this.pool.get(key) - if (value) return { value, fromParent: false } - - const parentValue = this.parent?.getInjectable(key) - if (parentValue) { - return { - value: parentValue.value, - fromParent: true, - } + if (value !== undefined) { + return value } - return undefined + return this.parent?.getInjectable(key) } protected _resolve( key: BindKey, - options: { optional?: boolean }, + options: InjectionOptions, ctx: RequestCtx, ): any { const data = this.getInjectable(key) - if (options?.optional && !data) { - return undefined - } + if (data === undefined) { + if (options.optional) { + return undefined + } - if (!data) { - throw new Error(`Key: ${keyToString(key)} not found`) + throw new PumpitError( + `Key: ${keyToString(key)} not found`, + ERROR_CODE.KEY_NOT_FOUND, + ) } - const poolData = data.value - - if (poolData.type === TYPE.VALUE) { + if (data.type === TYPE.VALUE) { // resolve immediately - value type has no dependencies - return poolData.value + return data.value } - // narrowed to a class or factory binding, so `value` is the resolver - const { value, scope } = poolData - const keySeen = ctx.requestedKeys.get(key) - - //if key has been seen - if (keySeen) { - //check if it is constructed - if (!keySeen.constructed) { - //throw circular reference error - const previous = Array.from(ctx.requestedKeys.entries()).pop() - const path = previous - ? `[ ${String(previous[0])}: ${previous[1].value.name} ]` - : "" - - throw new Error( - `Circular reference detected: ${path} -> [ ${keyToString( - key, - )}: ${value} ]`, - ) - } - } else { - ctx.requestedKeys.set(key, { constructed: false, value }) - } - - const fn = () => this.create(key, poolData, ctx) - - return this.run(scope, key, fn, ctx) + return this.run(data.scope, key, data, ctx) } - protected resolveDeps(deps: Injection[], ctx: RequestCtx): any[] { - const finalDeps = [] - for (const dep of deps) { - const { key, options } = parseInjectionData(dep) - let doneDep = ctx.singletonCache.get(key) - if (doneDep === undefined) { - doneDep = this._resolve(key, { ...options }, ctx) - } - finalDeps.push(doneDep) - } - - return finalDeps - } - - protected create( + protected run( + scope: AvailableScopes, key: BindKey, data: FactoryPoolData | ClassPoolData, ctx: RequestCtx, ) { - const { value, type } = data - const injectionData = value.inject - let resolvedDeps: any[] = [] + if (scope === SCOPE.SINGLETON || scope === SCOPE.CONTAINER_SINGLETON) { + const parent = this.parent + // a plain singleton belongs to the container that owns the key, so hand + // the resolve over to it - but keep the same request context, otherwise + // request scope, post construct ordering and circular detection would all + // restart at the boundary + if ( + scope === SCOPE.SINGLETON && + parent !== undefined && + !this.pool.has(key) + ) { + return parent._resolve(key, NO_OPTIONS, ctx) + } - if (injectionData) { - resolvedDeps = this.resolveDeps(injectionData, ctx) - } - const result = value(...resolvedDeps) + const cached = this.singletonCache.get(key) + if (cached !== undefined) { + return cached === UNDEFINED_RESULT ? undefined : cached + } + + const result = this.create(key, data, ctx) + this.singletonCache.set( + key, + result === undefined ? UNDEFINED_RESULT : result, + ) - const requested = ctx.requestedKeys.get(key) - if (requested) { - requested.constructed = true + return result } - if (type === TYPE.CLASS && "postConstruct" in result) { - ctx.postConstruct.push(result) + if (scope === SCOPE.REQUEST) { + const cache = ctx.requestCache + if (cache !== undefined) { + const cached = cache.get(key) + if (cached !== undefined) { + return cached === UNDEFINED_RESULT ? undefined : cached + } + } + + const result = this.create(key, data, ctx) + // re-read, creating the value may have populated the cache + let target = ctx.requestCache + if (target === undefined) { + target = new Map() + ctx.requestCache = target + } + target.set(key, result === undefined ? UNDEFINED_RESULT : result) + + return result } - return result + //transient scope + return this.create(key, data, ctx) } - protected run( - scope: AvailableScopes, + protected create( key: BindKey, - fn: (...args: any[]) => any, + data: FactoryPoolData | ClassPoolData, ctx: RequestCtx, ) { - if (scope === SCOPE.SINGLETON || scope === SCOPE.CONTAINER_SINGLETON) { - //if singleton and key is on the parent resolve the key via parent - if (!this.pool.has(key) && scope === SCOPE.SINGLETON) { - return this.parent?.resolve(key) + const { value, type } = data + const injectionData = value.inject + + let result: any + if (injectionData !== undefined && injectionData.length > 0) { + // only bindings with dependencies can take part in a cycle, so the stack + // is only touched here + let stack = ctx.stack + if (stack === undefined) { + stack = [] + ctx.stack = stack } - const cachedValue = ctx.singletonCache.get(key) - if (cachedValue !== undefined) { - return cachedValue === UNDEFINED_RESULT ? undefined : cachedValue + if (stack.includes(key)) { + throw this.circularError(stack, key) } - let result = fn() - if (result === undefined) { - result = UNDEFINED_RESULT + stack.push(key) + try { + result = value(...this.resolveDeps(injectionData, ctx)) + } finally { + stack.pop() } - this.singletonCache.set(key, result) - - return result + } else { + result = value(...NO_DEPS) } - if (SCOPE.REQUEST === scope) { - const cachedValue = ctx.requestCache.get(key) - if (cachedValue !== undefined) { - return cachedValue === UNDEFINED_RESULT ? undefined : cachedValue + if (type === TYPE.CLASS && "postConstruct" in result) { + let hooks = ctx.postConstruct + if (hooks === undefined) { + hooks = [] + ctx.postConstruct = hooks } - let result = fn() + hooks.push(result) + } - if (result === undefined) { - result = UNDEFINED_RESULT - } - ctx.requestCache.set(key, result) + return result + } - return result + protected resolveDeps(deps: ParsedInjectionData[], ctx: RequestCtx): any[] { + const finalDeps = [] + for (const dep of deps) { + finalDeps.push(this._resolve(dep.key, dep.options, ctx)) } - //transient scope - const result = fn() - - //transient cache is only used for proxies - ctx.transientCache.set(key, result) + return finalDeps + } - return result + protected circularError(stack: BindKey[], key: BindKey): PumpitError { + const path = [...stack, key] + .map((pathKey) => { + const data = this.getInjectable(pathKey) + const name = + data !== undefined && data.type !== TYPE.VALUE + ? data.value.original.name + : undefined + + return name + ? `[ ${keyToString(pathKey)}: ${name} ]` + : `[ ${keyToString(pathKey)} ]` + }) + .join(" -> ") + + return new PumpitError( + `Circular reference detected: ${path}`, + ERROR_CODE.CIRCULAR_REFERENCE, + ) } - protected _validate(safe = false): ValidationResult | undefined { - const seen = new Set() + protected _validate(safe: boolean): ValidationResult { const wantedBy = new Map() for (const [bindKey, data] of this.pool.entries()) { - if (seen.has(bindKey)) { - return + const toInject = data.type === TYPE.VALUE ? undefined : data.value.inject + if (toInject === undefined) { + continue } - seen.add(bindKey) + for (const dep of toInject) { + // a missing optional dependency resolves to undefined by design + if (dep.options.optional || this.has(dep.key)) { + continue + } - const toInject = data.type === TYPE.VALUE ? undefined : data.value.inject - if (toInject) { - for (const dep of toInject) { - const { key: depKey } = parseInjectionData(dep) - - seen.add(depKey) - if (!this.has(depKey)) { - let wanted = wantedBy.get(depKey) - if (!wanted) { - wanted = [] - wantedBy.set(depKey, wanted) - } - wanted.push(bindKey) - } + let wanted = wantedBy.get(dep.key) + if (wanted === undefined) { + wanted = [] + wantedBy.set(dep.key, wanted) + } + if (!wanted.includes(bindKey)) { + wanted.push(bindKey) } } } const errors: ValidationError[] = [] - for (const [key, value] of wantedBy.entries()) { - if (value.length > 0) { - errors.push({ - key, - wantedBy: value, - }) - } + for (const [key, wanted] of wantedBy.entries()) { + errors.push({ + key, + wantedBy: wanted, + }) } - const valid = errors.length === 0 - if (!safe && !valid) { - throw new PumpitError("Validation", errors) + if (!safe && errors.length > 0) { + throw new PumpitValidationError(errors) } return { - valid, + valid: errors.length === 0, errors, } } @@ -508,6 +632,7 @@ export class PumpIt { * If the validation fails it will throw an error. * It will not instantiate the classes or execute the functions. * + * @throws {PumpitValidationError} If any dependency cannot be resolved. */ validate(): void { this._validate(false) @@ -518,7 +643,7 @@ export class PumpIt { * It will check if all the dependencies that are required by other bindings are present in the container. * It will not instantiate the classes or execute the functions. */ - validateSafe(): ValidationResult | undefined { + validateSafe(): ValidationResult { return this._validate(true) } @@ -536,3 +661,18 @@ export class PumpIt { return this.locked } } + +// `using container = new PumpIt()` unbinds everything on scope exit. The method +// is attached to the prototype below rather than declared in the class body, +// because `Symbol.dispose` does not exist on every supported runtime. +export interface PumpIt extends Disposable {} + +if (DISPOSE_SYMBOL !== undefined) { + Object.defineProperty(PumpIt.prototype, DISPOSE_SYMBOL, { + value: function dispose(this: PumpIt) { + this.unbindAll() + }, + writable: true, + configurable: true, + }) +} diff --git a/src/types-internal.ts b/src/types-internal.ts index 6f73f52..1044fc2 100644 --- a/src/types-internal.ts +++ b/src/types-internal.ts @@ -6,11 +6,7 @@ import type { FactoryFn, FactoryOptions, } from "./types" -import type { InjectionData } from "./utils" - -type InternalResolveCtx = { - data?: Record -} +import type { ParsedInjectionData } from "./utils" /** * What the pool actually holds for class and factory bindings: the bound value @@ -18,8 +14,11 @@ type InternalResolveCtx = { * with the parsed injection data attached. */ export type ResolverFn = ((...args: any[]) => any) & { - /** Dependencies to resolve before invoking the resolver */ - inject: InjectionData | undefined + /** + * Dependencies to resolve before invoking the resolver. Parsed once when the + * value is bound, since injection metadata cannot change afterwards. + */ + inject: ParsedInjectionData[] | undefined /** The unwrapped class or factory that was bound */ original: ClassConstructor | FactoryFn } @@ -44,14 +43,20 @@ export type FactoryPoolData = FactoryOptions & { export type PoolData = ValuePoolData | ClassPoolData | FactoryPoolData +/** + * State that lives for exactly one {@link PumpIt.resolve | PumpIt.resolve()} + * call, and is shared with parent containers when resolution crosses into one. + * Every field is allocated on first use, since most resolve calls need none of + * them. + */ export type RequestCtx = { - singletonCache: Map - requestCache: Map - transientCache: Map - // only class and factory bindings are tracked here, values resolve immediately - requestedKeys: Map - ctx?: InternalResolveCtx - postConstruct: PostConstruct[] + /** Values already built for {@link SCOPE.REQUEST} bindings */ + requestCache: Map | undefined + /** Keys currently being constructed, deepest last. Doubles as the path + * reported when a circular reference is detected. */ + stack: BindKey[] | undefined + /** Instances waiting for their `postConstruct` hook to run */ + postConstruct: PostConstruct[] | undefined } /** An instance that wants a callback once the whole resolve call completes*/ diff --git a/src/types.ts b/src/types.ts index d10c5b2..d252c1b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -38,6 +38,37 @@ export type ClassValue = inject: InjectionData } +/** A class binding constrained to produce `T`, used with typed tokens*/ +export type ClassValueFor = + | ((new (...args: any[]) => T) & WithInjectProp) + | { + value: new (...args: any[]) => T + inject: InjectionData + } + +/** A factory binding constrained to produce `T`, used with typed tokens*/ +export type FactoryValueFor = + | (((...args: any[]) => T) & WithInjectProp) + | { + value: (...args: any[]) => T + inject: InjectionData + } + +/** Options accepted when binding a class or a factory*/ +export type BindOptions = { + /** Scope that is going to be used {@link AvailableScopes}*/ + scope?: AvailableScopes + /** + * Replace an existing binding under the same key instead of throwing. + * The previous binding is unbound first, which disposes its cached + * singleton (if any). + */ + replace?: boolean +} + +/** Options accepted when binding a value*/ +export type ValueBindOptions = Pick + /** Class bind options*/ export type ClassOptions = { /** Class constant type {@link AvailableTypes} */ diff --git a/src/utils.ts b/src/utils.ts index d87c9e0..59aeaf0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -32,6 +32,26 @@ export type InjectionData = Injection[] /** Anything that can carry injection metadata*/ export type Injectable = ClassConstructor | FactoryFn +// phantom brand: only ever exists in the type system, never at runtime +declare const TOKEN_TYPE: unique symbol + +/** + * A bind key that remembers what it resolves to, so + * {@link PumpIt.resolve | PumpIt.resolve()} can infer the type instead of + * being told. Created with {@link token | token()}. + */ +export type Token = symbol & { readonly [TOKEN_TYPE]: T } + +/** + * Creates a typed injection token. + * + * @typeParam T - the type that will be bound and resolved under this token + * @param description - optional symbol description, shows up in error messages + */ +export function token(description?: string): Token { + return Symbol(description) as Token +} + /** * get dependency by key * @param key - dependency {@link BindKey} From 3e525e03056d64287298096a2283ad1ddf890134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Vlatkovic=CC=81?= <390700+ivandotv@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:26:50 +0200 Subject: [PATCH 2/3] fix: let container disposal bypass the lock, pin resolution semantics `using container = new PumpIt()` threw "Container is locked" at scope exit when the container had been locked. Throwing out of a disposal is harmful in JS: an error raised by the block body gets wrapped in a SuppressedError and the original is masked. The lock guards callers editing bindings, while `using` owns the whole lifetime, so disposal now removes bindings directly. `unbindAll` still refuses a locked container. Adds tests for behaviours nothing guarded, all of which already passed: - a parent owned singleton cannot reach a child only dependency, and siblings share one instance - a throwing constructor, a missing dependency and a caught circular reference all leave the container usable and retryable - a throwing postConstruct propagates, skips remaining hooks, and leaves the singleton cached - Symbol.dispose on a factory result - replace is scoped to the container it is called on - typed tokens as optional dependencies, and resolved from a child Co-Authored-By: Claude Opus 5 --- .changeset/quiet-moons-invite.md | 2 +- README.md | 7 +- src/__tests__/container-api.test.ts | 72 ++++++++++++++ src/__tests__/regressions.test.ts | 141 ++++++++++++++++++++++++++++ src/__tests__/token.test.ts | 41 +++++++- src/pumpit.ts | 50 ++++++---- 6 files changed, 293 insertions(+), 20 deletions(-) diff --git a/.changeset/quiet-moons-invite.md b/.changeset/quiet-moons-invite.md index fdea5dd..0bb0036 100644 --- a/.changeset/quiet-moons-invite.md +++ b/.changeset/quiet-moons-invite.md @@ -10,6 +10,6 @@ Add `getKeys`, which lists the keys bound on the container, optionally including Add a `replace` bind option, so a key can be rebound without unbinding it first. The previous binding is unbound, which disposes its cached singleton. -Support `Symbol.dispose` on resolved singletons, preferred over a `dispose` method. The container itself is now disposable, so `using container = new PumpIt()` unbinds everything on scope exit. +Support `Symbol.dispose` on resolved singletons, preferred over a `dispose` method. The container itself is now disposable, so `using container = new PumpIt()` unbinds everything on scope exit. Disposal ignores the lock, since throwing out of a disposal would mask whatever the enclosing block was doing. `unbindAll` still refuses a locked container. Every error thrown by the container is now a `PumpitError` carrying a machine readable `code`, exported as `ERROR_CODE`. diff --git a/README.md b/README.md index 5e67197..2d474f4 100644 --- a/README.md +++ b/README.md @@ -664,9 +664,14 @@ calls `unbindAll()`, which disposes every cached singleton. container.bindClass(TestA, TestA, { scope: SCOPE.SINGLETON }) container.resolve(TestA) -} // container.unbindAll() runs here, TestA instance is disposed +} // every binding is removed here, TestA instance is disposed ``` +> Disposal ignores [the lock](#locking-the-container). Locking guards against +> *callers* editing bindings, while `using` owns the container's whole lifetime - +> and throwing out of a disposal would mask whatever the enclosing block was +> doing. `unbindAll()` still refuses a locked container. + ### Locking the container If the container is `locked` that particular container can't accept new bindings or unbind the values already in the container. diff --git a/src/__tests__/container-api.test.ts b/src/__tests__/container-api.test.ts index 86f4e78..915dc64 100644 --- a/src/__tests__/container-api.test.ts +++ b/src/__tests__/container-api.test.ts @@ -172,6 +172,30 @@ describe("Container API", () => { ) }) + test("replacing on a child leaves the parent binding alone", () => { + const parent = new PumpIt() + const child = parent.child() + + parent.bindValue("key", "parent") + child.bindValue("key", "child", { replace: true }) + + expect(child.resolve("key")).toBe("child") + expect(parent.resolve("key")).toBe("parent") + }) + + test("replace does not reach a key that only the parent has", () => { + const parent = new PumpIt() + const child = parent.child() + + parent.bindValue("key", "parent") + // the child has nothing to replace, so this just shadows + child.bindValue("key", "child", { replace: true }) + + expect(parent.getKeys()).toEqual(["key"]) + expect(child.getKeys()).toEqual(["key"]) + expect(parent.resolve("key")).toBe("parent") + }) + test("replace works on a key that is not bound yet", () => { const pumpIt = new PumpIt() @@ -240,6 +264,54 @@ describe("Container API", () => { expect(pumpIt.getKeys()).toEqual([]) }) + test("Symbol.dispose is honoured on a factory result", () => { + const pumpIt = new PumpIt() + const symbolDispose = vi.fn() + + pumpIt.bindFactory( + "key", + () => ({ + [Symbol.dispose]: symbolDispose, + }), + { scope: SCOPE.SINGLETON }, + ) + pumpIt.resolve("key") + pumpIt.unbind("key") + + expect(symbolDispose).toHaveBeenCalledTimes(1) + }) + + test("disposing a locked container still cleans up", () => { + const pumpIt = new PumpIt() + const disposeCall = vi.fn() + + class TestA { + dispose() { + disposeCall() + } + } + + pumpIt.bindClass("key", TestA, { scope: SCOPE.SINGLETON }) + pumpIt.resolve("key") + pumpIt.lock() + + // the lock stops callers from editing bindings, it does not stop the + // container from being torn down - throwing here would mask whatever the + // enclosing `using` block was doing + expect(() => pumpIt[Symbol.dispose]()).not.toThrow() + expect(disposeCall).toHaveBeenCalledTimes(1) + expect(pumpIt.getKeys()).toEqual([]) + }) + + test("unbindAll still refuses a locked container", () => { + const pumpIt = new PumpIt() + pumpIt.bindValue("key", 1) + pumpIt.lock() + + expect(() => pumpIt.unbindAll()).toThrow("Container is locked") + expect(pumpIt.getKeys()).toEqual(["key"]) + }) + test("`using` unbinds everything on scope exit", () => { const disposeCall = vi.fn() diff --git a/src/__tests__/regressions.test.ts b/src/__tests__/regressions.test.ts index 6dcb128..b178020 100644 --- a/src/__tests__/regressions.test.ts +++ b/src/__tests__/regressions.test.ts @@ -249,6 +249,147 @@ describe("Regressions", () => { expect(child.resolve("a")).toBe(parent.resolve("a")) }) + + test("a parent owned singleton cannot reach a child only dependency", () => { + const parent = new PumpIt() + const child = parent.child() + + class OnParent { + static inject = ["child_only"] + + constructor(public dep: string) {} + } + + parent.bindClass("on_parent", OnParent, { scope: SCOPE.SINGLETON }) + child.bindValue("child_only", "from the child") + + // the singleton belongs to the parent and is shared by every sibling, so + // it must not capture something only this child can see + expect(() => child.resolve("on_parent")).toThrow( + "Key: child_only not found", + ) + }) + + test("siblings share one parent owned singleton", () => { + const parent = new PumpIt() + const childOne = parent.child() + const childTwo = parent.child() + + class TestA { + static count = 0 + + constructor() { + TestA.count++ + } + } + parent.bindClass("a", TestA, { scope: SCOPE.SINGLETON }) + + expect(childOne.resolve("a")).toBe(childTwo.resolve("a")) + expect(TestA.count).toBe(1) + }) + }) + + describe("a failed resolve leaves the container usable", () => { + test("a throwing constructor does not cache a broken singleton", () => { + const pumpIt = new PumpIt() + let shouldThrow = true + + class TestA { + constructor() { + if (shouldThrow) { + throw new Error("constructor failed") + } + } + } + pumpIt.bindClass("a", TestA, { scope: SCOPE.SINGLETON }) + + expect(() => pumpIt.resolve("a")).toThrow("constructor failed") + + shouldThrow = false + + expect(pumpIt.resolve("a")).toBeInstanceOf(TestA) + }) + + test("a missing dependency deeper in the graph can be fixed and retried", () => { + const pumpIt = new PumpIt() + + class Dep {} + class Root { + static inject = ["dep"] + + constructor(public dep: Dep) {} + } + pumpIt.bindClass("root", Root, { scope: SCOPE.SINGLETON }) + + expect(() => pumpIt.resolve("root")).toThrow("Key: dep not found") + + pumpIt.bindClass("dep", Dep) + + expect(pumpIt.resolve("root").dep).toBeInstanceOf(Dep) + }) + + test("a caught circular reference does not leave the container dirty", () => { + const pumpIt = new PumpIt() + + class TestA { + static inject = ["b"] + } + class TestB { + static inject = ["a"] + } + class Leaf {} + + pumpIt.bindClass("a", TestA).bindClass("b", TestB).bindClass("leaf", Leaf) + + expect(() => pumpIt.resolve("a")).toThrow("Circular reference detected") + expect(pumpIt.resolve("leaf")).toBeInstanceOf(Leaf) + }) + }) + + describe("post construct failures", () => { + test("the error propagates and the remaining hooks are skipped", () => { + const pumpIt = new PumpIt() + const ran: string[] = [] + + class Dep { + postConstruct() { + ran.push("dep") + throw new Error("dep hook failed") + } + } + class Root { + static inject = ["dep"] + + postConstruct() { + ran.push("root") + } + } + pumpIt.bindClass("dep", Dep).bindClass("root", Root) + + expect(() => pumpIt.resolve("root")).toThrow("dep hook failed") + // hooks run bottom up, so the root hook never gets its turn + expect(ran).toEqual(["dep"]) + }) + + test("a singleton whose hook threw is still cached", () => { + const pumpIt = new PumpIt() + let hookCalls = 0 + + class TestA { + postConstruct() { + hookCalls++ + throw new Error("hook failed") + } + } + pumpIt.bindClass("a", TestA, { scope: SCOPE.SINGLETON }) + + expect(() => pumpIt.resolve("a")).toThrow("hook failed") + + // the instance was built and cached before the hook ran, so the second + // resolve hands back the same instance and does not retry the hook + expect(() => pumpIt.resolve("a")).not.toThrow() + expect(hookCalls).toBe(1) + }) }) describe("circular reference reporting", () => { diff --git a/src/__tests__/token.test.ts b/src/__tests__/token.test.ts index 19ecb32..f56595e 100644 --- a/src/__tests__/token.test.ts +++ b/src/__tests__/token.test.ts @@ -1,6 +1,6 @@ import { describe, expect, expectTypeOf, test } from "vitest" import { PumpIt } from "../pumpit" -import { token } from "../utils" +import { get, token } from "../utils" type Config = { url: string; retries: number } @@ -76,6 +76,45 @@ describe("Typed tokens", () => { expect(pumpIt.resolve(Service).config).toBe(config) }) + test("a token can be an optional dependency", () => { + const pumpIt = new PumpIt() + const missing = token("missing") + + class Service { + static inject = [get(missing, { optional: true })] + + constructor(public config?: Config) {} + } + + pumpIt.bindClass(Service, Service) + + expect(pumpIt.resolve(Service).config).toBeUndefined() + }) + + test("an optional token is not reported by validation", () => { + const pumpIt = new PumpIt() + const missing = token("missing") + + class Service { + static inject = [get(missing, { optional: true })] + } + + pumpIt.bindClass(Service, Service) + + expect(pumpIt.validateSafe()).toEqual({ valid: true, errors: [] }) + }) + + test("a token bound on the parent resolves from a child", () => { + const parent = new PumpIt() + const child = parent.child() + const configToken = token("config") + const config = { url: "https://example.com", retries: 3 } + + parent.bindValue(configToken, config) + + expect(child.resolve(configToken)).toBe(config) + }) + // `expectTypeOf` still evaluates its argument, so everything asserted on here // is bound first and the assertions double as runtime checks describe("types", () => { diff --git a/src/pumpit.ts b/src/pumpit.ts index 90e31f8..7517050 100644 --- a/src/pumpit.ts +++ b/src/pumpit.ts @@ -103,12 +103,37 @@ export class PumpIt { ERROR_CODE.KEY_ALREADY_EXISTS, ) } - this.unbind(key) + // the lock was already checked above + this.remove(key, true) } this.pool.set(key, info) } + /** Removes a key without consulting the lock */ + protected remove(key: BindKey, dispose: boolean): void { + // `has` rather than a truthiness check, so the intent does not depend on + // what the singleton happens to resolve to + const hasSingleton = this.singletonCache.has(key) + const singleton = this.singletonCache.get(key) + + this.pool.delete(key) + this.singletonCache.delete(key) + + if (hasSingleton && dispose) { + this.callDispose(singleton) + } + } + + /** Removes every key without consulting the lock */ + protected removeAll(dispose: boolean): void { + for (const key of this.pool.keys()) { + this.remove(key, dispose) + } + this.pool.clear() + this.singletonCache.clear() + } + /** * Unbinds a dependency from the container. * @param key - The key to unbind. @@ -127,16 +152,7 @@ export class PumpIt { ) } - // `has` rather than a truthiness check, so falsy singletons are disposed too - const hasSingleton = this.singletonCache.has(key) - const singleton = this.singletonCache.get(key) - - this.pool.delete(key) - this.singletonCache.delete(key) - - if (hasSingleton && dispose) { - this.callDispose(singleton) - } + this.remove(key, dispose) } /** @@ -149,11 +165,7 @@ export class PumpIt { throw new PumpitError("Container is locked", ERROR_CODE.CONTAINER_LOCKED) } - for (const key of this.pool.keys()) { - this.unbind(key, callDispose) - } - this.pool.clear() - this.singletonCache.clear() + this.removeAll(callDispose) } protected callDispose(value: unknown): void { @@ -669,8 +681,12 @@ export interface PumpIt extends Disposable {} if (DISPOSE_SYMBOL !== undefined) { Object.defineProperty(PumpIt.prototype, DISPOSE_SYMBOL, { + // deliberately not `unbindAll`: the lock stops callers from editing + // bindings, but `using` owns the whole lifetime, and throwing out of a + // disposal would mask whatever the enclosing block was doing value: function dispose(this: PumpIt) { - this.unbindAll() + // biome-ignore lint/complexity/useLiteralKeys: reaching the protected member from the prototype install + this["removeAll"](true) }, writable: true, configurable: true, From b289dd3fcda369695791cb70a92aa81b25ab7b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Vlatkovic=CC=81?= <390700+ivandotv@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:49:50 +0200 Subject: [PATCH 3/3] ci: run pnpm 11, surface store path failures CI pinned pnpm 9, but `pnpm-workspace.yaml` (added in 9d792d0) only sets `allowBuilds` and has no `packages` field. pnpm 9 requires that field and bailed with "packages field missing or empty", so `pnpm store path` wrote nothing to STORE_PATH and actions/cache failed with "Input required and not supplied: path". `pnpm install` would have failed for the same reason. `allowBuilds` is a pnpm 11 setting, so CI now runs 11 to match local. The store path step also masked the error: `echo "X=$(cmd)"` exits 0 even when the substitution fails, so the step went green while exporting an empty value. Assigning first lets `set -e` fail it at the real cause. Co-Authored-By: Claude Opus 5 --- .github/workflows/CI.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 5a019af..3097b4c 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -27,13 +27,14 @@ jobs: - name: Install pnpm uses: pnpm/action-setup@v4.4.0 with: - version: 9 + version: 11 run_install: false - name: Get pnpm store directory shell: bash run: | - echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + STORE_PATH="$(pnpm store path --silent)" + echo "STORE_PATH=$STORE_PATH" >> $GITHUB_ENV - name: Setup pnpm cache uses: actions/cache@v4