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 76edc4894fd615d67cd997f8f3bd705f647f1fe7 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Wed, 20 May 2026 00:08:49 +1000 Subject: [PATCH 12/13] Bump version to 1.0.0 The Container.provides() refactor in this branch changes externally- observable behavior in two ways (parent containers stay a self-consistent snapshot after a child fork; token names that collide with Object.prototype methods no longer leak through get()). Major bump per the project's release guide. Co-Authored-By: Claude Opus 4.7 (1M context) --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index b62decd..7e2b547 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@snap/ts-inject", - "version": "0.4.0", + "version": "1.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@snap/ts-inject", - "version": "0.4.0", + "version": "1.0.0", "license": "MIT", "devDependencies": { "@types/jest": "^29.5.12", diff --git a/package.json b/package.json index d62f23a..0cd27d0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@snap/ts-inject", - "version": "0.4.0", + "version": "1.0.0", "description": "100% typesafe dependency injection framework for TypeScript projects", "license": "MIT", "author": "Snap Inc.", From 1283794bc4d6fc963109ca7be3ba32d769bcd384 Mon Sep 17 00:00:00 2001 From: Konstantin Burov Date: Wed, 20 May 2026 00:32:08 +1000 Subject: [PATCH 13/13] README: note that registration methods return a new child container Make the immutable-on-update semantic explicit so readers don't expect the original container to be mutated by `.providesValue(...)`, `.provides(...)`, etc. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 23b999b..e709530 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,10 @@ const db = appContainer.get("Database"); db.save("user2"); // Log: Saving record: user2 ``` +> **Note:** Each registration method (`provides`, `providesValue`, `providesClass`, etc.) +> returns a **new child container** — the original container is never modified. +> Always use the returned value; calls whose return value is discarded have no effect. + You can also bootstrap a container from a plain object with `fromObject`: ```ts