From 99734481c584847101ee92d8c5f81df74bb8688c Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 10:36:39 +1000 Subject: [PATCH 01/13] =?UTF-8?q?Make=20Container.provides()=20chain=20O(N?= =?UTF-8?q?)=20instead=20of=20O(N=C2=B2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long .provides()..provides() chains caused container assembly ANRs on low-end devices because each call shallow-copied the entire factories map and the constructor walked every factory to rebind thisArg. With N services that's O(N²) work. This patch replaces the spread with prototype-chained extension (Object.create) and switches memoize to call-site `this`, eliminating the per-step rebinding pass. 8000-deep chain: 10286 ms -> 496 ms (~21x). All 87 tests pass with 100% coverage. See benchmarks/INVESTIGATION.md for the full write-up and benchmarks/provides-chain.ts (npm run bench) to reproduce. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 2 + benchmarks/INVESTIGATION.md | 132 +++++++++++++++++++++++++++++++ benchmarks/provides-chain.ts | 60 ++++++++++++++ package.json | 1 + src/Container.ts | 85 ++++++++++++-------- src/PartialContainer.ts | 13 ++- src/__tests__/Container.spec.ts | 57 +++++++++++++ src/__tests__/Injectable.spec.ts | 32 +++++++- src/entries.ts | 4 - src/memoize.ts | 10 +-- 10 files changed, 348 insertions(+), 48 deletions(-) create mode 100644 benchmarks/INVESTIGATION.md create mode 100644 benchmarks/provides-chain.ts diff --git a/.gitignore b/.gitignore index 651d780..ecf44bf 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ dist docs node_modules .DS_Store +.tmp/ +.idea/ diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md new file mode 100644 index 0000000..88c4a15 --- /dev/null +++ b/benchmarks/INVESTIGATION.md @@ -0,0 +1,132 @@ +# `provides()` Chain Performance Investigation + +## TL;DR + +Long `Container.provides().provides()...provides()` chains used to construct in O(N²) time. With a few hundred services this caused ANRs on low-end Android devices during container assembly. The fix makes chain construction O(N) per step (~O(N) total) by: + +1. Replacing the shallow spread of `this.factories` with prototype-chained extension (`Object.create`). +2. Removing the per-step `for...in` loop in the `Container` constructor that rebinds `thisArg` on every already-memoized factory. + +End-to-end speedup: **20× faster for 8000-deep chains** (10,286 ms → 496 ms), **9× faster at 800**, **7.5× at 400**. All 87 tests pass with 100% line/branch/function coverage. + +## How to reproduce + +```bash +npm run bench +``` + +This builds chains of 50 → 8000 services and reports `ms/build`. There's also a get-pass over an 800-deep chain to track lookup cost. + +## Numbers + +| N services | before (ms) | after (ms) | speedup | +|------------|-------------|------------|---------| +| 50 | 0.05 | 0.03 | ~1.7× | +| 100 | 0.19 | 0.08 | 2.4× | +| 200 | 0.81 | 0.28 | 2.9× | +| 400 | 4.96 | 0.69 | 7.2× | +| 800 | 32.08 | 4.27 | 7.5× | +| 1600 | 309.44 | 18.32 | 16.9× | +| 3200 | 1518.40 | 75.28 | 20.2× | +| 8000 | 10285.63 | 495.87 | 20.7× | + +Lookup cost on an 800-deep chain went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see *Trade-offs* below. + +Measured on Apple silicon, Node 20. Numbers will differ on other devices, but the relative shapes are what matter. + +## What was triggering the issue + +The report came from a service team observing ANRs on container assembly for low-end devices, with stack traces converging on `Container.provides`. Inspecting the old `providesService`: + +```ts +// old +const factories = { ...this.factories, [token]: factory }; +return new Container(factories); +``` + +Plus the `Container` constructor: + +```ts +// old +constructor(factories: MaybeMemoizedFactories) { + const memoizedFactories = {} as Factories; + for (const k in factories) { + const fn = factories[k]; + if (isMemoized(fn)) { + memoizedFactories[k] = fn; + fn.thisArg = this; // rebind every memoized factory to the new container + } else { + memoizedFactories[k] = memoize(this, fn); + } + } + this.factories = memoizedFactories; +} +``` + +Per `provides()` call, both the spread and the constructor's `for...in` walked **every** factory currently in the container. That's O(N) per step, O(N²) for N chained calls. + +The constructor's `thisArg` rebinding existed so that overrides could flow: each memoized factory captured a `thisArg` reference, and when a new container was built (potentially overriding services), all existing factories were re-pointed at the new container so they resolved dependencies through it. + +## Options considered + +1. **Drop the constructor's `thisArg` rebinding only.** Smaller diff. Removes one of the two O(N) operations per step but the spread remains. Estimated ~2–3× speedup — not enough to clear the ANR threshold at high service counts. +2. **Prototype-chained factories.** Replace `{ ...this.factories, [token]: factory }` with `Object.create(this.factories)` + assign. Per-step construction becomes O(1). Property access on the resulting container walks the prototype chain (O(depth) once per token, then memoized). +3. **Linked-list / parent-pointer container.** Each container holds `(parent, ownFactories)`. Most idiomatic, but a larger refactor and changes the public-ish `factories` field shape. +4. **Persistent immutable map (HAMT etc.).** Over-engineered for the scale (hundreds, not millions, of services). + +We went with **(1) + (2) together**. They compose: (1) lets us skip the constructor's per-step walk, (2) lets us skip the spread. Together they bring per-step cost to O(1) and the chain to O(N). + +## What changed + +- **`src/memoize.ts`** — dropped the `thisArg` field from `Memoized`. Memoized functions now use the call-site `this` (`memo = delegate.apply(this, args)`). +- **`src/Container.ts`**: + - Constructor: fast path that *shares* an already-memoized factories object (common internal case) and a slow path that memoizes own keys while preserving any prototype chain. + - `get()`: invokes the factory via `factory.call(this)` so dependencies resolve against the calling container. + - `provides()` (per-token and Container/PartialContainer merge): builds the new factories object via `Object.create(this.factories)` instead of a spread. + - `copy()`: same prototype-chain approach; only scoped tokens get freshly-memoized own properties. +- **`src/PartialContainer.ts`**: + - `getFactories()`: drops the now-unused `thisArg` argument to `memoize`. + - `provides(Container)`: walks the source container's factories with `for...in` (so prototype-chained factories come through) and pre-binds each wrapper to the source container via `factory.call(first)` — preserves the old "value pulled from source container" semantics for the bridge case. + +## Trade-offs + +### Lookup cost grew slightly + +`get(token)` now walks a prototype chain to find the factory. For an 800-deep chain that's ~1.2 µs per lookup on first access. After the factory is memoized once, the chain walk still happens to find the factory function, but the cached result short-circuits the body. + +In real applications a cold start typically touches a small handful of services, so the additional walk cost is negligible. If a workload ever does mass-access of services, a "flatten chain when depth exceeds N" optimization can be layered on later. + +### Subtle semantic shift: parent stays consistent after a fork + +Previously, building a child container `C` from a parent `B` mutated `B`'s factories to point their `thisArg` at `C`. As a result, `b.get('svc')` *after* the fork could resolve dependencies through `C` — meaning the parent was no longer a self-consistent snapshot of its own state. + +Now each container is a true snapshot: `b.get('svc')` resolves through `b`. This is captured by the new test `forking a child does not change the parent's view of its services`. + +This **is** a behavioral change. It's strictly more correct (and intuitive), and the existing test suite — which covers override-before-init and override-after-init for the *child* — continues to pass. + +### What is *not* fixed: sibling isolation + +Memoization is per-factory, not per-container. If you fork two children `C1` and `C2` from the same parent `B` and override the same token in each, the two siblings share the parent's factory for any service they didn't override. Whichever sibling resolves that service first sets the memoization, and the second sibling sees the cached value. + +This was equally broken in the old code (with a different failure mode: whichever container was *built* most recently won). Sibling isolation requires `copy(['token'])` to un-memoize and re-memoize per scope. + +### Public `factories` field + +`Container.factories` is `public` for type-emission reasons (see the inline comment in `Container.ts`). Direct invocation of a factory via `container.factories.svc()` used to rely on `thisArg`. After the change, only zero-dep factories work via direct invocation; factories with dependencies need `factory.call(container)` (or just use `container.get('svc')`, which is the supported API). + +The one existing test that touches `container.factories.svc()` uses a zero-dep service, so it still passes. No documentation or API change was needed. + +## Test coverage + +All paths exercised — `npm test` reports: + +``` +File | % Stmts | % Branch | % Funcs | % Lines +All files | 100 | 100 | 100 | 100 +``` + +with 87/87 tests passing. + +Two new tests lock in semantics that the patch introduces: +- `forking a child does not change the parent's view of its services` — parent stays consistent after a fork. +- (Plus four small tests to close pre-existing coverage gaps: constructor slow path with raw factories, `InjectableCompat`, and the two `ConcatInjectable` validation branches.) diff --git a/benchmarks/provides-chain.ts b/benchmarks/provides-chain.ts new file mode 100644 index 0000000..ef995a5 --- /dev/null +++ b/benchmarks/provides-chain.ts @@ -0,0 +1,60 @@ +/** + * Microbenchmark for `Container.provides()` chain construction and `Container.get()` lookups. + * + * Run with: + * npm run bench + * + * Designed to verify the per-step cost of `provides()` stays roughly linear in chain length + * (it was previously O(N²) — see docs/perf-provides-chain.md). Adjust `sizes` and `iters` + * if you want to exercise different regimes. + */ + +import { Container } from "../src/Container"; + +function buildChain(n: number): Container> { + let c: Container = new Container({}); + for (let i = 0; i < n; i++) { + c = c.provides(`svc${i}`, () => i); + } + return c; +} + +function timeMs(fn: () => void): number { + const start = process.hrtime.bigint(); + fn(); + const end = process.hrtime.bigint(); + return Number(end - start) / 1e6; +} + +function bench(n: number, iters: number): number { + buildChain(n); // warmup + return timeMs(() => { + for (let i = 0; i < iters; i++) buildChain(n); + }) / iters; +} + +const sizes = [50, 100, 200, 400, 800, 1600, 3200, 8000]; +// More iterations for the cheap cases so we get a stable signal; fewer for the expensive ones +// so the suite finishes in seconds rather than minutes. +const itersFor = (n: number): number => + n <= 200 ? 50 : n <= 400 ? 20 : n <= 800 ? 5 : n <= 3200 ? 3 : 2; + +console.log("Chain construction:"); +console.log("size\tms/build"); +for (const n of sizes) { + const ms = bench(n, itersFor(n)); + console.log(`${n}\t${ms.toFixed(2)}`); +} + +// Lookup-cost probe: services added later in the chain are shallow in the prototype chain, +// services added earlier are deep. A full pass exercises a range of depths. +const lookupChainSize = 800; +const lookupIters = 50; +const c = buildChain(lookupChainSize); +const lookupMs = + timeMs(() => { + for (let i = 0; i < lookupIters; i++) { + for (let k = 0; k < lookupChainSize; k++) c.get(`svc${k}`); + } + }) / lookupIters; +console.log(`\nFull get-pass on ${lookupChainSize}-deep chain: ${lookupMs.toFixed(2)} ms/iter`); diff --git a/package.json b/package.json index 0c95e3e..97c9855 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "format:fix": "npm run format:check -- --write || exit 0", "test": "jest", "test:watch": "jest --clearCache && jest --watch", + "bench": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/provides-chain.ts", "compile": "tsc -b ./tsconfig.cjs.json ./tsconfig.esm.json ./tsconfig.types.json", "docs": "typedoc src/index.ts", "build": "rm -rf dist && rm -rf docs && npm run compile && npm run docs" diff --git a/src/Container.ts b/src/Container.ts index 2e78b03..cfc59f6 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -172,19 +172,31 @@ export class Container { readonly factories: Readonly>; constructor(factories: MaybeMemoizedFactories) { - const memoizedFactories = {} as Factories; - for (const k in factories) { - const fn = factories[k]; - if (isMemoized(fn)) { - memoizedFactories[k] = fn; - // to allow overriding values in the container we replace the factory's reference to the container with the - // newly created one, this makes sure that overrides are taken into account when resolving the service's - // dependencies. - fn.thisArg = this; - } else { - memoizedFactories[k] = memoize(this, fn); + // Fast path: when every own factory is already memoized, share the input directly. + // Internal builders (`provides`, `copy`, etc.) prepare factories this way and chain + // them via `Object.create`, keeping per-step construction O(1). + const ownKeys = Object.keys(factories); + let allMemoized = true; + for (const k of ownKeys) { + if (!isMemoized((factories as any)[k])) { + allMemoized = false; + break; } } + if (allMemoized) { + this.factories = factories as unknown as Factories; + return; + } + // Slow path: memoize any non-memoized own factories, preserving any prototype chain + // so inherited memoized factories stay reachable. + const proto = Object.getPrototypeOf(factories); + const memoizedFactories = (proto && proto !== Object.prototype + ? Object.create(proto) + : ({} as Factories)) as Factories; + for (const k of ownKeys) { + const fn = (factories as any)[k]; + (memoizedFactories as any)[k] = isMemoized(fn) ? fn : memoize(fn); + } this.factories = memoizedFactories; } @@ -222,14 +234,17 @@ export class Container { * instances to the new Container. */ copy(scopedServices?: Tokens): Container { - const factories: MaybeMemoizedFactories = { ...this.factories }; - - // We "un-memoize" scoped Service InjectableFunctions so they will create a new copy of their Service when - // provided by the new Container – we re-memoize them so the new Container will itself only create one Service - // instance. - (scopedServices || []).forEach((token: keyof Services) => { - factories[token] = this.factories[token].delegate; - }); + if (!scopedServices || scopedServices.length === 0) { + // Share factories via prototype chain — the new container resolves to the same memoized + // instances as the original. + return new Container(Object.create(this.factories) as MaybeMemoizedFactories); + } + // Override scoped tokens with freshly-memoized copies of the original delegates so the new + // container produces independent service instances for those tokens. + const factories = Object.create(this.factories) as MaybeMemoizedFactories; + for (const token of scopedServices) { + factories[token] = memoize(this.factories[token].delegate); + } return new Container(factories); } @@ -262,7 +277,9 @@ export class Container { "definitely initialized before the call to Injectable." ); } - return factory(); + // Pass `this` so factories that depend on other services resolve them through the calling + // container — supporting overrides applied after the factory was registered. + return factory.call(this); } /** @@ -450,14 +467,16 @@ export class Container { } // Original single-arg forms if (first instanceof PartialContainer || first instanceof Container) { - const factories = first instanceof PartialContainer ? first.getFactories(this) : first.factories; - // Safety: `this.factories` and `factories` are both properly type checked, so merging them produces - // a Factories object with keys from both Services and AdditionalServices. The compiler is unable to - // infer that Factories & Factories == Factories, so the cast is required. - return new Container({ - ...this.factories, - ...factories, - } as unknown as MaybeMemoizedFactories>); + const incoming = first instanceof PartialContainer ? first.getFactories(this) : first.factories; + // Layer the incoming factories on top of this.factories via prototype chain — O(1) base + // plus O(K) for K incoming keys (instead of spreading every factory in `this`). + const factories = Object.create(this.factories) as MaybeMemoizedFactories>; + // `for...in` walks the prototype chain so chained factories from the incoming container + // come through. Newer keys (own properties) override older ones (inherited). + for (const k in incoming) { + (factories as any)[k] = (incoming as any)[k]; + } + return new Container(factories); } return this.providesService(first); } @@ -636,14 +655,14 @@ export class Container { // If the service depends on itself, e.g. in the multi-binding case, where we call append multiple times with // the same token, we always must resolve the dependency using the parent container to avoid infinite loop. const getFromParent = dependencies.indexOf(token) === -1 ? undefined : () => this.get(token as any); - const factory = memoize(this, function (this: Container) { + const factory = memoize(function (this: Container) { // Safety: getFromParent is defined if the token is in the dependencies list, so it is safe to call it. return fn(...(dependencies.map((t) => (t === token ? getFromParent!() : this.get(t))) as any)); }); - // Safety: `token` and `factory` are properly type checked, so extending `this.factories` produces a - // MaybeMemoizedFactories object with the expected set of services – but when using the spread operation to - // merge two objects, the compiler widens the Token type to string. So we must re-narrow via casting. - const factories = { ...this.factories, [token]: factory }; + // Extend `this.factories` via prototype chain so adding a service is O(1) — a chain of N + // `provides` calls is O(N) total instead of O(N²). + const factories = Object.create(this.factories) as MaybeMemoizedFactories>; + (factories as any)[token] = factory; return new Container(factories) as Container>; } } diff --git a/src/PartialContainer.ts b/src/PartialContainer.ts index 5f101ba..b8e0976 100644 --- a/src/PartialContainer.ts +++ b/src/PartialContainer.ts @@ -241,9 +241,14 @@ export class PartialContainer { // provides(Container) if (first instanceof Container) { const containerInjectables: Record> = {}; - for (const key of Object.keys(first.factories)) { - const factory = first.factories[key]; - containerInjectables[key] = Injectable(key, () => factory()); + // The source container's factories may be prototype-chained — walk the full chain + // with `for...in` so inherited factories are included. + const sourceFactories = first.factories; + for (const key in sourceFactories) { + const factory = (sourceFactories as any)[key]; + // Bind to the source container so dependencies still resolve from it when this + // injectable runs in a different container's context. + containerInjectables[key] = Injectable(key, () => factory.call(first)); } return new PartialContainer({ ...this.injectables, ...containerInjectables } as any); } @@ -325,7 +330,7 @@ export class PartialContainer { return (factories = Object.fromEntries( entries(this.injectables).map(([token, fn]) => [ token, - memoize(parent, () => + memoize(() => fn( ...(fn.dependencies.map((t) => { return t === token diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index f3d4aed..dcc3562 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -424,6 +424,21 @@ describe("Container", () => { expect(childContainerWithOverride.get("service")).toBe(1); }); + test("forking a child does not change the parent's view of its services", () => { + // The parent must remain a self-consistent snapshot: resolving a service via the parent + // continues to use the parent's own dependencies, regardless of overrides applied in a + // forked child. + const parent = Container.providesValue("value", 1).provides( + Injectable("service", ["value"], (value: number) => value) + ); + // Fork a child with an override; do NOT resolve anything on the child before reading from + // the parent. (Once a shared factory is resolved by any container, subsequent reads return + // the memoized value — sibling isolation requires copy(['token']).) + parent.providesValue("value", 2); + expect(parent.get("service")).toBe(1); + expect(parent.get("value")).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"); @@ -495,6 +510,48 @@ describe("Container", () => { }); }); + describe("when constructed with raw, non-memoized factories", () => { + test("the constructor memoizes them and resolution works", () => { + // Exercises the constructor's slow path — internal builders feed pre-memoized factories, + // so this path is only hit when the constructor is called directly with raw functions. + const raw = { a: () => 1, b: () => "two" }; + const c = new Container(raw); + expect(c.get("a")).toBe(1); + expect(c.get("b")).toBe("two"); + }); + + test("memoization holds across repeated gets", () => { + const factory = jest.fn(() => ({})); + const c = new Container({ thing: factory }); + const first = c.get("thing"); + const second = c.get("thing"); + expect(first).toBe(second); + expect(factory).toHaveBeenCalledTimes(1); + }); + + test("passes through already-memoized own factories on the slow path", () => { + // Slow path is entered because `raw` has at least one non-memoized own factory; the + // already-memoized `memoized` own factory must be preserved as-is. + const memoized = Container.providesValue("first", 1).factories.first; + const mixed = { first: memoized, second: () => 2 }; + const c: Container<{ first: number; second: number }> = new Container(mixed); + expect(c.get("first")).toBe(1); + expect(c.get("second")).toBe(2); + }); + + test("preserves a non-trivial prototype while memoizing own factories", () => { + // Build a factories-like object whose prototype chain holds an existing service, then + // mix in a non-memoized own factory. The constructor must memoize the own key while + // keeping the inherited service reachable. + const parent = Container.providesValue("inherited", "p"); + const child = Object.create(parent.factories) as any; + child.fresh = () => "child"; + const c: Container<{ inherited: string; fresh: string }> = new Container(child); + expect(c.get("fresh")).toBe("child"); + expect(c.get("inherited")).toBe("p"); + }); + }); + describe("when running a PartialContainer", () => { let service1: InjectableFunction; let service2: InjectableFunction; diff --git a/src/__tests__/Injectable.spec.ts b/src/__tests__/Injectable.spec.ts index a19d384..638b063 100644 --- a/src/__tests__/Injectable.spec.ts +++ b/src/__tests__/Injectable.spec.ts @@ -1,4 +1,5 @@ -import { Injectable } from "../Injectable"; +import { ConcatInjectable, Injectable, InjectableCompat } from "../Injectable"; +import { Container } from "../Container"; describe("Injectable", () => { describe("when given invalid arguments", () => { @@ -22,3 +23,32 @@ describe("Injectable", () => { }); }); }); + +describe("InjectableCompat", () => { + test("produces a working factory equivalent to Injectable()", () => { + const fn = InjectableCompat("TestService", ["dep"] as const, (dep: number) => dep * 2); + expect(fn.token).toBe("TestService"); + expect(fn.dependencies).toEqual(["dep"]); + // InjectableCompat widens its return type; call through `any` to exercise the runtime body. + expect((fn as any)(21)).toBe(42); + }); +}); + +describe("ConcatInjectable", () => { + test("appends the produced value to an existing array service", () => { + const container = Container.providesValue("items", [1] as number[]).provides( + ConcatInjectable("items", () => 2) + ); + expect(container.get("items")).toEqual([1, 2]); + }); + + test("throws when called with no factory function", () => { + expect(() => ConcatInjectable("items", undefined as any)).toThrowError(/Received invalid arguments/); + }); + + test("throws when factory arity does not match dependency count", () => { + expect(() => ConcatInjectable("items", ["dep"] as const, () => 1)).toThrowError( + /Function arity does not match/ + ); + }); +}); diff --git a/src/entries.ts b/src/entries.ts index cbc93ea..96114c1 100644 --- a/src/entries.ts +++ b/src/entries.ts @@ -1,7 +1,3 @@ // `Object.entries` does not use `keyof` types, so it loses type specificity. We'll fix this with a wrapper. export const entries = , U>(o: T): Array<[keyof T, T[keyof T]]> => Object.entries(o) as unknown as Array<[keyof T, T[keyof T]]>; - -// `Object.fromEntries` similarly does not preserve key types. -export const fromEntries = (entries: ReadonlyArray<[K, V]>): Record => - Object.fromEntries(entries) as Record; diff --git a/src/memoize.ts b/src/memoize.ts index 6fcff3c..43f90a8 100644 --- a/src/memoize.ts +++ b/src/memoize.ts @@ -3,21 +3,19 @@ type AnyFunction = (...args: A) => B; export type Memoized = { (...args: Parameters): ReturnType; delegate: Fn; - thisArg: any; }; export function isMemoized(fn: unknown): fn is Memoized { return typeof fn === "function" && typeof (fn as any).delegate === "function"; } -export function memoize(thisArg: any, delegate: Fn): Memoized { +export function memoize(delegate: Fn): Memoized { let memo: any; - const memoized = (...args: any[]) => { + const memoized = function (this: any, ...args: any[]) { if (typeof memo !== "undefined") return memo; - memo = delegate.apply(memoized.thisArg, args); + memo = delegate.apply(this, args); return memo; }; memoized.delegate = delegate; - memoized.thisArg = thisArg; - return memoized; + return memoized as Memoized; } From 44c999ac74e5c6961ce4bf03af7a7d1a52a5c24f Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 10:39:32 +1000 Subject: [PATCH 02/13] Skip allMemoized scan via private withMemoizedFactories factory Internal chain-building callers (providesService, copy, provides(Container|PartialContainer)) always feed pre-memoized factories. Route them through a trusted private static factory that bypasses the constructor's per-step scan entirely. The public constructor keeps its backwards-compatible memoize-as-needed behaviour for raw user input. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Container.ts | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Container.ts b/src/Container.ts index cfc59f6..91c3578 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -172,23 +172,11 @@ export class Container { readonly factories: Readonly>; constructor(factories: MaybeMemoizedFactories) { - // Fast path: when every own factory is already memoized, share the input directly. - // Internal builders (`provides`, `copy`, etc.) prepare factories this way and chain - // them via `Object.create`, keeping per-step construction O(1). + // Public path: callers may hand us raw, non-memoized factories. Memoize any own keys + // that aren't already memoized, preserving any prototype chain so inherited memoized + // factories stay reachable. Internal builders use {@link withMemoizedFactories} below + // to skip this scan entirely. const ownKeys = Object.keys(factories); - let allMemoized = true; - for (const k of ownKeys) { - if (!isMemoized((factories as any)[k])) { - allMemoized = false; - break; - } - } - if (allMemoized) { - this.factories = factories as unknown as Factories; - return; - } - // Slow path: memoize any non-memoized own factories, preserving any prototype chain - // so inherited memoized factories stay reachable. const proto = Object.getPrototypeOf(factories); const memoizedFactories = (proto && proto !== Object.prototype ? Object.create(proto) @@ -200,6 +188,17 @@ export class Container { this.factories = memoizedFactories; } + /** + * Trusted internal construction: skips the scan + per-key check that the public constructor + * performs, since chain-building paths (`provides`, `copy`, etc.) prepare factories that + * are guaranteed memoized. Keeps the per-step cost of building a `provides()` chain O(1). + */ + private static withMemoizedFactories(factories: Factories): Container { + const c = Object.create(Container.prototype) as Container; + (c as { factories: Factories }).factories = factories; + return c; + } + /** * Creates a copy of this Container, optionally scoping specified services to the new copy. * Unspecified services are shared between the original and copied containers, @@ -237,15 +236,15 @@ export class Container { if (!scopedServices || scopedServices.length === 0) { // Share factories via prototype chain — the new container resolves to the same memoized // instances as the original. - return new Container(Object.create(this.factories) as MaybeMemoizedFactories); + return Container.withMemoizedFactories(Object.create(this.factories) as Factories); } // Override scoped tokens with freshly-memoized copies of the original delegates so the new // container produces independent service instances for those tokens. - const factories = Object.create(this.factories) as MaybeMemoizedFactories; + const factories = Object.create(this.factories) as Factories; for (const token of scopedServices) { factories[token] = memoize(this.factories[token].delegate); } - return new Container(factories); + return Container.withMemoizedFactories(factories); } /** @@ -470,13 +469,13 @@ export class Container { const incoming = first instanceof PartialContainer ? first.getFactories(this) : first.factories; // Layer the incoming factories on top of this.factories via prototype chain — O(1) base // plus O(K) for K incoming keys (instead of spreading every factory in `this`). - const factories = Object.create(this.factories) as MaybeMemoizedFactories>; + const factories = Object.create(this.factories) as Factories>; // `for...in` walks the prototype chain so chained factories from the incoming container // come through. Newer keys (own properties) override older ones (inherited). for (const k in incoming) { (factories as any)[k] = (incoming as any)[k]; } - return new Container(factories); + return Container.withMemoizedFactories(factories); } return this.providesService(first); } @@ -661,8 +660,8 @@ export class Container { }); // Extend `this.factories` via prototype chain so adding a service is O(1) — a chain of N // `provides` calls is O(N) total instead of O(N²). - const factories = Object.create(this.factories) as MaybeMemoizedFactories>; + const factories = Object.create(this.factories) as Factories>; (factories as any)[token] = factory; - return new Container(factories) as Container>; + return Container.withMemoizedFactories(factories); } } From 3dfbf3402a3d9a1f095d8f1d9b2c4a3a89982668 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 10:55:36 +1000 Subject: [PATCH 03/13] Apply the same fix to PartialContainer + add chainedForEach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PartialContainer.provides() had the same O(N²) spread pattern. Apply the same prototype-chain extension via Object.create on this.injectables. PartialContainer chain at 8000 services: 5199 ms -> 513 ms (~10x). Also: for...in and naive [k] lookup on a deep Object.create chain are themselves O(N²) in V8 (key deduplication). Introduce a chainedForEach helper that walks the chain in a single linear pass, and use it from all sites that traverse a potentially-chained map (Container.provides merge, PartialContainer.provides + getFactories + getTokens). The benchmark now also covers PartialContainer construction and materialization. Investigation doc updated with the new numbers. Co-Authored-By: Claude Opus 4.7 (1M context) --- benchmarks/INVESTIGATION.md | 34 ++++++++++++++++------ benchmarks/provides-chain.ts | 43 +++++++++++++++++++++++---- src/Container.ts | 36 +++++++++++------------ src/PartialContainer.ts | 56 ++++++++++++++++++++---------------- src/entries.ts | 38 ++++++++++++++++++++++++ 5 files changed, 151 insertions(+), 56 deletions(-) diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md index 88c4a15..4d1e31c 100644 --- a/benchmarks/INVESTIGATION.md +++ b/benchmarks/INVESTIGATION.md @@ -2,12 +2,13 @@ ## TL;DR -Long `Container.provides().provides()...provides()` chains used to construct in O(N²) time. With a few hundred services this caused ANRs on low-end Android devices during container assembly. The fix makes chain construction O(N) per step (~O(N) total) by: +Long `Container.provides()` and `PartialContainer.provides()` chains used to construct in O(N²) time. With a few hundred services this caused ANRs on low-end Android devices during container assembly. The fix makes chain construction O(N) per step (~O(N) total) by: -1. Replacing the shallow spread of `this.factories` with prototype-chained extension (`Object.create`). +1. Replacing the shallow spread of `this.factories` / `this.injectables` with prototype-chained extension (`Object.create`). 2. Removing the per-step `for...in` loop in the `Container` constructor that rebinds `thisArg` on every already-memoized factory. +3. Replacing `for...in` and `obj[k]` traversal over chained maps with a single-pass `chainedForEach` helper — `for...in` is itself O(N²) on deep `Object.create` chains in V8, as is repeated `[k]` lookup. -End-to-end speedup: **20× faster for 8000-deep chains** (10,286 ms → 496 ms), **9× faster at 800**, **7.5× at 400**. All 87 tests pass with 100% line/branch/function coverage. +End-to-end speedup at chain depth 8000: **Container** 10,286 ms → ~500 ms (~20×), **PartialContainer** 5,199 ms → ~513 ms (~10×). All 87 tests pass with 100% line/branch/function coverage. ## How to reproduce @@ -15,10 +16,12 @@ End-to-end speedup: **20× faster for 8000-deep chains** (10,286 ms → 496 ms), npm run bench ``` -This builds chains of 50 → 8000 services and reports `ms/build`. There's also a get-pass over an 800-deep chain to track lookup cost. +The benchmark builds Container and PartialContainer chains of 50 → 8000 services, reports `ms/build`, then probes lookup cost on an 800-deep Container and materialization cost (`Container.provides(partial)`) across the same range. ## Numbers +**Container chain construction:** + | N services | before (ms) | after (ms) | speedup | |------------|-------------|------------|---------| | 50 | 0.05 | 0.03 | ~1.7× | @@ -30,7 +33,19 @@ This builds chains of 50 → 8000 services and reports `ms/build`. There's also | 3200 | 1518.40 | 75.28 | 20.2× | | 8000 | 10285.63 | 495.87 | 20.7× | -Lookup cost on an 800-deep chain went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see *Trade-offs* below. +**PartialContainer chain construction:** + +| N services | before (ms) | after (ms) | speedup | +|------------|-------------|------------|---------| +| 200 | 0.57 | 0.18 | 3.2× | +| 800 | 14.20 | 2.67 | 5.3× | +| 1600 | 134.99 | 15.90 | 8.5× | +| 3200 | 715.93 | 76.67 | 9.3× | +| 8000 | 5199.09 | 512.58 | 10.1× | + +**Materialization (`Container.provides(partial)`)** remains a fast linear pass: 2.4 ms at N=8000. + +Lookup cost on an 800-deep Container went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see *Trade-offs* below. Measured on Apple silicon, Node 20. Numbers will differ on other devices, but the relative shapes are what matter. @@ -80,13 +95,16 @@ We went with **(1) + (2) together**. They compose: (1) lets us skip the construc - **`src/memoize.ts`** — dropped the `thisArg` field from `Memoized`. Memoized functions now use the call-site `this` (`memo = delegate.apply(this, args)`). - **`src/Container.ts`**: - - Constructor: fast path that *shares* an already-memoized factories object (common internal case) and a slow path that memoizes own keys while preserving any prototype chain. + - Constructor: memoizes any non-memoized own factories of the input while preserving any prototype chain. + - Private `withMemoizedFactories` factory used by internal chain builders that already guarantee memoized input; skips the constructor scan entirely. - `get()`: invokes the factory via `factory.call(this)` so dependencies resolve against the calling container. - `provides()` (per-token and Container/PartialContainer merge): builds the new factories object via `Object.create(this.factories)` instead of a spread. - `copy()`: same prototype-chain approach; only scoped tokens get freshly-memoized own properties. - **`src/PartialContainer.ts`**: - - `getFactories()`: drops the now-unused `thisArg` argument to `memoize`. - - `provides(Container)`: walks the source container's factories with `for...in` (so prototype-chained factories come through) and pre-binds each wrapper to the source container via `factory.call(first)` — preserves the old "value pulled from source container" semantics for the bridge case. + - `getFactories()`: drops the now-unused `thisArg` argument to `memoize`; iterates via `chainedForEach` so prototype-chained injectables come through. + - `addInjectable`, `provides(PartialContainer)`, `provides(Container)`: use `Object.create(this.injectables)` + `chainedForEach` instead of object spread. Per-step cost is now O(1). + - `getTokens()`: uses `chainedKeys` so chained injectables are included. +- **`src/entries.ts`** — new `chainedForEach` / `chainedKeys` helpers. They walk an object's prototype chain explicitly because `for...in` (and naïve repeated `[k]` lookup) is O(N²) on deep `Object.create` chains in V8; the helpers stay linear (≈90× faster than `for...in` at depth 8000). ## Trade-offs diff --git a/benchmarks/provides-chain.ts b/benchmarks/provides-chain.ts index ef995a5..f5924f4 100644 --- a/benchmarks/provides-chain.ts +++ b/benchmarks/provides-chain.ts @@ -10,6 +10,7 @@ */ import { Container } from "../src/Container"; +import { PartialContainer } from "../src/PartialContainer"; function buildChain(n: number): Container> { let c: Container = new Container({}); @@ -19,6 +20,14 @@ function buildChain(n: number): Container> { return c; } +function buildPartialChain(n: number): PartialContainer { + let p: PartialContainer = new PartialContainer({}); + for (let i = 0; i < n; i++) { + p = p.provides(`svc${i}`, () => i); + } + return p; +} + function timeMs(fn: () => void): number { const start = process.hrtime.bigint(); fn(); @@ -26,10 +35,10 @@ function timeMs(fn: () => void): number { return Number(end - start) / 1e6; } -function bench(n: number, iters: number): number { - buildChain(n); // warmup +function bench(build: (n: number) => unknown, n: number, iters: number): number { + build(n); // warmup return timeMs(() => { - for (let i = 0; i < iters; i++) buildChain(n); + for (let i = 0; i < iters; i++) build(n); }) / iters; } @@ -39,10 +48,17 @@ const sizes = [50, 100, 200, 400, 800, 1600, 3200, 8000]; const itersFor = (n: number): number => n <= 200 ? 50 : n <= 400 ? 20 : n <= 800 ? 5 : n <= 3200 ? 3 : 2; -console.log("Chain construction:"); +console.log("Container chain construction:"); console.log("size\tms/build"); for (const n of sizes) { - const ms = bench(n, itersFor(n)); + const ms = bench(buildChain, n, itersFor(n)); + console.log(`${n}\t${ms.toFixed(2)}`); +} + +console.log("\nPartialContainer chain construction:"); +console.log("size\tms/build"); +for (const n of sizes) { + const ms = bench(buildPartialChain, n, itersFor(n)); console.log(`${n}\t${ms.toFixed(2)}`); } @@ -57,4 +73,19 @@ const lookupMs = for (let k = 0; k < lookupChainSize; k++) c.get(`svc${k}`); } }) / lookupIters; -console.log(`\nFull get-pass on ${lookupChainSize}-deep chain: ${lookupMs.toFixed(2)} ms/iter`); +console.log(`\nFull get-pass on ${lookupChainSize}-deep Container chain: ${lookupMs.toFixed(2)} ms/iter`); + +// Materialization probe: hand a PartialContainer of size N to a Container and time the merge. +// This exercises PartialContainer.getFactories + Container.provides(partial). +console.log("\nMaterialize PartialContainer into Container (`Container.provides(partial)`):"); +console.log("size\tms/materialize"); +for (const n of sizes) { + const partial = buildPartialChain(n); + const iters = itersFor(n); + Container.provides(partial); // warmup + const ms = + timeMs(() => { + for (let i = 0; i < iters; i++) Container.provides(partial); + }) / iters; + console.log(`${n}\t${ms.toFixed(2)}`); +} diff --git a/src/Container.ts b/src/Container.ts index 91c3578..e669e98 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -11,7 +11,7 @@ import type { ValidTokens, } from "./types"; import { ClassInjectable, ConcatInjectable, Injectable } from "./Injectable"; -import { entries } from "./entries"; +import { chainedForEach, entries } from "./entries"; type MaybeMemoizedFactories = { [K in keyof Services]: (() => Services[K]) | Memoized<() => Services[K]>; @@ -166,6 +166,17 @@ export class Container { ) as Container; } + /** + * Trusted internal construction: skips the per-key memoization scan the public constructor + * performs, since chain-building paths (`provides`, `copy`, etc.) prepare factories that + * are guaranteed memoized. Keeps the per-step cost of building a `provides()` chain O(1). + */ + private static withMemoizedFactories(factories: Factories): Container { + const c = Object.create(Container.prototype) as Container; + (c as { factories: Factories }).factories = factories; + return c; + } + // this is public on purpose; if the field is declared as private generated *.d.ts files do not include the field type // which makes typescript compiler behave differently when resolving container types; e.g. it becomes impossible to // assign a container of type Container<{ a: number, b: string }> to a variable of type Container<{ a: number }>. @@ -174,7 +185,7 @@ export class Container { constructor(factories: MaybeMemoizedFactories) { // Public path: callers may hand us raw, non-memoized factories. Memoize any own keys // that aren't already memoized, preserving any prototype chain so inherited memoized - // factories stay reachable. Internal builders use {@link withMemoizedFactories} below + // factories stay reachable. Internal builders use {@link withMemoizedFactories} above // to skip this scan entirely. const ownKeys = Object.keys(factories); const proto = Object.getPrototypeOf(factories); @@ -188,17 +199,6 @@ export class Container { this.factories = memoizedFactories; } - /** - * Trusted internal construction: skips the scan + per-key check that the public constructor - * performs, since chain-building paths (`provides`, `copy`, etc.) prepare factories that - * are guaranteed memoized. Keeps the per-step cost of building a `provides()` chain O(1). - */ - private static withMemoizedFactories(factories: Factories): Container { - const c = Object.create(Container.prototype) as Container; - (c as { factories: Factories }).factories = factories; - return c; - } - /** * Creates a copy of this Container, optionally scoping specified services to the new copy. * Unspecified services are shared between the original and copied containers, @@ -470,11 +470,11 @@ export class Container { // Layer the incoming factories on top of this.factories via prototype chain — O(1) base // plus O(K) for K incoming keys (instead of spreading every factory in `this`). const factories = Object.create(this.factories) as Factories>; - // `for...in` walks the prototype chain so chained factories from the incoming container - // come through. Newer keys (own properties) override older ones (inherited). - for (const k in incoming) { - (factories as any)[k] = (incoming as any)[k]; - } + // `chainedForEach` walks own + inherited keys with their declared-at-level values in + // a single pass, avoiding the O(N²) cost of `for...in` + `[k]` lookup on deep chains. + chainedForEach(incoming, (k, v) => { + (factories as any)[k] = v; + }); return Container.withMemoizedFactories(factories); } return this.providesService(first); diff --git a/src/PartialContainer.ts b/src/PartialContainer.ts index b8e0976..cebd51d 100644 --- a/src/PartialContainer.ts +++ b/src/PartialContainer.ts @@ -1,4 +1,4 @@ -import { entries } from "./entries"; +import { chainedForEach, chainedKeys, entries } from "./entries"; import type { Memoized } from "./memoize"; import { memoize } from "./memoize"; import { Container } from "./Container"; @@ -236,21 +236,24 @@ export class PartialContainer { } // provides(PartialContainer) if (first instanceof PartialContainer) { - return new PartialContainer({ ...this.injectables, ...first.injectables } as any); + // Layer the other partial's injectables on top via prototype chain. `chainedForEach` + // visits own + inherited bindings in a single linear pass. + const injectables = Object.create(this.injectables); + chainedForEach(first.injectables, (key, fn) => { + injectables[key] = fn; + }); + return new PartialContainer(injectables); } // provides(Container) if (first instanceof Container) { - const containerInjectables: Record> = {}; - // The source container's factories may be prototype-chained — walk the full chain - // with `for...in` so inherited factories are included. const sourceFactories = first.factories; - for (const key in sourceFactories) { - const factory = (sourceFactories as any)[key]; - // Bind to the source container so dependencies still resolve from it when this - // injectable runs in a different container's context. - containerInjectables[key] = Injectable(key, () => factory.call(first)); - } - return new PartialContainer({ ...this.injectables, ...containerInjectables } as any); + const injectables = Object.create(this.injectables); + // Bind each wrapper to the source container so the injectable resolves dependencies + // from there when it runs in a different container's context. + chainedForEach<(this: Container) => any>(sourceFactories, (key, factory) => { + injectables[key] = Injectable(key, () => factory.call(first)); + }); + return new PartialContainer(injectables); } // Original single-arg form: provides(InjectableFunction) return this.addInjectable((first as any).token, first as any); @@ -260,7 +263,11 @@ export class PartialContainer { token: TokenType, fn: InjectableFunction ): PartialContainer { - return new PartialContainer({ ...this.injectables, [token]: fn } as any); + // Extend `this.injectables` via prototype chain so each `provides()` is O(1) — a chain + // of N calls is O(N) total instead of O(N²). + const injectables = Object.create(this.injectables); + injectables[token] = fn; + return new PartialContainer(injectables); } /** @@ -326,26 +333,27 @@ export class PartialContainer { * @param parent A [Container] which provides all the required Dependencies of this PartialContainer. */ getFactories(parent: Container): PartialContainerFactories { - let factories: PartialContainerFactories | undefined = undefined; - return (factories = Object.fromEntries( - entries(this.injectables).map(([token, fn]) => [ - token, - memoize(() => + const factories = {} as PartialContainerFactories; + chainedForEach>( + this.injectables, + (token, fn) => { + (factories as any)[token] = memoize(() => fn( ...(fn.dependencies.map((t) => { return t === token ? parent.get(t as keyof Dependencies) - : factories![t as keyof Services & Dependencies] - ? factories![t]() + : factories[t as keyof Services & Dependencies] + ? factories[t as keyof Services]() : parent.get(t as keyof Dependencies); }) as any) ) - ), - ]) - ) as PartialContainerFactories); + ); + } + ); + return factories; } getTokens(): Array { - return Object.keys(this.injectables) as Array; + return chainedKeys(this.injectables) as Array; } } diff --git a/src/entries.ts b/src/entries.ts index 96114c1..0bd27e8 100644 --- a/src/entries.ts +++ b/src/entries.ts @@ -1,3 +1,41 @@ // `Object.entries` does not use `keyof` types, so it loses type specificity. We'll fix this with a wrapper. export const entries = , U>(o: T): Array<[keyof T, T[keyof T]]> => Object.entries(o) as unknown as Array<[keyof T, T[keyof T]]>; + +/** + * Walk an object's prototype chain in derivation order (most-derived first), invoking `cb` + * once for each enumerable string key with its own-property value at the level where it was + * declared. Keys defined at a more-derived level shadow inherited ones, just like normal + * property lookup. + * + * Why this exists: `for...in` is O(N²) on deep `Object.create` chains in V8 (the engine + * deduplicates keys as it descends), and so is `obj[k]` lookup. Chained `provides()` calls + * now build prototype-linked factory maps; this helper keeps full traversal linear (≈90× + * faster than `for...in` at chain depth 8000). + */ +export function chainedForEach(o: object, cb: (key: string, value: V) => void): void { + const seen = new Set(); + let cur: object | null = o; + while (cur && cur !== Object.prototype) { + const own = Object.keys(cur); + for (let i = 0; i < own.length; i++) { + const k = own[i]; + if (!seen.has(k)) { + seen.add(k); + cb(k, (cur as any)[k]); + } + } + cur = Object.getPrototypeOf(cur); + } +} + +/** + * Collect own + inherited enumerable string keys from a (possibly prototype-chained) object. + * Use {@link chainedForEach} if you also need values — this helper exists for cases that + * only need the key set. + */ +export function chainedKeys(o: object): string[] { + const keys: string[] = []; + chainedForEach(o, (k) => keys.push(k)); + return keys; +} From 5e2cef4cf4eb9f25f9f7a2e391ecdb7e2ab8e6c3 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 12:04:21 +1000 Subject: [PATCH 04/13] Apply prettier formatting CI's format:check (prettier -l *) was failing on the changes from earlier commits. No functional changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- benchmarks/INVESTIGATION.md | 15 ++++++++------- benchmarks/provides-chain.ts | 11 ++++++----- src/Container.ts | 6 +++--- src/PartialContainer.ts | 29 +++++++++++++---------------- src/__tests__/Injectable.spec.ts | 8 ++------ 5 files changed, 32 insertions(+), 37 deletions(-) diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md index 4d1e31c..b49a60e 100644 --- a/benchmarks/INVESTIGATION.md +++ b/benchmarks/INVESTIGATION.md @@ -23,7 +23,7 @@ The benchmark builds Container and PartialContainer chains of 50 → 8000 servic **Container chain construction:** | N services | before (ms) | after (ms) | speedup | -|------------|-------------|------------|---------| +| ---------- | ----------- | ---------- | ------- | | 50 | 0.05 | 0.03 | ~1.7× | | 100 | 0.19 | 0.08 | 2.4× | | 200 | 0.81 | 0.28 | 2.9× | @@ -36,7 +36,7 @@ The benchmark builds Container and PartialContainer chains of 50 → 8000 servic **PartialContainer chain construction:** | N services | before (ms) | after (ms) | speedup | -|------------|-------------|------------|---------| +| ---------- | ----------- | ---------- | ------- | | 200 | 0.57 | 0.18 | 3.2× | | 800 | 14.20 | 2.67 | 5.3× | | 1600 | 134.99 | 15.90 | 8.5× | @@ -45,7 +45,7 @@ The benchmark builds Container and PartialContainer chains of 50 → 8000 servic **Materialization (`Container.provides(partial)`)** remains a fast linear pass: 2.4 ms at N=8000. -Lookup cost on an 800-deep Container went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see *Trade-offs* below. +Lookup cost on an 800-deep Container went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see _Trade-offs_ below. Measured on Apple silicon, Node 20. Numbers will differ on other devices, but the relative shapes are what matter. @@ -116,17 +116,17 @@ In real applications a cold start typically touches a small handful of services, ### Subtle semantic shift: parent stays consistent after a fork -Previously, building a child container `C` from a parent `B` mutated `B`'s factories to point their `thisArg` at `C`. As a result, `b.get('svc')` *after* the fork could resolve dependencies through `C` — meaning the parent was no longer a self-consistent snapshot of its own state. +Previously, building a child container `C` from a parent `B` mutated `B`'s factories to point their `thisArg` at `C`. As a result, `b.get('svc')` _after_ the fork could resolve dependencies through `C` — meaning the parent was no longer a self-consistent snapshot of its own state. Now each container is a true snapshot: `b.get('svc')` resolves through `b`. This is captured by the new test `forking a child does not change the parent's view of its services`. -This **is** a behavioral change. It's strictly more correct (and intuitive), and the existing test suite — which covers override-before-init and override-after-init for the *child* — continues to pass. +This **is** a behavioral change. It's strictly more correct (and intuitive), and the existing test suite — which covers override-before-init and override-after-init for the _child_ — continues to pass. -### What is *not* fixed: sibling isolation +### What is _not_ fixed: sibling isolation Memoization is per-factory, not per-container. If you fork two children `C1` and `C2` from the same parent `B` and override the same token in each, the two siblings share the parent's factory for any service they didn't override. Whichever sibling resolves that service first sets the memoization, and the second sibling sees the cached value. -This was equally broken in the old code (with a different failure mode: whichever container was *built* most recently won). Sibling isolation requires `copy(['token'])` to un-memoize and re-memoize per scope. +This was equally broken in the old code (with a different failure mode: whichever container was _built_ most recently won). Sibling isolation requires `copy(['token'])` to un-memoize and re-memoize per scope. ### Public `factories` field @@ -146,5 +146,6 @@ All files | 100 | 100 | 100 | 100 with 87/87 tests passing. Two new tests lock in semantics that the patch introduces: + - `forking a child does not change the parent's view of its services` — parent stays consistent after a fork. - (Plus four small tests to close pre-existing coverage gaps: constructor slow path with raw factories, `InjectableCompat`, and the two `ConcatInjectable` validation branches.) diff --git a/benchmarks/provides-chain.ts b/benchmarks/provides-chain.ts index f5924f4..6998e4e 100644 --- a/benchmarks/provides-chain.ts +++ b/benchmarks/provides-chain.ts @@ -37,16 +37,17 @@ function timeMs(fn: () => void): number { function bench(build: (n: number) => unknown, n: number, iters: number): number { build(n); // warmup - return timeMs(() => { - for (let i = 0; i < iters; i++) build(n); - }) / iters; + return ( + timeMs(() => { + for (let i = 0; i < iters; i++) build(n); + }) / iters + ); } const sizes = [50, 100, 200, 400, 800, 1600, 3200, 8000]; // More iterations for the cheap cases so we get a stable signal; fewer for the expensive ones // so the suite finishes in seconds rather than minutes. -const itersFor = (n: number): number => - n <= 200 ? 50 : n <= 400 ? 20 : n <= 800 ? 5 : n <= 3200 ? 3 : 2; +const itersFor = (n: number): number => (n <= 200 ? 50 : n <= 400 ? 20 : n <= 800 ? 5 : n <= 3200 ? 3 : 2); console.log("Container chain construction:"); console.log("size\tms/build"); diff --git a/src/Container.ts b/src/Container.ts index e669e98..7267264 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -189,9 +189,9 @@ export class Container { // to skip this scan entirely. const ownKeys = Object.keys(factories); const proto = Object.getPrototypeOf(factories); - const memoizedFactories = (proto && proto !== Object.prototype - ? Object.create(proto) - : ({} as Factories)) as Factories; + const memoizedFactories = ( + proto && proto !== Object.prototype ? Object.create(proto) : ({} as Factories) + ) as Factories; for (const k of ownKeys) { const fn = (factories as any)[k]; (memoizedFactories as any)[k] = isMemoized(fn) ? fn : memoize(fn); diff --git a/src/PartialContainer.ts b/src/PartialContainer.ts index cebd51d..30e81f4 100644 --- a/src/PartialContainer.ts +++ b/src/PartialContainer.ts @@ -334,22 +334,19 @@ export class PartialContainer { */ getFactories(parent: Container): PartialContainerFactories { const factories = {} as PartialContainerFactories; - chainedForEach>( - this.injectables, - (token, fn) => { - (factories as any)[token] = memoize(() => - fn( - ...(fn.dependencies.map((t) => { - return t === token - ? parent.get(t as keyof Dependencies) - : factories[t as keyof Services & Dependencies] - ? factories[t as keyof Services]() - : parent.get(t as keyof Dependencies); - }) as any) - ) - ); - } - ); + chainedForEach>(this.injectables, (token, fn) => { + (factories as any)[token] = memoize(() => + fn( + ...(fn.dependencies.map((t) => { + return t === token + ? parent.get(t as keyof Dependencies) + : factories[t as keyof Services & Dependencies] + ? factories[t as keyof Services]() + : parent.get(t as keyof Dependencies); + }) as any) + ) + ); + }); return factories; } diff --git a/src/__tests__/Injectable.spec.ts b/src/__tests__/Injectable.spec.ts index 638b063..79656ee 100644 --- a/src/__tests__/Injectable.spec.ts +++ b/src/__tests__/Injectable.spec.ts @@ -36,9 +36,7 @@ describe("InjectableCompat", () => { describe("ConcatInjectable", () => { test("appends the produced value to an existing array service", () => { - const container = Container.providesValue("items", [1] as number[]).provides( - ConcatInjectable("items", () => 2) - ); + const container = Container.providesValue("items", [1] as number[]).provides(ConcatInjectable("items", () => 2)); expect(container.get("items")).toEqual([1, 2]); }); @@ -47,8 +45,6 @@ describe("ConcatInjectable", () => { }); test("throws when factory arity does not match dependency count", () => { - expect(() => ConcatInjectable("items", ["dep"] as const, () => 1)).toThrowError( - /Function arity does not match/ - ); + expect(() => ConcatInjectable("items", ["dep"] as const, () => 1)).toThrowError(/Function arity does not match/); }); }); From 4de62035638ac1bad968f19ec63829f1145205a6 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 12:33:26 +1000 Subject: [PATCH 05/13] Root Container.factories at a null prototype; add use-case tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The factories map was inheriting from Object.prototype, so c.get("toString") on a container that hadn't registered the token would silently return the result of Object.prototype.toString instead of throwing — a regression introduced when we switched to Object.create-based chains. Root every factory chain at a null-prototype object so token names that match Object.prototype methods don't leak through get(). Add tests for four use cases that 100% line coverage didn't pin down: - A service that declares $container as a dependency receives the container that resolved it (including after a fork with override). - Tokens that shadow Object.prototype methods (toString, hasOwnProperty, constructor) resolve to their registered values, while unregistered ones throw. - A 100-deep linear dependency chain resolves to the correct values. - copy(['token']) on a deeply chained container produces a fresh memoization for the scoped service while non-scoped services stay shared with the original. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Container.ts | 12 ++++-- src/__tests__/Container.spec.ts | 74 ++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/Container.ts b/src/Container.ts index 7267264..5f86ba0 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -184,13 +184,17 @@ export class Container { constructor(factories: MaybeMemoizedFactories) { // Public path: callers may hand us raw, non-memoized factories. Memoize any own keys - // that aren't already memoized, preserving any prototype chain so inherited memoized - // factories stay reachable. Internal builders use {@link withMemoizedFactories} above - // to skip this scan entirely. + // that aren't already memoized. Internal builders use {@link withMemoizedFactories} + // above to skip this scan entirely. + // + // Root the new factories at a null-prototype object so token names that match + // Object.prototype methods (`toString`, `constructor`, `__proto__`, …) don't leak + // through `get()` as if they were registered services. Non-trivial input prototypes + // are preserved so already-chained factory maps remain reachable. const ownKeys = Object.keys(factories); const proto = Object.getPrototypeOf(factories); const memoizedFactories = ( - proto && proto !== Object.prototype ? Object.create(proto) : ({} as Factories) + proto && proto !== Object.prototype ? Object.create(proto) : Object.create(null) ) as Factories; for (const k of ownKeys) { const fn = (factories as any)[k]; diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index dcc3562..c03b652 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line max-classes-per-file -import { Container } from "../Container"; +import { CONTAINER, Container } from "../Container"; import { Injectable } from "../Injectable"; import { PartialContainer } from "../PartialContainer"; import type { InjectableFunction } from "../types"; @@ -405,6 +405,58 @@ describe("Container", () => { }); }); + describe("when a service depends on the $container token", () => { + test("the service receives the container that resolved it", () => { + const initial = Container.providesValue("value", 10); + const extended = initial.provides( + Injectable("service", [CONTAINER] as const, (c: typeof initial) => c.get("value") * 2) + ); + expect(extended.get("service")).toBe(20); + }); + + test("after a fork with an override, $container resolves through the forked child", () => { + const parent = Container.providesValue("value", 1).provides( + Injectable("service", [CONTAINER] as const, (c: any) => c.get("value") * 10) + ); + // Fork an override without first resolving on the parent. + const child = parent.providesValue("value", 5); + expect(child.get("service")).toBe(50); + }); + }); + + describe("when token names collide with Object.prototype properties", () => { + test("registered tokens that shadow Object.prototype methods resolve correctly", () => { + const c = Container.providesValue("toString", "custom-toString") + .providesValue("hasOwnProperty", 42) + .providesValue("constructor", "fake-ctor"); + expect(c.get("toString")).toBe("custom-toString"); + expect(c.get("hasOwnProperty")).toBe(42); + expect(c.get("constructor")).toBe("fake-ctor"); + }); + + test("unregistered tokens that match Object.prototype methods still throw", () => { + // Without the null-prototype root, `c.factories.toString` would return + // Object.prototype.toString and silently invoke it instead of throwing. + const c = Container.providesValue("foo", 1) as Container; + expect(() => c.get("toString")).toThrowError(/Could not find Service for Token "toString"/); + expect(() => c.get("constructor")).toThrowError(/Could not find Service for Token "constructor"/); + }); + }); + + describe("on a deep dependency chain", () => { + test("resolves a 100-deep linear chain to the correct values", () => { + const SIZE = 100; + let c: Container = Container.providesValue("v0", 0); + for (let i = 1; i <= SIZE; i++) { + c = c.provides(`v${i}`, [`v${i - 1}`] as const, (prev: number) => prev + 1); + } + expect(c.get(`v${SIZE}`)).toBe(SIZE); + // Re-resolve a shallow service after the deep walk to confirm memoization stays distinct. + expect(c.get("v0")).toBe(0); + expect(c.get("v50")).toBe(50); + }); + }); + describe("overrides", () => { test("overriding value is supplied to the parent container function as a dependency", () => { let containerWithOverride = Container.providesValue("value", 1) @@ -488,6 +540,26 @@ describe("Container", () => { // The InjectableFunction was used to create a separate Service instance for each Container. expect(injectable).toBeCalledTimes(2); }); + + test("scoped copy on a deeply chained container produces a fresh memoization", () => { + // Bury the scoped service deep in a chain to exercise prototype-chain copy semantics. + let counter = 0; + let c: Container = Container.providesValue("base", 1); + for (let i = 0; i < 50; i++) c = c.providesValue(`pad${i}`, i); + c = c.provides("counter", () => ++counter); + for (let i = 0; i < 50; i++) c = c.providesValue(`tail${i}`, i); + + const originalValue = c.get("counter"); + const copy = c.copy(["counter"]); + const copiedValue = copy.get("counter"); + + expect(originalValue).toBe(1); + expect(copiedValue).toBe(2); + // Original's memoization is untouched. + expect(c.get("counter")).toBe(1); + // Non-scoped, inherited services still share memoization with the original. + expect(copy.get("tail10")).toBe(c.get("tail10")); + }); }); describe("when running a Service", () => { From 896be4e6d22b612bbd3804f9c5a29efe5ebd4735 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 14:14:07 +1000 Subject: [PATCH 06/13] Cache a flat view of Container.factories on first get PR-introduced regression: switching from `{...this.factories}` to `Object.create(this.factories)` made provides() construction O(1) per step but turned every Container.get() into a prototype-chain walk costing O(depth-of-token). V8's inline caches hide it; on Hermes (Snap mobile / Composer, no JIT) the team measured a ~20x regression on a full key sweep of an 800-deep chain. Fix: materialize a flat own-property snapshot of `factories` on the first get() and read from it thereafter. The factories chain itself stays prototype-linked so provides() construction remains O(1). Container instances are immutable after construction, so the cached snapshot is valid for the container's lifetime. Hot read path full sweep on V8 (Node): N=1600 drops from 13.3 ms/iter to 0.10 ms/iter (~129x). Curve is now linear in N. Construction numbers unchanged (within ~10% noise). Adds benchmarks/get-pass.ts + `npm run bench:get` to track this in CI. Co-Authored-By: Claude Opus 4.7 (1M context) --- benchmarks/INVESTIGATION.md | 33 +++++++++++----- benchmarks/get-pass.ts | 77 +++++++++++++++++++++++++++++++++++++ package.json | 1 + src/Container.ts | 23 ++++++++++- 4 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 benchmarks/get-pass.ts diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md index b49a60e..714fbe8 100644 --- a/benchmarks/INVESTIGATION.md +++ b/benchmarks/INVESTIGATION.md @@ -7,16 +7,18 @@ Long `Container.provides()` and `PartialContainer.provides()` chains used to con 1. Replacing the shallow spread of `this.factories` / `this.injectables` with prototype-chained extension (`Object.create`). 2. Removing the per-step `for...in` loop in the `Container` constructor that rebinds `thisArg` on every already-memoized factory. 3. Replacing `for...in` and `obj[k]` traversal over chained maps with a single-pass `chainedForEach` helper — `for...in` is itself O(N²) on deep `Object.create` chains in V8, as is repeated `[k]` lookup. +4. Materializing a flat own-property snapshot of `factories` on the first `Container.get()` so subsequent reads are O(1) regardless of where the token sits in the prototype chain. Without this, the read path was O(depth-of-token), and a full key sweep over a deep chain was O(N²). The hit is invisible on V8 (inline caches mask it) but real on Hermes — measured ~20× regression vs. the pre-PR flat-map baseline on Snap's mobile DI assembly workload. -End-to-end speedup at chain depth 8000: **Container** 10,286 ms → ~500 ms (~20×), **PartialContainer** 5,199 ms → ~513 ms (~10×). All 87 tests pass with 100% line/branch/function coverage. +End-to-end speedup at chain depth 8000: **Container** 10,286 ms → ~500 ms (~20×), **PartialContainer** 5,199 ms → ~513 ms (~10×). Hot read-path full-sweep on a 1600-deep chain: 13.3 ms/iter → 0.10 ms/iter (~129× on V8; more on engines without prototype-chain ICs). All 93 tests pass with 100% line/branch/function coverage. ## How to reproduce ```bash -npm run bench +npm run bench # chain construction + materialization + 800-deep get-pass probe +npm run bench:get # dedicated read-path bench: full key sweep at 50 … 1600 ``` -The benchmark builds Container and PartialContainer chains of 50 → 8000 services, reports `ms/build`, then probes lookup cost on an 800-deep Container and materialization cost (`Container.provides(partial)`) across the same range. +The first benchmark builds Container and PartialContainer chains of 50 → 8000 services and reports `ms/build`, then probes lookup cost on an 800-deep Container and materialization cost (`Container.provides(partial)`) across the same range. The second isolates the read path: a single container is built and the full key sweep is timed in a hot loop, which is what surfaced the Hermes regression. ## Numbers @@ -43,9 +45,20 @@ The benchmark builds Container and PartialContainer chains of 50 → 8000 servic | 3200 | 715.93 | 76.67 | 9.3× | | 8000 | 5199.09 | 512.58 | 10.1× | -**Materialization (`Container.provides(partial)`)** remains a fast linear pass: 2.4 ms at N=8000. +**Materialization (`Container.provides(partial)`)** remains a fast linear pass: ~3 ms at N=8000. -Lookup cost on an 800-deep Container went from ~0.09 ms/full-pass to ~0.95 ms/full-pass (≈1.2 µs per `get()` on first access). This is a deliberate trade — see _Trade-offs_ below. +**Hot read path** (`npm run bench:get`, same container, full key sweep, V8): + +| N | without flatten cache | with flatten cache | +| ---- | --------------------: | -----------------: | +| 50 | 0.004 ms | 0.003 ms | +| 100 | 0.013 ms | 0.005 ms | +| 200 | 0.044 ms | 0.010 ms | +| 400 | 0.180 ms | 0.024 ms | +| 800 | 1.883 ms | 0.069 ms | +| 1600 | 13.313 ms | 0.103 ms | + +Without the flatten cache the curve is O(N²); with it the curve is linear (≈ 60 ns per `get` once warmed). On Hermes the original O(N²) was much worse because there's no prototype-chain inline cache to mask it — Snap's mobile team measured ~20× regression on the read path at chain depth 800 before the cache was added. Measured on Apple silicon, Node 20. Numbers will differ on other devices, but the relative shapes are what matter. @@ -95,9 +108,9 @@ We went with **(1) + (2) together**. They compose: (1) lets us skip the construc - **`src/memoize.ts`** — dropped the `thisArg` field from `Memoized`. Memoized functions now use the call-site `this` (`memo = delegate.apply(this, args)`). - **`src/Container.ts`**: - - Constructor: memoizes any non-memoized own factories of the input while preserving any prototype chain. + - Constructor: memoizes any non-memoized own factories of the input, rooting the resulting map at a null-prototype object so token names that collide with `Object.prototype` methods (e.g. `"toString"`) don't leak through `get()`. - Private `withMemoizedFactories` factory used by internal chain builders that already guarantee memoized input; skips the constructor scan entirely. - - `get()`: invokes the factory via `factory.call(this)` so dependencies resolve against the calling container. + - `get()`: builds a lazy flat own-property snapshot of `factories` on first call and reads from it thereafter, so lookups stay O(1) regardless of prototype-chain depth. Invokes the factory via `factory.call(this)` so dependencies resolve against the calling container. - `provides()` (per-token and Container/PartialContainer merge): builds the new factories object via `Object.create(this.factories)` instead of a spread. - `copy()`: same prototype-chain approach; only scoped tokens get freshly-memoized own properties. - **`src/PartialContainer.ts`**: @@ -108,11 +121,11 @@ We went with **(1) + (2) together**. They compose: (1) lets us skip the construc ## Trade-offs -### Lookup cost grew slightly +### First-`get` cost on each container -`get(token)` now walks a prototype chain to find the factory. For an 800-deep chain that's ~1.2 µs per lookup on first access. After the factory is memoized once, the chain walk still happens to find the factory function, but the cached result short-circuits the body. +`Container.get()` materializes a flat own-property snapshot of `factories` on first call. That snapshot pays one O(N) walk of the prototype chain. The total work for `G` gets on a container with `N` services is therefore `N + G` (flatten once + G O(1) reads) instead of `G × depth-of-token` (current pre-cache state) or `G × 1` (pre-chain spread state). -In real applications a cold start typically touches a small handful of services, so the additional walk cost is negligible. If a workload ever does mass-access of services, a "flatten chain when depth exceeds N" optimization can be layered on later. +For real cold-start workloads (≥1 `get` per container, often N gets when running a partial), the flatten cost is amortized away and the net change vs. the old flat-map baseline is small. For pathological cases where you build a Container, do exactly one shallow `get`, and discard it, the flatten still walks the whole chain — but in that case the chain is also typically short, and even the pre-cache cost was negligible. ### Subtle semantic shift: parent stays consistent after a fork diff --git a/benchmarks/get-pass.ts b/benchmarks/get-pass.ts new file mode 100644 index 0000000..0cc29ea --- /dev/null +++ b/benchmarks/get-pass.ts @@ -0,0 +1,77 @@ +/** + * Microbenchmark for `Container.get()` cost across chain depth. + * + * Run with: + * npm run bench:get + * + * Designed to confirm that resolving a service is O(1) regardless of where it + * sits in the prototype-chained factories map. Without the read-path flatten, + * `factories[token]` is a chain walk costing O(depth), so a full get-pass over + * an N-deep chain is O(N²). With the flatten cache, the first `get` pays one + * O(N) walk and all subsequent gets are O(1), so the full sweep is O(N). + */ + +import { Container } from "../src/Container"; + +function buildChain(n: number): Container> { + let c: Container = new Container({}); + for (let i = 0; i < n; i++) { + c = c.provides(`svc${i}`, () => i); + } + return c; +} + +function timeMs(fn: () => void): number { + const start = process.hrtime.bigint(); + fn(); + const end = process.hrtime.bigint(); + return Number(end - start) / 1e6; +} + +const sizes = [50, 100, 200, 400, 800, 1600]; +// Cheap sizes need many iterations for a stable signal; expensive ones need fewer +// to keep the suite finishing in seconds. +const itersFor = (n: number): number => (n <= 100 ? 200 : n <= 400 ? 50 : n <= 800 ? 20 : 10); + +// Build a fresh container per iteration so we measure cold + warm gets together. +// (A single container would amortize the flatten over warm reads only, hiding the +// first-call cost. Real workloads typically resolve each service a handful of times.) +console.log("Full get-pass per iteration (build container + resolve every service once):"); +console.log("size\tms/iter"); +for (const n of sizes) { + const iters = itersFor(n); + // warmup + { + const c = buildChain(n); + for (let k = 0; k < n; k++) c.get(`svc${k}`); + } + const ms = + timeMs(() => { + for (let i = 0; i < iters; i++) { + const c = buildChain(n); + for (let k = 0; k < n; k++) c.get(`svc${k}`); + } + }) / iters; + console.log(`${n}\t${ms.toFixed(2)}`); +} + +// Hot read path: a single container, repeated full passes. The first pass pays +// any one-time setup (e.g. lazy flatten); subsequent passes hit memoized factories +// and should be uniformly cheap. If `get` is O(1), per-iter time stays linear in N. +console.log("\nHot read path per iteration (same container, repeated full sweep):"); +console.log("size\tms/iter"); +for (const n of sizes) { + const iters = 500; + const c = buildChain(n); + // warmup — triggers flatten (if any) and primes V8 inline caches. + for (let pass = 0; pass < 3; pass++) { + for (let k = 0; k < n; k++) c.get(`svc${k}`); + } + const ms = + timeMs(() => { + for (let i = 0; i < iters; i++) { + for (let k = 0; k < n; k++) c.get(`svc${k}`); + } + }) / iters; + console.log(`${n}\t${ms.toFixed(3)}`); +} diff --git a/package.json b/package.json index 97c9855..6b95363 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "test": "jest", "test:watch": "jest --clearCache && jest --watch", "bench": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/provides-chain.ts", + "bench:get": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/get-pass.ts", "compile": "tsc -b ./tsconfig.cjs.json ./tsconfig.esm.json ./tsconfig.types.json", "docs": "typedoc src/index.ts", "build": "rm -rf dist && rm -rf docs && npm run compile && npm run docs" diff --git a/src/Container.ts b/src/Container.ts index 5f86ba0..5231a35 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -182,6 +182,15 @@ export class Container { // assign a container of type Container<{ a: number, b: string }> to a variable of type Container<{ a: number }>. readonly factories: Readonly>; + // Lazy flat view of `factories` used by `get` for O(1) lookup. `factories` itself is a + // prototype-chained map (so `provides()` stays O(1) per step), but reading from it costs + // O(depth-of-token) per lookup. We materialize a single own-property snapshot on the first + // `get` and read from it thereafter. Container instances are immutable after construction, + // so the snapshot stays valid for the lifetime of this container. Engines without inline + // caches for prototype-chain access (notably Hermes) make this materially important; even + // on V8 a full key sweep on a deep chain drops from O(N²) to O(N). + private flatFactories?: Factories; + constructor(factories: MaybeMemoizedFactories) { // Public path: callers may hand us raw, non-memoized factories. Memoize any own keys // that aren't already memoized. Internal builders use {@link withMemoizedFactories} @@ -271,7 +280,11 @@ export class Container { get(token: ContainerToken | keyof Services): this | Services[keyof Services] { if (token === CONTAINER) return this; - const factory = this.factories[token]; + // Materialize a flat own-property snapshot of `factories` on first read so subsequent + // lookups don't pay the prototype-chain walk cost. Once built, the snapshot is reused + // for every `get` on this container. + const flat = this.flatFactories ?? (this.flatFactories = this.buildFlatFactories()); + const factory = flat[token as keyof Services]; if (!factory) { throw new Error( `[Container::get] Could not find Service for Token "${String(token)}". This should've caused a ` + @@ -285,6 +298,14 @@ export class Container { return factory.call(this); } + private buildFlatFactories(): Factories { + const flat = Object.create(null) as Factories; + chainedForEach unknown>>(this.factories, (k, v) => { + (flat as Record unknown>>)[k] = v; + }); + return flat; + } + /** * Runs the factory functions for all services listed in the provided {@link PartialContainer}, * along with their dependencies that are registered within *this* container. From cd84f0696c5c7eb13b4144d920fe257e52f7932b Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 14:28:57 +1000 Subject: [PATCH 07/13] Consolidate bench scripts into one entry point Drop the separate `bench:get` script; `npm run bench` now runs both construction and read-path benchmarks back-to-back via benchmarks/index.ts. One command, complete perf picture. Co-Authored-By: Claude Opus 4.7 (1M context) --- benchmarks/INVESTIGATION.md | 10 ++++++---- benchmarks/index.ts | 4 ++++ package.json | 3 +-- 3 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 benchmarks/index.ts diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md index 714fbe8..75ddd61 100644 --- a/benchmarks/INVESTIGATION.md +++ b/benchmarks/INVESTIGATION.md @@ -14,11 +14,13 @@ End-to-end speedup at chain depth 8000: **Container** 10,286 ms → ~500 ms (~20 ## How to reproduce ```bash -npm run bench # chain construction + materialization + 800-deep get-pass probe -npm run bench:get # dedicated read-path bench: full key sweep at 50 … 1600 +npm run bench ``` -The first benchmark builds Container and PartialContainer chains of 50 → 8000 services and reports `ms/build`, then probes lookup cost on an 800-deep Container and materialization cost (`Container.provides(partial)`) across the same range. The second isolates the read path: a single container is built and the full key sweep is timed in a hot loop, which is what surfaced the Hermes regression. +Runs two sections back-to-back: + +1. **Construction + materialization** (`benchmarks/provides-chain.ts`) — Container and PartialContainer chains of 50 → 8000 services, `ms/build`, plus an 800-deep lookup probe and `Container.provides(partial)` materialization cost across the same range. +2. **Read-path** (`benchmarks/get-pass.ts`) — single container, full key sweep in a hot loop at 50 → 1600. This is what surfaced the Hermes regression. ## Numbers @@ -47,7 +49,7 @@ The first benchmark builds Container and PartialContainer chains of 50 → 8000 **Materialization (`Container.provides(partial)`)** remains a fast linear pass: ~3 ms at N=8000. -**Hot read path** (`npm run bench:get`, same container, full key sweep, V8): +**Hot read path** (same container, full key sweep, V8): | N | without flatten cache | with flatten cache | | ---- | --------------------: | -----------------: | diff --git a/benchmarks/index.ts b/benchmarks/index.ts new file mode 100644 index 0000000..c75cf91 --- /dev/null +++ b/benchmarks/index.ts @@ -0,0 +1,4 @@ +// Single entry point for `npm run bench`. Runs both bench files in order so the +// full perf picture (construction + read path) is captured in one invocation. +import "./provides-chain"; +import "./get-pass"; diff --git a/package.json b/package.json index 6b95363..d62f23a 100644 --- a/package.json +++ b/package.json @@ -28,8 +28,7 @@ "format:fix": "npm run format:check -- --write || exit 0", "test": "jest", "test:watch": "jest --clearCache && jest --watch", - "bench": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/provides-chain.ts", - "bench:get": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/get-pass.ts", + "bench": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"moduleResolution\":\"node\",\"target\":\"es2020\",\"esModuleInterop\":true}' ts-node --transpile-only benchmarks/index.ts", "compile": "tsc -b ./tsconfig.cjs.json ./tsconfig.esm.json ./tsconfig.types.json", "docs": "typedoc src/index.ts", "build": "rm -rf dist && rm -rf docs && npm run compile && npm run docs" From 1a96aef5854e1c493d2527c3583286d051c7b94a Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 16:28:50 +1000 Subject: [PATCH 08/13] Address review: flat factories shape + inherited memoization + __proto__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three regressions surfaced in PR review, all rooted in the Object.create-based chain leaking into externally observable behavior: 1. Container.factories observable shape changed. It was a flat own-property map; the chain switch made Object.keys(c.factories) return only the most recent layer. Fix: rename internal storage to `factoriesChain` (private) and expose `factories` as a lazy flat getter built on first access. External readers see the full set of tokens again; internal chain extension and the existing flat-cache lookup in get() continue to work unchanged. 2. Constructor skipped memoizing inherited factories. The slow path only scanned Object.keys(input), so `new Container(Object.create(parent))` left inherited raw factories un-memoized — breaking singleton semantics and crashing copy() (no .delegate to un-memoize). Fix: iterate via chainedForEach so own + inherited are both memoized into a clean null-rooted own-property root. 3. PartialContainer.__proto__ tokens disappeared. addInjectable did `Object.create(this.injectables)` where this.injectables inherited Object.prototype, so assigning to "__proto__" triggered the inherited setter and silently mutated the prototype. Fix: same null-rooted normalization as Container, plus a private withInjectables fast path that skips re-normalization for internal chain builders. Also null-rooted the temporary factories object built inside PartialContainer.getFactories so the materialized form preserves "__proto__" too. Adds one test per regression. 100% coverage holds at 96/96. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Container.ts | 84 +++++++++++++------------- src/PartialContainer.ts | 38 ++++++++++-- src/__tests__/Container.spec.ts | 45 +++++++++++--- src/__tests__/PartialContainer.spec.ts | 23 +++++++ 4 files changed, 133 insertions(+), 57 deletions(-) diff --git a/src/Container.ts b/src/Container.ts index 5231a35..9e7ee39 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -171,45 +171,45 @@ export class Container { * performs, since chain-building paths (`provides`, `copy`, etc.) prepare factories that * are guaranteed memoized. Keeps the per-step cost of building a `provides()` chain O(1). */ - private static withMemoizedFactories(factories: Factories): Container { + private static withMemoizedFactories(factoriesChain: Factories): Container { const c = Object.create(Container.prototype) as Container; - (c as { factories: Factories }).factories = factories; + (c as unknown as { factoriesChain: Factories }).factoriesChain = factoriesChain; return c; } - // this is public on purpose; if the field is declared as private generated *.d.ts files do not include the field type - // which makes typescript compiler behave differently when resolving container types; e.g. it becomes impossible to - // assign a container of type Container<{ a: number, b: string }> to a variable of type Container<{ a: number }>. - readonly factories: Readonly>; - - // Lazy flat view of `factories` used by `get` for O(1) lookup. `factories` itself is a - // prototype-chained map (so `provides()` stays O(1) per step), but reading from it costs - // O(depth-of-token) per lookup. We materialize a single own-property snapshot on the first - // `get` and read from it thereafter. Container instances are immutable after construction, - // so the snapshot stays valid for the lifetime of this container. Engines without inline - // caches for prototype-chain access (notably Hermes) make this materially important; even - // on V8 a full key sweep on a deep chain drops from O(N²) to O(N). + // Internal prototype-chained storage. Each `provides()` call extends this via + // `Object.create` so per-step construction stays O(1). The chain is null-prototype-rooted + // so token names that match `Object.prototype` methods (`toString`, `__proto__`, …) don't + // leak through reads as if they were registered services. + private factoriesChain!: Factories; + + // Lazy flat own-property snapshot of `factoriesChain`. Materialized on first read of the + // public `factories` getter (or first `get()` call). Container instances are immutable + // after construction, so the snapshot stays valid for this container's lifetime. private flatFactories?: Factories; + /** + * A flat own-property map of every Service registered in this Container. Materialized + * lazily on first access; safe to inspect with `Object.keys` / `Object.entries` / spread. + * + * Declared as a public getter (rather than a private field) so the generated `*.d.ts` + * exposes the property's type — otherwise TypeScript can't tell that a container of type + * `Container<{ a: number, b: string }>` is assignable to `Container<{ a: number }>`. + */ + get factories(): Readonly> { + return this.flatFactories ?? (this.flatFactories = this.buildFlatFactories()); + } + constructor(factories: MaybeMemoizedFactories) { - // Public path: callers may hand us raw, non-memoized factories. Memoize any own keys - // that aren't already memoized. Internal builders use {@link withMemoizedFactories} - // above to skip this scan entirely. - // - // Root the new factories at a null-prototype object so token names that match - // Object.prototype methods (`toString`, `constructor`, `__proto__`, …) don't leak - // through `get()` as if they were registered services. Non-trivial input prototypes - // are preserved so already-chained factory maps remain reachable. - const ownKeys = Object.keys(factories); - const proto = Object.getPrototypeOf(factories); - const memoizedFactories = ( - proto && proto !== Object.prototype ? Object.create(proto) : Object.create(null) - ) as Factories; - for (const k of ownKeys) { - const fn = (factories as any)[k]; - (memoizedFactories as any)[k] = isMemoized(fn) ? fn : memoize(fn); - } - this.factories = memoizedFactories; + // Public construction path. Flatten the input — own + inherited — into a clean + // null-prototype-rooted own-property map, memoizing any non-memoized factories along + // the way. Internal builders bypass this via {@link withMemoizedFactories} when they + // can guarantee the input is already memoized. + const root = Object.create(null) as Factories; + chainedForEach<(() => unknown) | Memoized<() => unknown>>(factories, (k, fn) => { + (root as Record unknown>>)[k] = isMemoized(fn) ? fn : memoize(fn); + }); + (this as unknown as { factoriesChain: Factories }).factoriesChain = root; } /** @@ -249,13 +249,13 @@ export class Container { if (!scopedServices || scopedServices.length === 0) { // Share factories via prototype chain — the new container resolves to the same memoized // instances as the original. - return Container.withMemoizedFactories(Object.create(this.factories) as Factories); + return Container.withMemoizedFactories(Object.create(this.factoriesChain) as Factories); } // Override scoped tokens with freshly-memoized copies of the original delegates so the new // container produces independent service instances for those tokens. - const factories = Object.create(this.factories) as Factories; + const factories = Object.create(this.factoriesChain) as Factories; for (const token of scopedServices) { - factories[token] = memoize(this.factories[token].delegate); + factories[token] = memoize(this.factoriesChain[token].delegate); } return Container.withMemoizedFactories(factories); } @@ -300,7 +300,7 @@ export class Container { private buildFlatFactories(): Factories { const flat = Object.create(null) as Factories; - chainedForEach unknown>>(this.factories, (k, v) => { + chainedForEach unknown>>(this.factoriesChain, (k, v) => { (flat as Record unknown>>)[k] = v; }); return flat; @@ -492,9 +492,9 @@ export class Container { // Original single-arg forms if (first instanceof PartialContainer || first instanceof Container) { const incoming = first instanceof PartialContainer ? first.getFactories(this) : first.factories; - // Layer the incoming factories on top of this.factories via prototype chain — O(1) base - // plus O(K) for K incoming keys (instead of spreading every factory in `this`). - const factories = Object.create(this.factories) as Factories>; + // Layer the incoming factories on top of this.factoriesChain via prototype chain — + // O(1) base plus O(K) for K incoming keys (instead of spreading every factory in `this`). + const factories = Object.create(this.factoriesChain) as Factories>; // `chainedForEach` walks own + inherited keys with their declared-at-level values in // a single pass, avoiding the O(N²) cost of `for...in` + `[k]` lookup on deep chains. chainedForEach(incoming, (k, v) => { @@ -683,9 +683,9 @@ export class Container { // Safety: getFromParent is defined if the token is in the dependencies list, so it is safe to call it. return fn(...(dependencies.map((t) => (t === token ? getFromParent!() : this.get(t))) as any)); }); - // Extend `this.factories` via prototype chain so adding a service is O(1) — a chain of N - // `provides` calls is O(N) total instead of O(N²). - const factories = Object.create(this.factories) as Factories>; + // Extend `this.factoriesChain` via prototype chain so adding a service is O(1) — a chain + // of N `provides` calls is O(N) total instead of O(N²). + const factories = Object.create(this.factoriesChain) as Factories>; (factories as any)[token] = factory; return Container.withMemoizedFactories(factories); } diff --git a/src/PartialContainer.ts b/src/PartialContainer.ts index 30e81f4..f0b40a5 100644 --- a/src/PartialContainer.ts +++ b/src/PartialContainer.ts @@ -103,7 +103,32 @@ export class PartialContainer { return container as PartialContainer; } - constructor(private readonly injectables: Injectables) {} + /** + * Trusted internal construction: skips the constructor's input-normalization pass since + * chain-building paths (`addInjectable`, `provides`) prepare an injectables object that is + * already null-prototype-rooted and chain-extended. Keeps per-step cost O(1). + */ + private static withInjectables(injectables: Injectables): PartialContainer { + const p = Object.create(PartialContainer.prototype) as PartialContainer; + (p as unknown as { injectables: Injectables }).injectables = injectables; + return p; + } + + // Internal prototype-chained storage. Each `provides()` call extends this via + // `Object.create` so per-step construction stays O(1). Rooted at a null-prototype object + // so token names that match `Object.prototype` methods (`__proto__`, `toString`, …) don't + // trigger inherited setters or shadow inherited methods through reads. + private readonly injectables!: Injectables; + + constructor(input: Injectables) { + // Public construction path. Flatten the input (own + inherited) into a null-prototype- + // rooted own-property map. Internal builders bypass this via {@link withInjectables}. + const root = Object.create(null) as Injectables; + chainedForEach>(input, (k, fn) => { + (root as any)[k] = fn; + }); + (this as unknown as { injectables: Injectables }).injectables = root; + } /** * Create a new PartialContainer which provides a Service created by a pre-built InjectableFunction. @@ -242,7 +267,7 @@ export class PartialContainer { chainedForEach(first.injectables, (key, fn) => { injectables[key] = fn; }); - return new PartialContainer(injectables); + return PartialContainer.withInjectables(injectables); } // provides(Container) if (first instanceof Container) { @@ -253,7 +278,7 @@ export class PartialContainer { chainedForEach<(this: Container) => any>(sourceFactories, (key, factory) => { injectables[key] = Injectable(key, () => factory.call(first)); }); - return new PartialContainer(injectables); + return PartialContainer.withInjectables(injectables); } // Original single-arg form: provides(InjectableFunction) return this.addInjectable((first as any).token, first as any); @@ -267,7 +292,7 @@ export class PartialContainer { // of N calls is O(N) total instead of O(N²). const injectables = Object.create(this.injectables); injectables[token] = fn; - return new PartialContainer(injectables); + return PartialContainer.withInjectables(injectables); } /** @@ -333,7 +358,10 @@ export class PartialContainer { * @param parent A [Container] which provides all the required Dependencies of this PartialContainer. */ getFactories(parent: Container): PartialContainerFactories { - const factories = {} as PartialContainerFactories; + // Null-prototype root so token names that match `Object.prototype` methods + // (`__proto__`, `toString`, …) get stored as own properties instead of triggering + // inherited setters. + const factories = Object.create(null) as PartialContainerFactories; chainedForEach>(this.injectables, (token, fn) => { (factories as any)[token] = memoize(() => fn( diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index c03b652..8142851 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -576,6 +576,13 @@ describe("Container", () => { }); describe("when accessing factories", () => { + test("Object.keys returns every registered token regardless of chain depth", () => { + // Public `factories` exposes a flat own-property view; internal chain extension via + // Object.create stays an implementation detail. + const c = Container.providesValue("a", 1).providesValue("b", 2).providesValue("c", 3); + expect(Object.keys(c.factories).sort()).toEqual(["a", "b", "c"]); + }); + test("the factories are returned", () => { let c = container.providesValue("service", "value"); expect(c.factories.service()).toEqual("value"); @@ -611,16 +618,34 @@ describe("Container", () => { expect(c.get("second")).toBe(2); }); - test("preserves a non-trivial prototype while memoizing own factories", () => { - // Build a factories-like object whose prototype chain holds an existing service, then - // mix in a non-memoized own factory. The constructor must memoize the own key while - // keeping the inherited service reachable. - const parent = Container.providesValue("inherited", "p"); - const child = Object.create(parent.factories) as any; - child.fresh = () => "child"; - const c: Container<{ inherited: string; fresh: string }> = new Container(child); - expect(c.get("fresh")).toBe("child"); - expect(c.get("inherited")).toBe("p"); + test("memoizes both own and inherited factories of a chained input", () => { + // The constructor must memoize EVERY enumerable factory it sees — own and inherited — + // otherwise inherited raw functions stay un-memoized (broken singleton semantics) and + // their `.delegate` is undefined (breaks `copy(['token'])`). + const sharedCount = { calls: 0 }; + const protoLike: Record unknown> = { + inherited: () => { + sharedCount.calls += 1; + return "inherited-value"; + }, + }; + const child = Object.create(protoLike) as Record unknown>; + child.fresh = () => "fresh-value"; + const c = new Container(child) as Container<{ inherited: string; fresh: string }>; + + // Resolution works for both, and the inherited factory is memoized (called once even + // across multiple resolutions and a scoped copy). + expect(c.get("fresh")).toBe("fresh-value"); + expect(c.get("inherited")).toBe("inherited-value"); + expect(c.get("inherited")).toBe("inherited-value"); + expect(sharedCount.calls).toBe(1); + + // copy(['inherited']) requires the inherited factory to be memoized so it has a + // `.delegate` to un-memoize. + const scoped = c.copy(["inherited"]); + expect(scoped.get("inherited")).toBe("inherited-value"); + // Fresh memoization on the copy means the delegate ran once more. + expect(sharedCount.calls).toBe(2); }); }); diff --git a/src/__tests__/PartialContainer.spec.ts b/src/__tests__/PartialContainer.spec.ts index cf20994..5e627c2 100644 --- a/src/__tests__/PartialContainer.spec.ts +++ b/src/__tests__/PartialContainer.spec.ts @@ -342,4 +342,27 @@ describe("PartialContainer", () => { }); }); }); + + describe("when constructed with a non-empty injectables map", () => { + test("flattens the input and exposes every injectable via getTokens", () => { + const a = Injectable("a", () => 1); + const b = Injectable("b", () => "two"); + const partial = new PartialContainer({ a, b } as any); + expect(partial.getTokens().sort()).toEqual(["a", "b"]); + const c = Container.provides(partial) as Container; + expect(c.get("a")).toBe(1); + expect(c.get("b")).toBe("two"); + }); + }); + + describe("token names that collide with Object.prototype properties", () => { + test("registering a service with the token '__proto__' preserves it through materialization", () => { + // Without a null-prototype root, assigning to __proto__ triggers the inherited setter + // and the token disappears from getTokens()/getFactories(). + const partial = new PartialContainer({}).providesValue("__proto__", 1); + expect(partial.getTokens()).toContain("__proto__"); + const container = Container.provides(partial) as Container; + expect(container.get("__proto__")).toBe(1); + }); + }); }); From 43214e3bf0453b394d4a59c06fed71b47c4795ff Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 16:38:28 +1000 Subject: [PATCH 09/13] Address review: keep zero-arg public factory calls working MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before PR #19 the public Container.factories map held memoized factories that carried a `thisArg`, so calling `c.factories.svc()` directly (without going through `get()`) would still resolve dependencies through the container. After the chain switch the factory body uses call-site `this`, which means a direct invocation makes `this` the factories map itself — `this.get(...)` then throws for any service with dependencies. Fix in `buildFlatFactories`: when materializing the public flat view, wrap each factory in a tiny forwarder that calls the underlying memoized factory with the container as `this`. Memo state stays in the underlying factory's closure (so memoization is preserved and shared across the chain), and both `get()` and a direct `c.factories[token]()` now route through the same container — matching the old public contract. Adds a test that exercises a dependent service through the public factories map. 97/97 tests pass, 100% coverage. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Container.ts | 12 +++++++++++- src/__tests__/Container.spec.ts | 9 +++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Container.ts b/src/Container.ts index 9e7ee39..fa9b670 100644 --- a/src/Container.ts +++ b/src/Container.ts @@ -300,8 +300,18 @@ export class Container { private buildFlatFactories(): Factories { const flat = Object.create(null) as Factories; + const self = this; chainedForEach unknown>>(this.factoriesChain, (k, v) => { - (flat as Record unknown>>)[k] = v; + // Wrap each factory so direct invocation via `c.factories[token]()` resolves the + // service through THIS container, not through the factories map (which would + // otherwise be `this` inside the underlying memoized closure and break any + // factory that calls `this.get(...)`). Forwarding to `v.call(self)` preserves + // memoization — memo state lives in `v`'s closure and is shared across the chain + // — and keeps `get()`'s call-site `this` semantics intact since `get()` and a + // direct call both end up routing through `self`. + const bound = (() => v.call(self)) as Memoized<() => unknown>; + bound.delegate = v.delegate; + (flat as Record unknown>>)[k] = bound; }); return flat; } diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index 8142851..41d7f6c 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -576,6 +576,15 @@ describe("Container", () => { }); describe("when accessing factories", () => { + test("direct invocation of factories[token] works for services with dependencies", () => { + // Pre-PR-#19, memoized factories carried `thisArg`, so calling a factory directly + // (bypassing `get()`) still resolved its dependencies through the container. + // Consumers using the public `factories` map should not see `this.get is not a + // function` from a dependent service. + const c = Container.providesValue("dep", 42).provides("svc", ["dep"] as const, (d: number) => d * 2); + expect(c.factories.svc()).toBe(84); + }); + test("Object.keys returns every registered token regardless of chain depth", () => { // Public `factories` exposes a flat own-property view; internal chain extension via // Object.create stays an implementation detail. From 19e02e6ab2312e53c6a423d0bc24402c08323cc3 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 16:43:29 +1000 Subject: [PATCH 10/13] Add tests for the three direct-invocation invariants Pin down the public-contract properties the previous commit re-established so they can't silently regress: - get() and direct factories[token]() share memoization (single run). - Direct invocation works for a service inherited via a deep chain. - Direct invocation picks up dependency overrides applied later in the chain (matching the get() codepath and pre-PR thisArg behavior). 100/100 tests pass, 100% coverage held. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/Container.spec.ts | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index 41d7f6c..4d173cc 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -585,6 +585,43 @@ describe("Container", () => { expect(c.factories.svc()).toBe(84); }); + test("get() and a direct factories[token]() call share memoization", () => { + // The underlying memoized factory runs at most once regardless of which entry + // point hits it first; both routes return the same instance. + let runs = 0; + const c = Container.providesValue("dep", 10).provides("svc", ["dep"] as const, (d: number) => { + runs += 1; + return { value: d }; + }); + const viaGet = c.get("svc"); + const viaDirect = c.factories.svc(); + expect(viaGet).toBe(viaDirect); + expect(runs).toBe(1); + }); + + test("direct invocation works for a service inherited via a deep chain", () => { + // `svc` is registered near the base; descendants reach it through the prototype + // chain. Direct invocation on a descendant must still route resolution through + // that descendant's flat view, not throw on the chain walk. + let c: Container = Container.providesValue("dep", 5).provides( + "svc", + ["dep"] as const, + (d: number) => d * 10 + ); + for (let i = 0; i < 50; i++) c = c.providesValue(`pad${i}`, i); + expect(c.factories.svc()).toBe(50); + }); + + test("direct invocation picks up dependency overrides applied later in the chain", () => { + // The dependent service is registered when `value` is 1; a later override sets it + // to 2. Mirrors the equivalent get() test but goes through the factories map — + // both routes must honor the override (matching the pre-PR thisArg behavior). + const c = Container.providesValue("value", 1) + .provides("svc", ["value"] as const, (v: number) => v) + .providesValue("value", 2); + expect(c.factories.svc()).toBe(2); + }); + test("Object.keys returns every registered token regardless of chain depth", () => { // Public `factories` exposes a flat own-property view; internal chain extension via // Object.create stays an implementation detail. From 2fce9d3f964e25b103737bd90f7601622883da39 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Thu, 14 May 2026 16:49:03 +1000 Subject: [PATCH 11/13] Address review: drop time-relative / PR-number references in tests Rewrite the two comments that anchored their explanation to "Pre-PR-#19" and "the pre-PR thisArg behavior" so each test describes the contract in terms of the current API. PR numbers age out; the test names already capture the property under test. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/Container.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index 4d173cc..f68dc78 100644 --- a/src/__tests__/Container.spec.ts +++ b/src/__tests__/Container.spec.ts @@ -577,10 +577,10 @@ describe("Container", () => { describe("when accessing factories", () => { test("direct invocation of factories[token] works for services with dependencies", () => { - // Pre-PR-#19, memoized factories carried `thisArg`, so calling a factory directly - // (bypassing `get()`) still resolved its dependencies through the container. - // Consumers using the public `factories` map should not see `this.get is not a - // function` from a dependent service. + // `Container.factories` is part of the public API; consumers must be able to call a + // factory directly without going through `get()`, even for services whose body + // resolves dependencies (which would otherwise see the factories map as `this` and + // throw on `this.get(...)`). const c = Container.providesValue("dep", 42).provides("svc", ["dep"] as const, (d: number) => d * 2); expect(c.factories.svc()).toBe(84); }); @@ -615,7 +615,7 @@ describe("Container", () => { test("direct invocation picks up dependency overrides applied later in the chain", () => { // The dependent service is registered when `value` is 1; a later override sets it // to 2. Mirrors the equivalent get() test but goes through the factories map — - // both routes must honor the override (matching the pre-PR thisArg behavior). + // both routes must resolve through the calling container so the override is seen. const c = Container.providesValue("value", 1) .provides("svc", ["value"] as const, (v: number) => v) .providesValue("value", 2); From ea09cab36dae7be055e07727cd1eebfe4f46b93c Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Fri, 15 May 2026 17:11:48 +1000 Subject: [PATCH 12/13] Add modular multibindings: bind() + compose() for cross-module contributions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `appendValue` / `appendClass` only work when one place owns the Container chain. They don't scale when independent modules want to contribute to a shared registry (plugins, middlewares, extension points) without coupling through a single chain. Multibindings reify a contribution as a value — modules export `Multibinding`s, the wire-up site composes them. API surface: - `bind()` returns a `MultibindingBuilder` against a registry shape `S` (typically `typeof someContainer`). Methods: - `contributeValue(token, value)` — eager literal - `contributeClass(token, MyClass)` — InjectableClass with static deps - `contribute(Injectable(...))` — pre-built InjectableFunction - `withInternal(PartialContainer)` — private helpers not exposed on the composed container's type - `compose(core, ...bindings)` applies the bindings in order. A binding's phantom `D` carries the union of its contributions' deps (minus what `withInternal` provides); `compose` rejects calls where the core doesn't satisfy `D`, surfacing the missing keys as a `missingDeps` field on the offending binding rather than a generic type mismatch. Implementation only uses public Container/PartialContainer APIs, so all the perf-branch wins (O(1) chain extensions, prototype-rooted factories, chainedForEach traversal, lazy flat-factories snapshot) flow through automatically. README gets a "Modular Multibindings" section: motivation, four-file plugin example (registry / auth / metrics / wire-up), `withInternal` privacy semantics with a worked dep-flow rule, and a when-to-use-which table. "Key Concepts" picks up a `Multibinding` entry. Tests + verification: - 14 new tests, 100% coverage on Multibinding.ts. - 4 `@ts-expect-error` guards pin missing-core-dep, missing-internal-dep, non-array-token, and element-type-mismatch. - Full suite: 114/114 pass at 100% coverage. tsc -b on cjs/esm/types and `npm run lint` are clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 128 +++++++++++++++ src/Multibinding.ts | 217 +++++++++++++++++++++++++ src/__tests__/Multibinding.spec.ts | 244 +++++++++++++++++++++++++++++ src/index.ts | 2 + 4 files changed, 591 insertions(+) create mode 100644 src/Multibinding.ts create mode 100644 src/__tests__/Multibinding.spec.ts diff --git a/README.md b/README.md index 23b999b..1f0b6e0 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,133 @@ const container = Container.providesValue("plugins", [] as Plugin[]) container.get("plugins"); // [AuthPlugin, LoggingPlugin, { name: "inline", ... }] ``` +#### Modular Multibindings: contributing across module boundaries + +`appendClass` / `appendValue` work when one place owns the whole `Container` chain. They break down when you want **independent modules to contribute to the same registry without seeing each other** — the canonical plugin/middleware/extension shape. `ts-inject` solves this with `bind()` and `compose()`: + +- `bind()` starts a self-contained contribution against a registry shape (typically `typeof someContainer`, but a plain type alias works too). +- `.contributeValue` / `.contributeClass` / `.contribute` add entries to one of the registry's array tokens; dependencies declared by a class or `Injectable()` flow into the binding's phantom dependency type. +- `.build()` produces a `Multibinding` — a portable value modules can export. +- `compose(core, ...bindings)` applies every binding in order, and the compiler verifies each binding's deps are present in `core`. Missing deps surface as a `missingDeps` field on the offending binding in the type error. + +A typical layout: + +```ts +// registry.ts — one place declares the shape of the extension points. +import { Container } from "@snap/ts-inject"; + +export interface Plugin { name: string; run(): void } + +export const registry = Container + .providesValue("plugins", [] as Plugin[]) + .providesValue("middlewares", [] as ((req: Request) => Request)[]); + +export type Registry = typeof registry extends import("@snap/ts-inject").Container ? S : never; +``` + +```ts +// auth/binding.ts — a module exports its contribution as a value. +import { bind } from "@snap/ts-inject"; +import type { Registry } from "../registry"; + +class AuthPlugin { + static dependencies = ["apiKey"] as const; + readonly name = "auth"; + constructor(private apiKey: string) {} + run() { /* ... */ } +} + +export const authBinding = bind() + .contributeClass("plugins", AuthPlugin) // requires `apiKey` from the core + .build(); +``` + +```ts +// metrics/binding.ts — pre-built Injectable() works the same way. +import { Injectable, bind } from "@snap/ts-inject"; +import type { Registry } from "../registry"; + +const metricsPlugin = Injectable( + "plugins", + ["statsPrefix", "logger"] as const, + (prefix: string, logger: Logger): Plugin => ({ + name: "metrics", + run: () => logger.info(`${prefix}.requests`), + }) +); + +export const metricsBinding = bind().contribute(metricsPlugin).build(); +``` + +```ts +// app.ts — wire-up site composes the core with every contribution. +import { compose } from "@snap/ts-inject"; +import { registry } from "./registry"; +import { authBinding } from "./auth/binding"; +import { metricsBinding } from "./metrics/binding"; + +const core = registry + .providesValue("apiKey", process.env.API_KEY!) + .providesValue("statsPrefix", "app") + .providesClass("logger", ConsoleLogger); + +const app = compose(core, authBinding, metricsBinding); +app.get("plugins"); // [AuthPlugin instance, metrics plugin] +``` + +If `metricsBinding` needs `statsPrefix` and the core doesn't provide it, `compose` rejects the call at compile time — the error names the missing key, not just "type mismatch". + +##### Private services with `withInternal` + +A binding's contribution often needs a helper that isn't a registry entry — say, an `HttpPlugin` that takes a `RetryPolicy`. You have three options: + +1. **Hard-code it** (`new RetryPolicy(3)` in the constructor). No DI, no config, no test seam. +2. **Add `retryPolicy` to the core container.** Now every other binding can see it, the core's service type lists it, and you've leaked one plugin's implementation detail into the global namespace. +3. **`withInternal`.** Attach a small `PartialContainer` of helpers that *only this binding's contributions* can see. The helper is properly DI-wired, but invisible to other bindings and to consumers of the composed container. + +```ts +import { bind, PartialContainer } from "@snap/ts-inject"; + +const internal = new PartialContainer({}).provides( + "retryPolicy", + ["maxRetries"] as const, + (n: number) => new RetryPolicy(n) +); + +class HttpPlugin { + static dependencies = ["retryPolicy", "endpoint"] as const; + readonly name = "http"; + constructor(private retry: RetryPolicy, private endpoint: string) {} + run() { /* ... */ } +} + +export const httpBinding = bind() + .withInternal(internal) + .contributeClass("plugins", HttpPlugin) + .build(); +``` + +**Dep-flow rule:** a binding requires whatever its contributions declare as deps, **minus** what `withInternal` provides, **plus** what `withInternal` itself depends on but doesn't provide. Above: + +- `HttpPlugin` declares `["retryPolicy", "endpoint"]`. +- `withInternal` provides `retryPolicy`, and itself needs `maxRetries`. +- → `compose(core, httpBinding)` requires `{ endpoint, maxRetries }` from the core. `retryPolicy` is satisfied internally and doesn't appear. + +The privacy is enforced on both axes: `compose`'s return type doesn't include `retryPolicy` (so `app.get("retryPolicy")` is a type error), and the helper is only resolvable inside this binding's apply step at runtime. + +Chain `.withInternal(...)` **before** the contributions that depend on its services — the dep subtraction only sees what's been declared so far. + +##### When to use which + +| You want to… | Use | +| --------------------------------------------------------------------------- | ----------------------------------------- | +| Append to an array in a `Container` you own end-to-end | `container.appendValue` / `appendClass` | +| Let several modules independently extend a shared registry | `bind()` + `compose()` | +| Contribute a value with no DI | `.contributeValue(token, value)` | +| Contribute a class whose deps live in the core | `.contributeClass(token, MyClass)` | +| Contribute a custom factory (closes over state, composes services manually) | `.contribute(Injectable(...))` | +| Inject a helper that's an implementation detail of one binding | `.withInternal(partialContainer)` | + ### Key Concepts - **Container**: A registry for all services, handling their creation and retrieval. @@ -115,6 +242,7 @@ container.get("plugins"); // [AuthPlugin, LoggingPlugin, { name: "inline", ... } - **Token**: A unique identifier for each service, used for registration and retrieval within the Container. - **InjectableClass**: Classes that can be instantiated by the Container. Dependencies are specified in a static `dependencies` field to enable automatic injection via `providesClass`. - **InjectableFunction**: A reusable factory object created by `Injectable()`. Rarely needed directly — prefer the inline `provides('token', factory)` form. Use `Injectable()` when you need to store or pass a factory to `run()`. +- **Multibinding**: A portable, type-branded contribution to a registry container's array-typed tokens, produced by `bind()` and applied via `compose()`. Lets independent modules extend a shared registry without sharing a Container chain. ### API Reference diff --git a/src/Multibinding.ts b/src/Multibinding.ts new file mode 100644 index 0000000..b962f17 --- /dev/null +++ b/src/Multibinding.ts @@ -0,0 +1,217 @@ +import type { Container } from "./Container"; +import type { PartialContainer } from "./PartialContainer"; +import type { InjectableClass, InjectableFunction } from "./types"; + +type ElementOf = T extends readonly (infer E)[] ? E : never; + +/** Tokens of `S` whose service type is a readonly array — the only tokens a multibinding can contribute to. */ +type ArrayTokens = { [K in keyof S]: S[K] extends readonly unknown[] ? K : never }[keyof S]; + +type DepsForClass = C extends { readonly dependencies: readonly (infer K extends string)[] } + ? Record + : {}; + +type DepsForInjectable = F extends InjectableFunction + ? Tokens extends readonly (infer K extends string)[] + ? Record + : {} + : {}; + +type ServicesOf = I extends PartialContainer ? S : never; +type InternalDeps = I extends PartialContainer ? D : never; + +/** + * A reified, type-branded contribution to a registry shape `S`, requiring extra dependencies `D` + * from the core container at compose time. + * + * Multibindings are produced by {@link bind} and applied via {@link compose}. They let separate + * modules contribute to the same array-typed registry tokens (e.g. `plugins`, `middlewares`) + * without sharing a Container chain — the contributions are values that can be exported, + * imported, and composed in one place. + * + * `D` is phantom: it carries the union of dependency keys the binding's contributions need, + * so `compose` can verify the core container satisfies them. + */ +export type Multibinding = ((core: Container) => Container) & { + readonly __deps?: D; +}; + +/** + * Type-level validator for `compose`: passes a binding through unchanged if its phantom deps + * are satisfied by the core's services, otherwise tags it with a `missingDeps` field naming + * the keys it needs. + */ +type Validated = Mb extends Multibinding + ? unknown extends D + ? Mb + : keyof D extends keyof Core + ? Mb + : Mb & { readonly missingDeps: Exclude } + : never; + +/** + * Builder for a single {@link Multibinding}. Tracks four type parameters: + * - `S` — the registry shape (phantom; usually `typeof registryContainer`). + * - `Internal` — services brought along via {@link MultibindingBuilder.withInternal}, used to + * satisfy contribution dependencies without forwarding them to the core container. + * - `IDeps` — unresolved dependencies of the internal `PartialContainer`, which *do* flow + * to the core. + * - `Deps` — dependencies of class/injectable contributions that aren't satisfied by + * `Internal`, which also flow to the core. + * + * The final {@link Multibinding} aggregates `IDeps & Deps` as its phantom `D`. + */ +export class MultibindingBuilder { + constructor(private readonly apply: (core: Container) => Container) {} + + /** + * Contribute a literal value to one of the registry's array tokens. + * + * @example + * ```ts + * bind().contributeValue("plugins", inlinePlugin).build(); + * ``` + */ + contributeValue>( + token: T, + value: ElementOf + ): MultibindingBuilder { + return new MultibindingBuilder((c) => + this.apply(c).appendValue(token as never, value as never) + ); + } + + /** + * Contribute an instance of a class to one of the registry's array tokens. The class's + * `static dependencies` are added to the binding's required deps and validated at + * {@link compose} time — dependencies already provided by a {@link withInternal} are subtracted. + * + * @example + * ```ts + * class AuthPlugin { + * static dependencies = ["config"] as const; + * constructor(private config: Config) {} + * } + * + * bind().contributeClass("plugins", AuthPlugin).build(); + * ``` + */ + contributeClass< + T extends ArrayTokens, + Class extends InjectableClass, readonly string[]>, + >( + token: T, + cls: Class + ): MultibindingBuilder, keyof Internal>> { + return new MultibindingBuilder, keyof Internal>>( + (c) => this.apply(c).appendClass(token as never, cls as never) + ); + } + + /** + * Contribute a pre-built {@link InjectableFunction} to a registry token. The injectable's + * own token determines which array it contributes to, and its declared `dependencies` are + * added to the binding's required deps (minus anything already provided by {@link withInternal}). + * + * Use this when a contribution needs a non-trivial factory — e.g. one that closes over + * configuration or composes other services — but doesn't fit the class shape. + * + * @example + * ```ts + * import { Injectable } from "@snap/ts-inject"; + * + * const metricsPlugin = Injectable( + * "plugins", + * ["config", "logger"] as const, + * (config: Config, logger: Logger) => new MetricsPlugin(config, logger) + * ); + * + * bind().contribute(metricsPlugin).build(); + * ``` + */ + contribute< + T extends ArrayTokens, + F extends InjectableFunction>, + >( + fn: F + ): MultibindingBuilder, keyof Internal>> { + return new MultibindingBuilder, keyof Internal>>( + (c) => this.apply(c).append(fn as never) + ); + } + + /** + * Attach a {@link PartialContainer} of private services that subsequent contributions in this + * binding can depend on. The partial's *unresolved* dependencies become required deps of the + * binding; its provided services are visible to later `contributeClass` / `contribute` calls + * but not exposed outside the binding's runtime scope. + * + * Chain `withInternal` **before** the contributions that depend on its services so the + * compile-time dep subtraction has the necessary information. + * + * @example + * ```ts + * const internal = new PartialContainer({}) + * .provides("retryPolicy", ["config"] as const, (c: Config) => new RetryPolicy(c)); + * + * bind() + * .withInternal(internal) + * .contributeClass("plugins", HttpPlugin) // depends on retryPolicy + anything in core + * .build(); + * ``` + */ + withInternal>( + internal: I + ): MultibindingBuilder, IDeps & InternalDeps, Omit>> { + return new MultibindingBuilder< + S, + Internal & ServicesOf, + IDeps & InternalDeps, + Omit> + >((c) => this.apply(c.provides(internal) as Container)); + } + + /** Finalize this builder into a portable {@link Multibinding} value. */ + build(): Multibinding { + return this.apply as Multibinding; + } +} + +/** + * Start a multibinding against a registry shape `S`. + * + * `S` is a phantom — pass `typeof registry` when you already have a Container that declares the + * array tokens, or a hand-written shape (e.g. `{ plugins: Plugin[] }`) when you don't yet. + * + * @example + * ```ts + * type Registry = { plugins: Plugin[]; middlewares: Middleware[] }; + * + * export const authBinding = bind() + * .contributeClass("plugins", AuthPlugin) + * .build(); + * ``` + */ +export function bind(): MultibindingBuilder { + return new MultibindingBuilder((c) => c); +} + +/** + * Apply a list of {@link Multibinding}s to a core container in order, returning a container of + * the same type. The compiler verifies that every binding's phantom dependencies are present in + * the core; missing deps appear as a `missingDeps` field on the offending binding in the error. + * + * @example + * ```ts + * const app = compose(core, authBinding, loggingBinding, metricsBinding); + * ``` + */ +export function compose[]>( + core: Container, + ...bindings: { [I in keyof Mbs]: Validated } +): Container { + return (bindings as readonly Multibinding[]).reduce>( + (c, mb) => (mb as (c: Container) => Container)(c), + core + ) as Container; +} diff --git a/src/__tests__/Multibinding.spec.ts b/src/__tests__/Multibinding.spec.ts new file mode 100644 index 0000000..2158b1f --- /dev/null +++ b/src/__tests__/Multibinding.spec.ts @@ -0,0 +1,244 @@ +/* eslint-disable max-classes-per-file */ +import { Container } from "../Container"; +import { Injectable } from "../Injectable"; +import { PartialContainer } from "../PartialContainer"; +import { bind, compose } from "../Multibinding"; +import type { Multibinding } from "../Multibinding"; + +interface Plugin { + name: string; + run(): string; +} + +type Registry = { + plugins: Plugin[]; + middlewares: ((req: string) => string)[]; +}; + +describe("Multibinding", () => { + describe("bind().contributeValue", () => { + test("appends a literal value to a registry token", () => { + const inline: Plugin = { name: "inline", run: () => "inline" }; + const binding = bind().contributeValue("plugins", inline).build(); + + const core = Container.providesValue("plugins", [] as Plugin[]); + const result = compose(core, binding); + + expect(result.get("plugins")).toEqual([inline]); + }); + + test("multiple contributeValue calls preserve insertion order", () => { + const a: Plugin = { name: "a", run: () => "a" }; + const b: Plugin = { name: "b", run: () => "b" }; + const binding = bind() + .contributeValue("plugins", a) + .contributeValue("plugins", b) + .build(); + + const core = Container.providesValue("plugins", [] as Plugin[]); + expect(compose(core, binding).get("plugins")).toEqual([a, b]); + }); + }); + + describe("bind().contributeClass", () => { + test("instantiates the class with deps resolved from the core container", () => { + class AuthPlugin implements Plugin { + static dependencies = ["apiKey"] as const; + readonly name = "auth"; + constructor(private apiKey: string) {} + run() { + return `auth:${this.apiKey}`; + } + } + + const binding = bind().contributeClass("plugins", AuthPlugin).build(); + const core = Container.providesValue("apiKey", "secret").providesValue("plugins", [] as Plugin[]); + const result = compose(core, binding); + + expect(result.get("plugins").map((p) => p.run())).toEqual(["auth:secret"]); + }); + + test("compose reports missing deps as a type error", () => { + class NeedsConfig implements Plugin { + static dependencies = ["config"] as const; + readonly name = "needs-config"; + constructor(private config: { url: string }) {} + run() { + return this.config.url; + } + } + + const binding = bind().contributeClass("plugins", NeedsConfig).build(); + const core = Container.providesValue("plugins", [] as Plugin[]); + + // @ts-expect-error: core does not provide "config" + compose(core, binding); + }); + }); + + describe("bind().contribute(Injectable)", () => { + test("appends a pre-built InjectableFunction, resolving its deps from the core", () => { + const metricsPlugin = Injectable( + "plugins", + ["statsPrefix"] as const, + (prefix: string): Plugin => ({ name: "metrics", run: () => `${prefix}.requests` }) + ); + + const binding = bind().contribute(metricsPlugin).build(); + const core = Container.providesValue("statsPrefix", "app").providesValue("plugins", [] as Plugin[]); + + expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["app.requests"]); + }); + + test("zero-dep Injectable", () => { + const ping = Injectable("plugins", (): Plugin => ({ name: "ping", run: () => "pong" })); + const binding = bind().contribute(ping).build(); + const core = Container.providesValue("plugins", [] as Plugin[]); + + expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["pong"]); + }); + }); + + describe("bind().withInternal", () => { + test("subtracts services provided by the partial from the required deps", () => { + class HttpPlugin implements Plugin { + static dependencies = ["retryPolicy", "endpoint"] as const; + readonly name = "http"; + constructor(private retry: { tries: number }, private endpoint: string) {} + run() { + return `${this.endpoint}#${this.retry.tries}`; + } + } + + const internal = new PartialContainer({}).provides( + "retryPolicy", + ["maxRetries"] as const, + (n: number) => ({ tries: n }) + ); + + // `retryPolicy` is satisfied by the internal partial; `endpoint` and `maxRetries` + // must come from the core container. + const binding = bind().withInternal(internal).contributeClass("plugins", HttpPlugin).build(); + const core = Container.providesValue("endpoint", "https://api.example.com") + .providesValue("maxRetries", 3) + .providesValue("plugins", [] as Plugin[]); + + expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual([ + "https://api.example.com#3", + ]); + }); + + test("compose still flags unresolved internal dependencies", () => { + class HttpPlugin implements Plugin { + static dependencies = ["retryPolicy"] as const; + readonly name = "http"; + constructor(private retry: { tries: number }) {} + run() { + return `tries:${this.retry.tries}`; + } + } + const internal = new PartialContainer({}).provides( + "retryPolicy", + ["maxRetries"] as const, + (n: number) => ({ tries: n }) + ); + const binding = bind().withInternal(internal).contributeClass("plugins", HttpPlugin).build(); + + const core = Container.providesValue("plugins", [] as Plugin[]); + // @ts-expect-error: core is missing "maxRetries", which the internal partial needs + compose(core, binding); + }); + }); + + describe("compose", () => { + test("applies multibindings left-to-right against a shared registry", () => { + const inline: Plugin = { name: "inline", run: () => "inline" }; + const first = bind().contributeValue("plugins", inline).build(); + + class Logging implements Plugin { + static dependencies = ["label"] as const; + readonly name = "logging"; + constructor(private label: string) {} + run() { + return `log:${this.label}`; + } + } + const second = bind().contributeClass("plugins", Logging).build(); + + const core = Container.providesValue("label", "dev").providesValue("plugins", [] as Plugin[]); + const result = compose(core, first, second); + + expect(result.get("plugins").map((p) => p.run())).toEqual(["inline", "log:dev"]); + }); + + test("returns a Container of the original Core type — internal services do not leak into types", () => { + const internal = new PartialContainer({}).providesValue("secret", "hidden"); + const binding = bind() + .withInternal(internal) + .contributeValue("plugins", { name: "x", run: () => "x" }) + .build(); + + const core = Container.providesValue("plugins", [] as Plugin[]); + const result = compose(core, binding); + + // `secret` is reachable at runtime via PartialContainer's merge, but not in the type. + // @ts-expect-error: "secret" is not part of Core's service surface + result.get("secret"); + }); + + test("contributions to different array tokens compose independently", () => { + const upper: (req: string) => string = (r) => r.toUpperCase(); + const trim: (req: string) => string = (r) => r.trim(); + + const pluginBinding = bind() + .contributeValue("plugins", { name: "noop", run: () => "noop" }) + .build(); + const mwBinding = bind() + .contributeValue("middlewares", upper) + .contributeValue("middlewares", trim) + .build(); + + const core = Container.providesValue("plugins", [] as Plugin[]).providesValue( + "middlewares", + [] as ((req: string) => string)[] + ); + const result = compose(core, pluginBinding, mwBinding); + + expect(result.get("plugins")).toHaveLength(1); + expect(result.get("middlewares").map((m) => m(" hi "))).toEqual([" HI ", "hi"]); + }); + + test("the same binding can be applied to multiple cores", () => { + class Counter implements Plugin { + static dependencies = ["base"] as const; + readonly name = "counter"; + constructor(private base: number) {} + run() { + return `${this.base + 1}`; + } + } + const binding: Multibinding = bind() + .contributeClass("plugins", Counter) + .build(); + + const coreA = Container.providesValue("base", 10).providesValue("plugins", [] as Plugin[]); + const coreB = Container.providesValue("base", 100).providesValue("plugins", [] as Plugin[]); + + expect(compose(coreA, binding).get("plugins")[0].run()).toBe("11"); + expect(compose(coreB, binding).get("plugins")[0].run()).toBe("101"); + }); + }); + + describe("type-level guards", () => { + test("contributeValue rejects non-array tokens", () => { + type Bad = { single: Plugin }; + // @ts-expect-error: "single" is not an array-typed token + bind().contributeValue("single", { name: "x", run: () => "x" }); + }); + + test("contributeValue rejects values whose type does not match the array element", () => { + // @ts-expect-error: number is not assignable to Plugin + bind().contributeValue("plugins", 42); + }); + }); +}); diff --git a/src/index.ts b/src/index.ts index d17449d..47929dc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,6 @@ export { CONTAINER, Container } from "./Container"; export { Injectable, InjectableCompat, ConcatInjectable } from "./Injectable"; export { PartialContainer } from "./PartialContainer"; +export { bind, compose, MultibindingBuilder } from "./Multibinding"; +export type { Multibinding } from "./Multibinding"; export { InjectableFunction, InjectableClass, ServicesFromInjectables } from "./types"; From 8c529271bd5e523aab5323a3d4c9b2ff16bd773a Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Mon, 18 May 2026 18:12:26 +1000 Subject: [PATCH 13/13] Replace MultibindingBuilder with multibindings()/combine()/withInternal() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swap the fluent builder for a smaller value-oriented API: multibindings(registry) — factory that captures the registry shape. Has `.contribute(token, value | class)` and `.contribute(injectable)` overloads. Also callable as `multibindings()` when only a type is available. combine(...mbs) — bundle several Multibindings into one; deps union, applied left-to-right. withInternal(partial, ...mbs) — non-positional private helpers; partial's services are subtracted from every binding's deps regardless of argument order. compose(core, ...mbs) — unchanged. The `Multibinding` brand and `compose`'s `missingDeps` error semantics are preserved. Why the swap: - Single-contribution modules (the common case) collapse from `bind().contributeClass("plugins", X).build()` to `m.contribute("plugins", X)` — one verb, no builder type to learn. - `withInternal` becomes non-positional. Previously, dep subtraction only saw contributions chained after the `.withInternal(...)` call, so swapping the order silently produced a binding with wrong deps. The new shape `withInternal(partial, mb1, mb2, ...)` subtracts over the union of every binding inside the call. - Class vs Injectable contribute through one overloaded method that dispatches at runtime on arg shape, rather than separate `contributeClass` / `contribute` methods. - Builder removed entirely — `multibindings()` returns a plain object with one method. No `MultibindingBuilder` class, no per-`.contributeX` allocation. Why the factory rather than a free `contribute(...)`: TypeScript doesn't allow partial type-arg application — `contribute(token, X)` would require providing every type param positionally, and adding defaults masks the inference of `Class` from the runtime arg (the default fires before inference, widening `D` to `Record`). Routing through `multibindings(registry)` (or `multibindings()`) captures `S` once and lets the remaining type params infer normally from runtime args. Tests + verification: - 19 tests, 100% coverage on Multibinding.ts. - Four `@ts-expect-error` guards (missing core dep, missing internal dep, non-array token, element-type mismatch, plus a Multibinding annotation round-trip). - Full suite: 119/119 pass at 100% coverage. `npm run lint` and `tsc -b` clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 80 +++++---- src/Multibinding.ts | 274 +++++++++++++++-------------- src/__tests__/Multibinding.spec.ts | 205 ++++++++++++++------- src/index.ts | 4 +- 4 files changed, 332 insertions(+), 231 deletions(-) diff --git a/README.md b/README.md index 1f0b6e0..48b2319 100644 --- a/README.md +++ b/README.md @@ -109,18 +109,18 @@ container.get("plugins"); // [AuthPlugin, LoggingPlugin, { name: "inline", ... } #### Modular Multibindings: contributing across module boundaries -`appendClass` / `appendValue` work when one place owns the whole `Container` chain. They break down when you want **independent modules to contribute to the same registry without seeing each other** — the canonical plugin/middleware/extension shape. `ts-inject` solves this with `bind()` and `compose()`: +`appendClass` / `appendValue` work when one place owns the whole `Container` chain. They break down when you want **independent modules to contribute to the same registry without seeing each other** — the canonical plugin/middleware/extension shape. `ts-inject` solves this with `multibindings()` and `compose()`: -- `bind()` starts a self-contained contribution against a registry shape (typically `typeof someContainer`, but a plain type alias works too). -- `.contributeValue` / `.contributeClass` / `.contribute` add entries to one of the registry's array tokens; dependencies declared by a class or `Injectable()` flow into the binding's phantom dependency type. -- `.build()` produces a `Multibinding` — a portable value modules can export. -- `compose(core, ...bindings)` applies every binding in order, and the compiler verifies each binding's deps are present in `core`. Missing deps surface as a `missingDeps` field on the offending binding in the type error. +- `multibindings(registry)` (or `multibindings()` if you only have a type) returns a small factory bound to the registry shape. Its `contribute(...)` method produces a `Multibinding` — a portable value modules can export. +- `compose(core, ...bindings)` applies every binding to a core container in order. The compiler verifies each binding's `Deps` are present in `core`; missing keys surface as a `missingDeps` field on the offending binding rather than as a generic type mismatch. +- `combine(...bindings)` bundles several `Multibinding`s into one — handy when a module ships a small family of contributions. +- `withInternal(partialContainer, ...bindings)` attaches private helpers visible only to the contributions inside the call. A typical layout: ```ts // registry.ts — one place declares the shape of the extension points. -import { Container } from "@snap/ts-inject"; +import { Container, multibindings } from "@snap/ts-inject"; export interface Plugin { name: string; run(): void } @@ -128,13 +128,13 @@ export const registry = Container .providesValue("plugins", [] as Plugin[]) .providesValue("middlewares", [] as ((req: Request) => Request)[]); -export type Registry = typeof registry extends import("@snap/ts-inject").Container ? S : never; +// Shared factory bound to the registry shape — modules import this. +export const m = multibindings(registry); ``` ```ts // auth/binding.ts — a module exports its contribution as a value. -import { bind } from "@snap/ts-inject"; -import type { Registry } from "../registry"; +import { m } from "../registry"; class AuthPlugin { static dependencies = ["apiKey"] as const; @@ -143,15 +143,13 @@ class AuthPlugin { run() { /* ... */ } } -export const authBinding = bind() - .contributeClass("plugins", AuthPlugin) // requires `apiKey` from the core - .build(); +export const authBinding = m.contribute("plugins", AuthPlugin); // requires `apiKey` from core ``` ```ts // metrics/binding.ts — pre-built Injectable() works the same way. -import { Injectable, bind } from "@snap/ts-inject"; -import type { Registry } from "../registry"; +import { Injectable } from "@snap/ts-inject"; +import { m } from "../registry"; const metricsPlugin = Injectable( "plugins", @@ -162,7 +160,7 @@ const metricsPlugin = Injectable( }) ); -export const metricsBinding = bind().contribute(metricsPlugin).build(); +export const metricsBinding = m.contribute(metricsPlugin); ``` ```ts @@ -183,6 +181,23 @@ app.get("plugins"); // [AuthPlugin instance, metrics plugin] If `metricsBinding` needs `statsPrefix` and the core doesn't provide it, `compose` rejects the call at compile time — the error names the missing key, not just "type mismatch". +##### Bundling multiple contributions: `combine` + +When one module wants to export several related contributions, wrap them in `combine` instead of exporting each separately: + +```ts +import { combine } from "@snap/ts-inject"; +import { m } from "../registry"; + +export const authBindings = combine( + m.contribute("plugins", AuthPlugin), + m.contribute("plugins", OAuthPlugin), + m.contribute(authMetricsInjectable), +); +``` + +The result is a single `Multibinding` whose deps are the union of the inputs' deps. Wire-up still passes it as one argument to `compose`. + ##### Private services with `withInternal` A binding's contribution often needs a helper that isn't a registry entry — say, an `HttpPlugin` that takes a `RetryPolicy`. You have three options: @@ -192,9 +207,10 @@ A binding's contribution often needs a helper that isn't a registry entry — sa 3. **`withInternal`.** Attach a small `PartialContainer` of helpers that *only this binding's contributions* can see. The helper is properly DI-wired, but invisible to other bindings and to consumers of the composed container. ```ts -import { bind, PartialContainer } from "@snap/ts-inject"; +import { PartialContainer, withInternal } from "@snap/ts-inject"; +import { m } from "../registry"; -const internal = new PartialContainer({}).provides( +const retryInternal = new PartialContainer({}).provides( "retryPolicy", ["maxRetries"] as const, (n: number) => new RetryPolicy(n) @@ -207,32 +223,32 @@ class HttpPlugin { run() { /* ... */ } } -export const httpBinding = bind() - .withInternal(internal) - .contributeClass("plugins", HttpPlugin) - .build(); +export const httpBinding = withInternal(retryInternal, + m.contribute("plugins", HttpPlugin), +); ``` **Dep-flow rule:** a binding requires whatever its contributions declare as deps, **minus** what `withInternal` provides, **plus** what `withInternal` itself depends on but doesn't provide. Above: - `HttpPlugin` declares `["retryPolicy", "endpoint"]`. -- `withInternal` provides `retryPolicy`, and itself needs `maxRetries`. +- `retryInternal` provides `retryPolicy` and itself needs `maxRetries`. - → `compose(core, httpBinding)` requires `{ endpoint, maxRetries }` from the core. `retryPolicy` is satisfied internally and doesn't appear. The privacy is enforced on both axes: `compose`'s return type doesn't include `retryPolicy` (so `app.get("retryPolicy")` is a type error), and the helper is only resolvable inside this binding's apply step at runtime. -Chain `.withInternal(...)` **before** the contributions that depend on its services — the dep subtraction only sees what's been declared so far. +Subtraction is non-positional — every binding inside the `withInternal(...)` call sees the partial, regardless of argument order. ##### When to use which -| You want to… | Use | -| --------------------------------------------------------------------------- | ----------------------------------------- | -| Append to an array in a `Container` you own end-to-end | `container.appendValue` / `appendClass` | -| Let several modules independently extend a shared registry | `bind()` + `compose()` | -| Contribute a value with no DI | `.contributeValue(token, value)` | -| Contribute a class whose deps live in the core | `.contributeClass(token, MyClass)` | -| Contribute a custom factory (closes over state, composes services manually) | `.contribute(Injectable(...))` | -| Inject a helper that's an implementation detail of one binding | `.withInternal(partialContainer)` | +| You want to… | Use | +| --------------------------------------------------------------------------- | ------------------------------------------------ | +| Append to an array in a `Container` you own end-to-end | `container.appendValue` / `appendClass` | +| Let several modules independently extend a shared registry | `multibindings(registry)` + `compose(core, ...)` | +| Contribute a value with no DI | `m.contribute(token, value)` | +| Contribute a class whose deps live in the core | `m.contribute(token, MyClass)` | +| Contribute a custom factory (closes over state, composes services manually) | `m.contribute(Injectable(...))` | +| Bundle several contributions from one module | `combine(mb1, mb2, …)` | +| Inject a helper that's an implementation detail of one binding | `withInternal(partial, mb1, mb2, …)` | ### Key Concepts @@ -242,7 +258,7 @@ Chain `.withInternal(...)` **before** the contributions that depend on its servi - **Token**: A unique identifier for each service, used for registration and retrieval within the Container. - **InjectableClass**: Classes that can be instantiated by the Container. Dependencies are specified in a static `dependencies` field to enable automatic injection via `providesClass`. - **InjectableFunction**: A reusable factory object created by `Injectable()`. Rarely needed directly — prefer the inline `provides('token', factory)` form. Use `Injectable()` when you need to store or pass a factory to `run()`. -- **Multibinding**: A portable, type-branded contribution to a registry container's array-typed tokens, produced by `bind()` and applied via `compose()`. Lets independent modules extend a shared registry without sharing a Container chain. +- **Multibinding**: A portable, type-branded contribution to a registry container's array-typed tokens, produced by `multibindings(registry).contribute(...)` and applied via `compose()`. Lets independent modules extend a shared registry without sharing a Container chain. ### API Reference diff --git a/src/Multibinding.ts b/src/Multibinding.ts index b962f17..0014918 100644 --- a/src/Multibinding.ts +++ b/src/Multibinding.ts @@ -20,26 +20,30 @@ type DepsForInjectable = F extends InjectableFunction = I extends PartialContainer ? S : never; type InternalDeps = I extends PartialContainer ? D : never; +/** Aggregate the phantom `D` of a tuple of Multibindings into a single dependency record. */ +type UnionDeps = Mbs extends readonly [infer H, ...infer R] + ? (H extends Multibinding ? D : {}) & UnionDeps + : {}; + /** * A reified, type-branded contribution to a registry shape `S`, requiring extra dependencies `D` * from the core container at compose time. * - * Multibindings are produced by {@link bind} and applied via {@link compose}. They let separate - * modules contribute to the same array-typed registry tokens (e.g. `plugins`, `middlewares`) - * without sharing a Container chain — the contributions are values that can be exported, - * imported, and composed in one place. + * Multibindings are values that can be exported, imported, combined, and finally applied with + * {@link compose}. They let separate modules contribute to the same array-typed registry tokens + * (e.g. `plugins`, `middlewares`) without sharing a Container chain. * - * `D` is phantom: it carries the union of dependency keys the binding's contributions need, - * so `compose` can verify the core container satisfies them. + * `D` is phantom: it carries the union of dependency keys the contribution needs, so `compose` + * can verify the core container satisfies them. */ export type Multibinding = ((core: Container) => Container) & { readonly __deps?: D; }; /** - * Type-level validator for `compose`: passes a binding through unchanged if its phantom deps - * are satisfied by the core's services, otherwise tags it with a `missingDeps` field naming - * the keys it needs. + * Type-level validator for `compose`: passes a binding through unchanged if its phantom deps are + * satisfied by the core's services, otherwise tags it with a `missingDeps` field naming the keys + * it needs. */ type Validated = Mb extends Multibinding ? unknown extends D @@ -50,150 +54,150 @@ type Validated = Mb extends Multibinding : never; /** - * Builder for a single {@link Multibinding}. Tracks four type parameters: - * - `S` — the registry shape (phantom; usually `typeof registryContainer`). - * - `Internal` — services brought along via {@link MultibindingBuilder.withInternal}, used to - * satisfy contribution dependencies without forwarding them to the core container. - * - `IDeps` — unresolved dependencies of the internal `PartialContainer`, which *do* flow - * to the core. - * - `Deps` — dependencies of class/injectable contributions that aren't satisfied by - * `Internal`, which also flow to the core. + * Family of multibinding helpers bound to a specific registry shape `S`. * - * The final {@link Multibinding} aggregates `IDeps & Deps` as its phantom `D`. + * Obtained from {@link multibindings} — either by passing a Container so `S` is inferred from + * its services, or by supplying `S` as a type argument when no Container instance is available. */ -export class MultibindingBuilder { - constructor(private readonly apply: (core: Container) => Container) {} - +export interface MultibindingFactory { /** - * Contribute a literal value to one of the registry's array tokens. - * - * @example - * ```ts - * bind().contributeValue("plugins", inlinePlugin).build(); - * ``` + * Produce a {@link Multibinding}. Three call shapes: + * - `contribute(token, value)` — appends a literal value. + * - `contribute(token, ClassWithDeps)` — appends an instance built from an + * {@link InjectableClass}; its `static dependencies` become the binding's required deps. + * - `contribute(injectable)` — appends a value produced by a pre-built + * {@link InjectableFunction}; the function's `token` selects the array, and its declared + * `dependencies` become the binding's required deps. */ - contributeValue>( - token: T, - value: ElementOf - ): MultibindingBuilder { - return new MultibindingBuilder((c) => - this.apply(c).appendValue(token as never, value as never) - ); - } + contribute: { + < + T extends ArrayTokens, + F extends InjectableFunction>, + >( + injectable: F + ): Multibinding>; + < + T extends ArrayTokens, + Class extends InjectableClass, readonly string[]>, + >( + token: T, + cls: Class + ): Multibinding>; + >(token: T, value: ElementOf): Multibinding; + }; +} - /** - * Contribute an instance of a class to one of the registry's array tokens. The class's - * `static dependencies` are added to the binding's required deps and validated at - * {@link compose} time — dependencies already provided by a {@link withInternal} are subtracted. - * - * @example - * ```ts - * class AuthPlugin { - * static dependencies = ["config"] as const; - * constructor(private config: Config) {} - * } - * - * bind().contributeClass("plugins", AuthPlugin).build(); - * ``` - */ - contributeClass< - T extends ArrayTokens, - Class extends InjectableClass, readonly string[]>, - >( - token: T, - cls: Class - ): MultibindingBuilder, keyof Internal>> { - return new MultibindingBuilder, keyof Internal>>( - (c) => this.apply(c).appendClass(token as never, cls as never) - ); +function contributeImpl(first: unknown, second?: unknown): Multibinding { + // 1-arg form: contribute(injectable). The injectable carries its own token. + if (second === undefined) { + const fn = first as InjectableFunction; + return ((core: Container) => core.append(fn as never)) as Multibinding; } - - /** - * Contribute a pre-built {@link InjectableFunction} to a registry token. The injectable's - * own token determines which array it contributes to, and its declared `dependencies` are - * added to the binding's required deps (minus anything already provided by {@link withInternal}). - * - * Use this when a contribution needs a non-trivial factory — e.g. one that closes over - * configuration or composes other services — but doesn't fit the class shape. - * - * @example - * ```ts - * import { Injectable } from "@snap/ts-inject"; - * - * const metricsPlugin = Injectable( - * "plugins", - * ["config", "logger"] as const, - * (config: Config, logger: Logger) => new MetricsPlugin(config, logger) - * ); - * - * bind().contribute(metricsPlugin).build(); - * ``` - */ - contribute< - T extends ArrayTokens, - F extends InjectableFunction>, - >( - fn: F - ): MultibindingBuilder, keyof Internal>> { - return new MultibindingBuilder, keyof Internal>>( - (c) => this.apply(c).append(fn as never) - ); + const token = first as never; + // 2-arg form, class: a function carrying a `dependencies` array (Injectables also carry one, + // but those should use the 1-arg form, and would be missing the `new`-ability `appendClass` expects). + if (typeof second === "function" && Array.isArray((second as { dependencies?: unknown }).dependencies)) { + const cls = second as InjectableClass; + return ((core: Container) => core.appendClass(token, cls as never)) as Multibinding; } + // 2-arg form, value. + return ((core: Container) => core.appendValue(token, second as never)) as Multibinding; +} - /** - * Attach a {@link PartialContainer} of private services that subsequent contributions in this - * binding can depend on. The partial's *unresolved* dependencies become required deps of the - * binding; its provided services are visible to later `contributeClass` / `contribute` calls - * but not exposed outside the binding's runtime scope. - * - * Chain `withInternal` **before** the contributions that depend on its services so the - * compile-time dep subtraction has the necessary information. - * - * @example - * ```ts - * const internal = new PartialContainer({}) - * .provides("retryPolicy", ["config"] as const, (c: Config) => new RetryPolicy(c)); - * - * bind() - * .withInternal(internal) - * .contributeClass("plugins", HttpPlugin) // depends on retryPolicy + anything in core - * .build(); - * ``` - */ - withInternal>( - internal: I - ): MultibindingBuilder, IDeps & InternalDeps, Omit>> { - return new MultibindingBuilder< - S, - Internal & ServicesOf, - IDeps & InternalDeps, - Omit> - >((c) => this.apply(c.provides(internal) as Container)); - } +/** + * Capture a registry shape so subsequent contribution calls don't need to repeat it. Two forms: + * + * - `multibindings(registryContainer)` — infers `S` from the container's services. + * - `multibindings()` — when only a type alias is available. + * + * Returns a {@link MultibindingFactory} with a `contribute` method whose overloads cover values, + * {@link InjectableClass}es, and pre-built {@link InjectableFunction}s. + * + * @example + * ```ts + * // With a Container instance: + * const m = multibindings(registry); + * export const authBinding = m.contribute("plugins", AuthPlugin); + * + * // With only a type: + * const m = multibindings(); + * export const inlineBinding = m.contribute("plugins", { name: "x", run: () => "x" }); + * ``` + */ +export function multibindings(): MultibindingFactory; +export function multibindings(registry: Container): MultibindingFactory; +export function multibindings(_registry?: Container): MultibindingFactory { + return { contribute: contributeImpl as MultibindingFactory["contribute"] }; +} - /** Finalize this builder into a portable {@link Multibinding} value. */ - build(): Multibinding { - return this.apply as Multibinding; - } +/** + * Bundle several {@link Multibinding}s that target the same registry shape into one. The + * resulting binding's deps are the union of the inputs' deps and contributions are applied + * left-to-right. + * + * Use this when one module exports several related contributions and you'd rather pass a single + * value to {@link compose}. + * + * @example + * ```ts + * const m = multibindings(); + * + * export const authBindings = combine( + * m.contribute("plugins", AuthPlugin), + * m.contribute("plugins", OAuthPlugin), + * m.contribute(authMetricsInjectable), + * ); + * ``` + */ +export function combine[] = readonly []>( + ...bindings: Mbs +): Multibinding> { + return ((core: Container) => + bindings.reduce>( + (c, mb) => (mb as (c: Container) => Container)(c), + core + )) as Multibinding>; } /** - * Start a multibinding against a registry shape `S`. + * Apply contributions against a private {@link PartialContainer} of helpers that's invisible to + * other bindings and to consumers of the composed container's type. * - * `S` is a phantom — pass `typeof registry` when you already have a Container that declares the - * array tokens, or a hand-written shape (e.g. `{ plugins: Plugin[] }`) when you don't yet. + * The partial's *unresolved* dependencies flow to the core; its *provided* services are + * subtracted from the contributions' deps. Subtraction is non-positional: every binding inside + * the call sees the partial. * * @example * ```ts - * type Registry = { plugins: Plugin[]; middlewares: Middleware[] }; + * const m = multibindings(); + * const retryInternal = new PartialContainer({}).provides( + * "retryPolicy", + * ["maxRetries"] as const, + * (n: number) => new RetryPolicy(n) + * ); * - * export const authBinding = bind() - * .contributeClass("plugins", AuthPlugin) - * .build(); + * // HttpPlugin.dependencies = ["retryPolicy", "endpoint"] + * // → the composed binding requires { endpoint, maxRetries } — retryPolicy is internal. + * export const httpBinding = withInternal(retryInternal, + * m.contribute("plugins", HttpPlugin), + * ); * ``` */ -export function bind(): MultibindingBuilder { - return new MultibindingBuilder((c) => c); +export function withInternal< + S, + I extends PartialContainer, + const Mbs extends readonly Multibinding[], +>( + internal: I, + ...bindings: Mbs +): Multibinding, keyof ServicesOf> & InternalDeps> { + return ((core: Container) => { + const merged = core.provides(internal) as Container; + return bindings.reduce>( + (c, mb) => (mb as (c: Container) => Container)(c), + merged + ); + }) as Multibinding, keyof ServicesOf> & InternalDeps>; } /** @@ -203,7 +207,7 @@ export function bind(): MultibindingBuilder { * * @example * ```ts - * const app = compose(core, authBinding, loggingBinding, metricsBinding); + * const app = compose(core, authBinding, httpBinding, metricsBinding); * ``` */ export function compose[]>( diff --git a/src/__tests__/Multibinding.spec.ts b/src/__tests__/Multibinding.spec.ts index 2158b1f..4cf6400 100644 --- a/src/__tests__/Multibinding.spec.ts +++ b/src/__tests__/Multibinding.spec.ts @@ -2,7 +2,7 @@ import { Container } from "../Container"; import { Injectable } from "../Injectable"; import { PartialContainer } from "../PartialContainer"; -import { bind, compose } from "../Multibinding"; +import { combine, compose, multibindings, withInternal } from "../Multibinding"; import type { Multibinding } from "../Multibinding"; interface Plugin { @@ -15,32 +15,31 @@ type Registry = { middlewares: ((req: string) => string)[]; }; +const m = multibindings(); + describe("Multibinding", () => { - describe("bind().contributeValue", () => { + describe("multibindings(...).contribute(token, value)", () => { test("appends a literal value to a registry token", () => { const inline: Plugin = { name: "inline", run: () => "inline" }; - const binding = bind().contributeValue("plugins", inline).build(); + const binding = m.contribute("plugins", inline); const core = Container.providesValue("plugins", [] as Plugin[]); - const result = compose(core, binding); - - expect(result.get("plugins")).toEqual([inline]); + expect(compose(core, binding).get("plugins")).toEqual([inline]); }); - test("multiple contributeValue calls preserve insertion order", () => { + test("compose preserves the order in which bindings are passed", () => { const a: Plugin = { name: "a", run: () => "a" }; const b: Plugin = { name: "b", run: () => "b" }; - const binding = bind() - .contributeValue("plugins", a) - .contributeValue("plugins", b) - .build(); const core = Container.providesValue("plugins", [] as Plugin[]); - expect(compose(core, binding).get("plugins")).toEqual([a, b]); + expect(compose(core, m.contribute("plugins", a), m.contribute("plugins", b)).get("plugins")).toEqual([ + a, + b, + ]); }); }); - describe("bind().contributeClass", () => { + describe("multibindings(...).contribute(token, class)", () => { test("instantiates the class with deps resolved from the core container", () => { class AuthPlugin implements Plugin { static dependencies = ["apiKey"] as const; @@ -51,14 +50,13 @@ describe("Multibinding", () => { } } - const binding = bind().contributeClass("plugins", AuthPlugin).build(); + const binding = m.contribute("plugins", AuthPlugin); const core = Container.providesValue("apiKey", "secret").providesValue("plugins", [] as Plugin[]); - const result = compose(core, binding); - expect(result.get("plugins").map((p) => p.run())).toEqual(["auth:secret"]); + expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["auth:secret"]); }); - test("compose reports missing deps as a type error", () => { + test("compose reports missing class deps as a type error", () => { class NeedsConfig implements Plugin { static dependencies = ["config"] as const; readonly name = "needs-config"; @@ -68,7 +66,7 @@ describe("Multibinding", () => { } } - const binding = bind().contributeClass("plugins", NeedsConfig).build(); + const binding = m.contribute("plugins", NeedsConfig); const core = Container.providesValue("plugins", [] as Plugin[]); // @ts-expect-error: core does not provide "config" @@ -76,7 +74,7 @@ describe("Multibinding", () => { }); }); - describe("bind().contribute(Injectable)", () => { + describe("multibindings(...).contribute(injectable)", () => { test("appends a pre-built InjectableFunction, resolving its deps from the core", () => { const metricsPlugin = Injectable( "plugins", @@ -84,7 +82,7 @@ describe("Multibinding", () => { (prefix: string): Plugin => ({ name: "metrics", run: () => `${prefix}.requests` }) ); - const binding = bind().contribute(metricsPlugin).build(); + const binding = m.contribute(metricsPlugin); const core = Container.providesValue("statsPrefix", "app").providesValue("plugins", [] as Plugin[]); expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["app.requests"]); @@ -92,14 +90,63 @@ describe("Multibinding", () => { test("zero-dep Injectable", () => { const ping = Injectable("plugins", (): Plugin => ({ name: "ping", run: () => "pong" })); - const binding = bind().contribute(ping).build(); const core = Container.providesValue("plugins", [] as Plugin[]); - expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["pong"]); + expect(compose(core, m.contribute(ping)).get("plugins").map((p) => p.run())).toEqual(["pong"]); }); }); - describe("bind().withInternal", () => { + describe("combine", () => { + test("bundles several Multibindings into one, applied in order", () => { + const inline: Plugin = { name: "inline", run: () => "inline" }; + + class Logging implements Plugin { + static dependencies = ["label"] as const; + readonly name = "logging"; + constructor(private label: string) {} + run() { + return `log:${this.label}`; + } + } + + const bundle = combine(m.contribute("plugins", inline), m.contribute("plugins", Logging)); + + const core = Container.providesValue("label", "dev").providesValue("plugins", [] as Plugin[]); + expect(compose(core, bundle).get("plugins").map((p) => p.run())).toEqual(["inline", "log:dev"]); + }); + + test("combine() with no bindings is the identity", () => { + const core = Container.providesValue("plugins", [] as Plugin[]); + expect(compose(core, combine()).get("plugins")).toEqual([]); + }); + + test("unions deps across bundled bindings", () => { + class A implements Plugin { + static dependencies = ["depA"] as const; + readonly name = "a"; + constructor(private d: string) {} + run() { + return this.d; + } + } + class B implements Plugin { + static dependencies = ["depB"] as const; + readonly name = "b"; + constructor(private d: string) {} + run() { + return this.d; + } + } + + const bundle = combine(m.contribute("plugins", A), m.contribute("plugins", B)); + const core = Container.providesValue("depA", "x").providesValue("plugins", [] as Plugin[]); + + // @ts-expect-error: bundle requires both depA and depB; core only has depA + compose(core, bundle); + }); + }); + + describe("withInternal", () => { test("subtracts services provided by the partial from the required deps", () => { class HttpPlugin implements Plugin { static dependencies = ["retryPolicy", "endpoint"] as const; @@ -116,9 +163,7 @@ describe("Multibinding", () => { (n: number) => ({ tries: n }) ); - // `retryPolicy` is satisfied by the internal partial; `endpoint` and `maxRetries` - // must come from the core container. - const binding = bind().withInternal(internal).contributeClass("plugins", HttpPlugin).build(); + const binding = withInternal(internal, m.contribute("plugins", HttpPlugin)); const core = Container.providesValue("endpoint", "https://api.example.com") .providesValue("maxRetries", 3) .providesValue("plugins", [] as Plugin[]); @@ -128,6 +173,28 @@ describe("Multibinding", () => { ]); }); + test("internal services are visible to every binding inside the call, regardless of order", () => { + class UsesRetry implements Plugin { + static dependencies = ["retryPolicy"] as const; + readonly name = "uses-retry"; + constructor(private retry: { tries: number }) {} + run() { + return `tries:${this.retry.tries}`; + } + } + + const internal = new PartialContainer({}).provides("retryPolicy", () => ({ tries: 5 })); + + const binding = withInternal( + internal, + m.contribute("plugins", UsesRetry), + m.contribute("plugins", UsesRetry) + ); + + const core = Container.providesValue("plugins", [] as Plugin[]); + expect(compose(core, binding).get("plugins").map((p) => p.run())).toEqual(["tries:5", "tries:5"]); + }); + test("compose still flags unresolved internal dependencies", () => { class HttpPlugin implements Plugin { static dependencies = ["retryPolicy"] as const; @@ -142,7 +209,7 @@ describe("Multibinding", () => { ["maxRetries"] as const, (n: number) => ({ tries: n }) ); - const binding = bind().withInternal(internal).contributeClass("plugins", HttpPlugin).build(); + const binding = withInternal(internal, m.contribute("plugins", HttpPlugin)); const core = Container.providesValue("plugins", [] as Plugin[]); // @ts-expect-error: core is missing "maxRetries", which the internal partial needs @@ -150,38 +217,44 @@ describe("Multibinding", () => { }); }); - describe("compose", () => { - test("applies multibindings left-to-right against a shared registry", () => { - const inline: Plugin = { name: "inline", run: () => "inline" }; - const first = bind().contributeValue("plugins", inline).build(); + describe("multibindings(registry) — runtime arg form", () => { + test("binds S from a real Container without an explicit type argument", () => { + const registry = Container.providesValue("plugins", [] as Plugin[]).providesValue( + "middlewares", + [] as ((req: string) => string)[] + ); + const m2 = multibindings(registry); - class Logging implements Plugin { - static dependencies = ["label"] as const; - readonly name = "logging"; - constructor(private label: string) {} + const inline: Plugin = { name: "inline", run: () => "inline" }; + class WithDep implements Plugin { + static dependencies = ["token"] as const; + readonly name = "wd"; + constructor(private t: string) {} run() { - return `log:${this.label}`; + return this.t; } } - const second = bind().contributeClass("plugins", Logging).build(); + const fromFactory = Injectable("plugins", (): Plugin => ({ name: "factory", run: () => "factory" })); - const core = Container.providesValue("label", "dev").providesValue("plugins", [] as Plugin[]); - const result = compose(core, first, second); + const bundle = combine( + m2.contribute("plugins", inline), + m2.contribute("plugins", WithDep), + m2.contribute(fromFactory) + ); - expect(result.get("plugins").map((p) => p.run())).toEqual(["inline", "log:dev"]); + const core = registry.providesValue("token", "tok"); + expect(compose(core, bundle).get("plugins").map((p) => p.name)).toEqual(["inline", "wd", "factory"]); }); + }); + describe("compose", () => { test("returns a Container of the original Core type — internal services do not leak into types", () => { const internal = new PartialContainer({}).providesValue("secret", "hidden"); - const binding = bind() - .withInternal(internal) - .contributeValue("plugins", { name: "x", run: () => "x" }) - .build(); + const binding = withInternal(internal, m.contribute("plugins", { name: "x", run: () => "x" })); const core = Container.providesValue("plugins", [] as Plugin[]); const result = compose(core, binding); - // `secret` is reachable at runtime via PartialContainer's merge, but not in the type. // @ts-expect-error: "secret" is not part of Core's service surface result.get("secret"); }); @@ -190,22 +263,17 @@ describe("Multibinding", () => { const upper: (req: string) => string = (r) => r.toUpperCase(); const trim: (req: string) => string = (r) => r.trim(); - const pluginBinding = bind() - .contributeValue("plugins", { name: "noop", run: () => "noop" }) - .build(); - const mwBinding = bind() - .contributeValue("middlewares", upper) - .contributeValue("middlewares", trim) - .build(); - const core = Container.providesValue("plugins", [] as Plugin[]).providesValue( "middlewares", [] as ((req: string) => string)[] ); - const result = compose(core, pluginBinding, mwBinding); + const pluginBinding = m.contribute("plugins", { name: "noop", run: () => "noop" }); + const mwBindings = combine(m.contribute("middlewares", upper), m.contribute("middlewares", trim)); + + const result = compose(core, pluginBinding, mwBindings); expect(result.get("plugins")).toHaveLength(1); - expect(result.get("middlewares").map((m) => m(" hi "))).toEqual([" HI ", "hi"]); + expect(result.get("middlewares").map((mw) => mw(" hi "))).toEqual([" HI ", "hi"]); }); test("the same binding can be applied to multiple cores", () => { @@ -217,9 +285,7 @@ describe("Multibinding", () => { return `${this.base + 1}`; } } - const binding: Multibinding = bind() - .contributeClass("plugins", Counter) - .build(); + const binding = m.contribute("plugins", Counter); const coreA = Container.providesValue("base", 10).providesValue("plugins", [] as Plugin[]); const coreB = Container.providesValue("base", 100).providesValue("plugins", [] as Plugin[]); @@ -230,15 +296,30 @@ describe("Multibinding", () => { }); describe("type-level guards", () => { - test("contributeValue rejects non-array tokens", () => { + test("contribute rejects non-array tokens", () => { type Bad = { single: Plugin }; + const bad = multibindings(); // @ts-expect-error: "single" is not an array-typed token - bind().contributeValue("single", { name: "x", run: () => "x" }); + bad.contribute("single", { name: "x", run: () => "x" }); }); - test("contributeValue rejects values whose type does not match the array element", () => { + test("contribute rejects values whose type does not match the array element", () => { // @ts-expect-error: number is not assignable to Plugin - bind().contributeValue("plugins", 42); + m.contribute("plugins", 42); + }); + + test("Multibinding can be written down explicitly", () => { + class WithBase implements Plugin { + static dependencies = ["base"] as const; + readonly name = "with-base"; + constructor(private b: number) {} + run() { + return `${this.b}`; + } + } + // The annotation should not widen D beyond the binding's actual deps. + const binding: Multibinding = m.contribute("plugins", WithBase); + expect(typeof binding).toBe("function"); }); }); }); diff --git a/src/index.ts b/src/index.ts index 47929dc..054ddd5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ export { CONTAINER, Container } from "./Container"; export { Injectable, InjectableCompat, ConcatInjectable } from "./Injectable"; export { PartialContainer } from "./PartialContainer"; -export { bind, compose, MultibindingBuilder } from "./Multibinding"; -export type { Multibinding } from "./Multibinding"; +export { multibindings, combine, withInternal, compose } from "./Multibinding"; +export type { Multibinding, MultibindingFactory } from "./Multibinding"; export { InjectableFunction, InjectableClass, ServicesFromInjectables } from "./types";