fix/correctness-types-cleanup #2

Merged
jperera merged 3 commits from fix/correctness-types-cleanup into main 2026-06-13 12:39:19 +02:00
Owner
No description provided.
Several resolution paths produced wrong results or crashed outright. All
are covered by new regression tests in tests/regression.test.ts.

- Concurrent/re-entrant getAsync() of the same token crashed with a stack
  overflow: the in-flight branch returned a lazy proxy, which an async
  function assimilates by reading `.then`, forcing the proxy to resolve to
  itself while still initializing. Replace the proxy path with per-token
  in-flight Promise memoization so concurrent callers share one resolution
  and the factory runs exactly once.

- init() invoked inter-dependent async factories more than once because it
  ran every factory directly in parallel with no de-duplication. Route it
  through getAsync() so the in-flight memoization is shared.

- Auto-resolved classes were not singletons: resolve()/resolveAsync() and
  get(Class) created a fresh instance on every call, contradicting the
  documented "singletons by default" behavior and breaking singleton
  sharing between parent and child containers. Cache resolved instances.

- A singleton factory returning undefined (or any value, when guarded by
  `instance !== undefined`) re-ran on every get(). Gate the cache hit on
  `initialized` alone so falsy/undefined singletons are cached.

- @inject in a subclass corrupted the parent class's parameter metadata,
  because Reflect.getMetadata walks the prototype chain and the shared Map
  was mutated in place. Read own-metadata and clone inherited tokens.

- createToken used Symbol.for(), interning tokens into the process-global
  registry so equally-named tokens from unrelated code collided. Use a
  unique Symbol() per token.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Type-safety:
- createToken<T> now returns a branded InjectionToken<T> instead of a bare
  symbol, so container.get(token) infers T with no explicit type argument.
  Previously the headline "type-safe" API resolved everything to unknown.
- Newable<T> used `new (...args: unknown[]) => T`, which made
  bind(token).to(AnyClassWithConstructorParams) fail to type-check (unknown
  is not assignable to a declared parameter type). Use `any[]`, the standard
  DI constructor type. This latent bug was invisible because tests were
  never type-checked.
- Passing an async function to the synchronous toFactory() now throws a
  clear ResolutionError on resolution instead of silently caching and
  returning the Promise as the value.
- typecheck now also checks the test suite (new tsconfig.test.json); the
  base tsconfig excludes tests, so type regressions there went unnoticed.

Packaging (published package was unusable):
- build now runs `tsc --emitDeclarationOnly` so dist/index.d.ts actually
  exists; `bun build` alone emits no declarations despite `types` pointing
  at them.
- Add a `files` allowlist so publish ships only dist/, not src/tests/config.
- Order the `exports` conditions with `types` first.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove dead code and guard against its recurrence
All checks were successful
CI / test (pull_request) Successful in 23s
00f60df641
No behavior change (46 tests still pass); net -108 lines of source.

- Remove lazyAsync(): it never invoked its factory, never set its
  initialized flag, and unconditionally threw — it could not work.
- Remove the abandoned circular-detection scaffolding that was never wired
  up: Resolver.{getOrCreateProxy,beginResolution,endResolution,isResolving,
  setPendingInstance,getPendingInstance,clearPendingInstance,
  getResolutionPath} and the resolutionStack/pendingInstances fields. The
  container detects cycles via its own in-progress set + lazy proxies.
- Remove the no-op `detectCircularDependencies` container option (stored but
  never read) and the `CircularDependencyError` class (exported but never
  thrown), plus their README references. Proxy-based circular resolution is
  unaffected.
- Drop unused imports across resolver/async/container/decorators/registry.
- Enable `noUnusedLocals` on the source build config so dead locals/imports
  (the lazyAsync class of bug) fail the build; left off for tests, whose
  fixtures intentionally declare unused @inject parameter properties.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jperera/di4ts!2
No description provided.