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/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 diff --git a/benchmarks/INVESTIGATION.md b/benchmarks/INVESTIGATION.md new file mode 100644 index 0000000..75ddd61 --- /dev/null +++ b/benchmarks/INVESTIGATION.md @@ -0,0 +1,166 @@ +# `provides()` Chain Performance Investigation + +## TL;DR + +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` / `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×). 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 +``` + +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 + +**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× | +| 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× | + +**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: ~3 ms at N=8000. + +**Hot read path** (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. + +## 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: 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()`: 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`**: + - `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 + +### First-`get` cost on each container + +`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). + +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 + +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/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/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/benchmarks/provides-chain.ts b/benchmarks/provides-chain.ts new file mode 100644 index 0000000..6998e4e --- /dev/null +++ b/benchmarks/provides-chain.ts @@ -0,0 +1,92 @@ +/** + * 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"; +import { PartialContainer } from "../src/PartialContainer"; + +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 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(); + const end = process.hrtime.bigint(); + return Number(end - start) / 1e6; +} + +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 + ); +} + +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("Container chain construction:"); +console.log("size\tms/build"); +for (const n of sizes) { + 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)}`); +} + +// 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 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/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 0c95e3e..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.", @@ -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/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" diff --git a/src/Container.ts b/src/Container.ts index 2e78b03..fa9b670 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,26 +166,50 @@ export class Container { ) as Container; } - // 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>; + /** + * 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(factoriesChain: Factories): Container { + const c = Object.create(Container.prototype) as Container; + (c as unknown as { factoriesChain: Factories }).factoriesChain = factoriesChain; + return c; + } + + // 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) { - 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); - } - } - 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; } /** @@ -222,15 +246,18 @@ 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; - }); - return new Container(factories); + 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.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.factoriesChain) as Factories; + for (const token of scopedServices) { + factories[token] = memoize(this.factoriesChain[token].delegate); + } + return Container.withMemoizedFactories(factories); } /** @@ -253,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 ` + @@ -262,7 +293,27 @@ 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); + } + + private buildFlatFactories(): Factories { + const flat = Object.create(null) as Factories; + const self = this; + chainedForEach unknown>>(this.factoriesChain, (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; } /** @@ -450,14 +501,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.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) => { + (factories as any)[k] = v; + }); + return Container.withMemoizedFactories(factories); } return this.providesService(first); } @@ -636,14 +689,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 }; - return new Container(factories) as Container>; + // 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 5f101ba..f0b40a5 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"; @@ -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. @@ -236,16 +261,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 PartialContainer.withInjectables(injectables); } // 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()); - } - return new PartialContainer({ ...this.injectables, ...containerInjectables } as any); + const sourceFactories = first.factories; + 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 PartialContainer.withInjectables(injectables); } // Original single-arg form: provides(InjectableFunction) return this.addInjectable((first as any).token, first as any); @@ -255,7 +288,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 PartialContainer.withInjectables(injectables); } /** @@ -321,26 +358,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(parent, () => - fn( - ...(fn.dependencies.map((t) => { - return t === token - ? parent.get(t as keyof Dependencies) - : factories![t as keyof Services & Dependencies] - ? factories![t]() - : parent.get(t as keyof Dependencies); - }) as any) - ) - ), - ]) - ) 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( + ...(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; } getTokens(): Array { - return Object.keys(this.injectables) as Array; + return chainedKeys(this.injectables) as Array; } } diff --git a/src/__tests__/Container.spec.ts b/src/__tests__/Container.spec.ts index f3d4aed..f68dc78 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) @@ -424,6 +476,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"); @@ -473,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", () => { @@ -489,12 +576,125 @@ describe("Container", () => { }); describe("when accessing factories", () => { + test("direct invocation of factories[token] works for services with dependencies", () => { + // `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); + }); + + 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 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); + 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. + 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"); }); }); + 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("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); + }); + }); + 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..79656ee 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,28 @@ 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/__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); + }); + }); }); diff --git a/src/entries.ts b/src/entries.ts index cbc93ea..0bd27e8 100644 --- a/src/entries.ts +++ b/src/entries.ts @@ -2,6 +2,40 @@ 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; +/** + * 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; +} 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; }