fix: resolution bugs, typed tokens, faster resolve - #126
Merged
Conversation
Correctness: - a SINGLETON/REQUEST binding producing `undefined` returned an internal symbol on the first resolve and injected it into dependents afterwards - validate/validateSafe silently aborted when a bind key had already been seen as another binding's dependency - resolving a parent owned SINGLETON started a fresh request context, which split REQUEST scope, reordered postConstruct and hid circular references - setParent accepted cycles, turning every lookup into infinite recursion - unbindAll ignored the lock on an empty container Features: - token<T>() typed bind keys, so resolve infers instead of being told - tryResolve, getKeys, and a `replace` bind option - Symbol.dispose on resolved singletons and on the container itself - every error is a PumpitError carrying an ERROR_CODE Performance (bound value ~2.5x, cached singleton ~5x, transient graph ~2.4x): - allocate request context maps lazily instead of on every resolve - parse injection metadata once at bind time - drop the write-only transient cache and the per lookup wrapper object Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`using container = new PumpIt()` threw "Container is locked" at scope exit when the container had been locked. Throwing out of a disposal is harmful in JS: an error raised by the block body gets wrapped in a SuppressedError and the original is masked. The lock guards callers editing bindings, while `using` owns the whole lifetime, so disposal now removes bindings directly. `unbindAll` still refuses a locked container. Adds tests for behaviours nothing guarded, all of which already passed: - a parent owned singleton cannot reach a child only dependency, and siblings share one instance - a throwing constructor, a missing dependency and a caught circular reference all leave the container usable and retryable - a throwing postConstruct propagates, skips remaining hooks, and leaves the singleton cached - Symbol.dispose on a factory result - replace is scoped to the container it is called on - typed tokens as optional dependencies, and resolved from a child Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI pinned pnpm 9, but `pnpm-workspace.yaml` (added in 9d792d0) only sets `allowBuilds` and has no `packages` field. pnpm 9 requires that field and bailed with "packages field missing or empty", so `pnpm store path` wrote nothing to STORE_PATH and actions/cache failed with "Input required and not supplied: path". `pnpm install` would have failed for the same reason. `allowBuilds` is a pnpm 11 setting, so CI now runs 11 to match local. The store path step also masked the error: `echo "X=$(cmd)"` exits 0 even when the substitution fails, so the step went green while exporting an empty value. Assigning first lets `set -e` fail it at the real cause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a set of resolution bugs, adds typed tokens and a few container APIs, and cuts per-resolve allocations. Ships with three changesets (one major, one minor, one patch).
Fixes
SINGLETON/REQUESTbinding whose class or factory producedundefinedreturned an internal sentinel symbol on the first resolve, then injected that symbol into every dependent afterwards.validate/validateSafesilently gave up: reaching a bind key that an earlier binding already listed as a dependency aborted the whole check, sovalidatedid not throw andvalidateSafereturnedundefined. Missing dependencies are now reported regardless of bind order, and optional ones are not reported at all.SINGLETONstarted a fresh request context. That splitREQUESTscope into two instances within a singleresolvecall, ranpostConstructbefore the outer graph finished building, and hid circular references behind a stack overflow.setParentaccepted a parent that formed a cycle, turning every lookup into infinite recursion. It now throws, and acceptsundefinedto detach.unbindAllignored the lock when the container was empty.using container = new PumpIt()threwContainer is lockedat scope exit on a locked container. Throwing out of a disposal masks the original error behind aSuppressedError, so disposal now bypasses the lock.unbindAllstill refuses a locked container.Features
token<T>()— typed bind keys.resolveinfers the result instead of being told, and bindings under the token are type checked.resolvealso infers the instance type when a class is the key.tryResolve— returnsundefinedfor an unbound key instead of throwing. A missing required dependency of a bound key still throws.getKeys— lists keys bound on the container, optionally walking the parent chain.replacebind option — rebind a key without unbinding first; the previous binding is unbound, disposing its cached singleton.Symbol.disposesupport on resolved singletons (preferred overdispose), and on the container itself.PumpitErrorcarrying a machine-readablecode, exported asERROR_CODE.Performance
Bound values ~2.5x, cached singletons ~5x, a small transient graph ~2.4x — from allocating request-context maps lazily, parsing injection metadata once at bind time, and dropping the write-only transient cache plus the per-lookup wrapper object.
Breaking changes
PumpitErroris now the base class for every container error and its constructor takes anErrorCodeinstead ofValidationError[]. Validation failures throwPumpitValidationError, which still carriesresultbut whose message lists the unresolved keys instead of the literal string"Validation".validateSafealways returns aValidationResult, neverundefined.registerInjections(or assigninginject/INJECT_KEY) must happen beforebindClass/bindFactory. Later changes are no longer picked up.validate/validateSafe.resolveinfers its return type for class and typed-token keys where it previously widened tounknown.Tests
Adds
container-api.test.ts,regressions.test.ts, andtoken.test.ts. The regression suite also pins behaviour nothing previously guarded — parent-owned singletons not reaching child-only deps, sibling sharing, throwing constructors/postConstructleaving the container usable and retryable,replacescoping, and typed tokens as optional deps and through a child.🤖 Generated with Claude Code