From 76b652a009359aa7732f68adc2f56485e791e350 Mon Sep 17 00:00:00 2001 From: Jinho Ayden Jeong <144667387+ayden94@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:46:58 +0900 Subject: [PATCH 1/4] Resolve #2279: harden DI scope ownership --- packages/di/src/container.test.ts | 138 ++++++++++++ packages/di/src/container.ts | 360 +++++++++++++++++++++--------- packages/di/src/types.ts | 18 +- 3 files changed, 396 insertions(+), 120 deletions(-) diff --git a/packages/di/src/container.test.ts b/packages/di/src/container.test.ts index e20d53353..d4bb81890 100644 --- a/packages/di/src/container.test.ts +++ b/packages/di/src/container.test.ts @@ -666,6 +666,31 @@ describe('Container', () => { expect(afterOverride).not.toBe(beforeOverride); }); + it('invalidates nested request-scope descendants when an ancestor dependency is overridden', async () => { + const CONFIG = Symbol('NestedConfig'); + + class RequestConsumer { + constructor(readonly config: string) {} + } + + const root = new Container().register( + { provide: CONFIG, useValue: 'before-override' }, + { provide: RequestConsumer, scope: Scope.REQUEST, useClass: RequestConsumer, inject: [CONFIG] }, + ); + const parentScope = root.createRequestScope(); + const descendantScope = parentScope.createRequestScope(); + + const beforeOverride = await descendantScope.resolve(RequestConsumer); + + root.override({ provide: CONFIG, useValue: 'after-override' }); + + const afterOverride = await descendantScope.resolve(RequestConsumer); + + expect(beforeOverride.config).toBe('before-override'); + expect(afterOverride.config).toBe('after-override'); + expect(afterOverride).not.toBe(beforeOverride); + }); + it('replaces existing multi providers when overriding a token', async () => { const token = Symbol('plugins'); const container = new Container().register( @@ -805,6 +830,20 @@ describe('Container', () => { expect(() => new Container().register(provider)).toThrow('provide token'); }); + it('throws InvalidProviderError when an object provider uses null provide', () => { + const provider = { provide: null, useValue: 'missing-token' } as unknown as Provider; + + expect(() => new Container().register(provider)).toThrow(InvalidProviderError); + expect(() => new Container().register(provider)).toThrow('provide token'); + }); + + it('throws InvalidProviderError when an object provider has no strategy', () => { + const provider = { provide: Symbol('strategy-less-provider') } as unknown as Provider; + + expect(() => new Container().register(provider)).toThrow(InvalidProviderError); + expect(() => new Container().register(provider)).toThrow('exactly one'); + }); + it('throws InvalidProviderError when an object provider has more than one strategy', () => { const token = Symbol('ambiguous-provider'); const provider = { provide: token, useValue: 'value', useFactory: () => 'factory' } as unknown as Provider; @@ -1425,6 +1464,38 @@ describe('Container', () => { }); }); + describe('resolution introspection', () => { + it('returns read-only map views and frozen provider records', async () => { + const token = Symbol('introspection-token'); + const plugins = Symbol('introspection-plugins'); + const container = new Container().register( + { provide: token, useValue: 'value' }, + { provide: plugins, useValue: 'plugin', multi: true }, + ); + + await container.resolve(token); + await container.resolve(plugins); + + const state = container.inspectResolutionState(); + const provider = state.registrations.get(token); + const multiProviders = state.multiRegistrations.get(plugins); + + if (!provider || !multiProviders) { + expect.unreachable('expected introspection state to expose registered providers'); + } + + expect(Object.isFrozen(provider)).toBe(true); + expect(Object.isFrozen(provider.inject)).toBe(true); + expect(Object.isFrozen(multiProviders)).toBe(true); + expect(Reflect.get(state.registrations, 'set')).toBeUndefined(); + expect(Reflect.get(state.multiRegistrations, 'clear')).toBeUndefined(); + expect(Reflect.get(state.singletonCache, 'delete')).toBeUndefined(); + expect(Reflect.get(state.multiSingletonCache, 'set')).toBeUndefined(); + await expect(container.resolve(token)).resolves.toBe('value'); + await expect(container.resolve(plugins)).resolves.toEqual(['plugin']); + }); + }); + describe('dispose', () => { it('calls onDestroy for resolved singleton instances in reverse creation order', async () => { const events: string[] = []; @@ -1483,6 +1554,40 @@ describe('Container', () => { expect(events).toEqual(['request', 'singleton']); }); + it('recursively disposes nested request scopes when a non-root request scope is disposed', async () => { + const events: string[] = []; + + class ParentRequestService { + onDestroy() { + events.push('parent'); + } + } + + class ChildRequestService { + onDestroy() { + events.push('child'); + } + } + + const root = new Container().register( + { provide: ParentRequestService, scope: Scope.REQUEST, useClass: ParentRequestService }, + { provide: ChildRequestService, scope: Scope.REQUEST, useClass: ChildRequestService }, + ); + const parentScope = root.createRequestScope(); + const childScope = parentScope.createRequestScope(); + + await parentScope.resolve(ParentRequestService); + await childScope.resolve(ChildRequestService); + + await parentScope.dispose(); + + expect(events).toEqual(['child', 'parent']); + + await root.dispose(); + + expect(events).toEqual(['child', 'parent']); + }); + it('removes materialized request scopes from the root child scope registry on dispose', async () => { class RequestStore {} @@ -1707,6 +1812,39 @@ describe('Container', () => { expect(events).toEqual(['consumer:before-override', 'consumer:after-override']); }); + it('disposes stale nested request-scope consumers invalidated by ancestor overrides exactly once', async () => { + const events: string[] = []; + const CONFIG = Symbol('NestedDisposableConsumerConfig'); + + class RequestConsumer { + constructor(readonly config: string) {} + + onDestroy() { + events.push(`consumer:${this.config}`); + } + } + + const root = new Container().register( + { provide: CONFIG, useValue: 'before-override' }, + { provide: RequestConsumer, scope: Scope.REQUEST, useClass: RequestConsumer, inject: [CONFIG] }, + ); + const parentScope = root.createRequestScope(); + const descendantScope = parentScope.createRequestScope(); + + await descendantScope.resolve(RequestConsumer); + + root.override({ provide: CONFIG, useValue: 'after-override' }); + await Promise.resolve(); + + expect(events).toEqual(['consumer:before-override']); + + await descendantScope.resolve(RequestConsumer); + await parentScope.dispose(); + await root.dispose(); + + expect(events).toEqual(['consumer:before-override', 'consumer:after-override']); + }); + it('disposes stale overridden multi singleton instances immediately and exactly once', async () => { const events: string[] = []; const token = Symbol('multi-rotating-disposable-token'); diff --git a/packages/di/src/container.ts b/packages/di/src/container.ts index 965655941..4c970e95b 100644 --- a/packages/di/src/container.ts +++ b/packages/di/src/container.ts @@ -11,19 +11,29 @@ import { } from './errors.js'; import type { ClassType, - ClassProvider, Disposable, - ExistingProvider, - FactoryProvider, ForwardRefFn, NormalizedProvider, OptionalToken, Provider, - ValueProvider, } from './types.js'; import { Scope, isForwardRef, isOptionalToken } from './types.js'; -type ObjectProvider = ClassProvider | ExistingProvider | FactoryProvider | ValueProvider; +type ProviderObjectInput = { + readonly inject?: readonly (Token | ForwardRefFn | OptionalToken)[]; + readonly multi?: boolean; + readonly provide?: Token | null; + readonly resolverClass?: ClassType; + readonly scope?: Scope; + readonly useClass?: unknown; + readonly useExisting?: Token | null; + readonly useFactory?: unknown; + readonly useValue?: unknown; +}; + +type ValidatedProviderObject = ProviderObjectInput & { readonly provide: Token }; + +type FactoryResolutionKind = 'async' | 'sync'; interface CachedResolutionPlan { readonly lineageRevision: string; @@ -31,46 +41,157 @@ interface CachedResolutionPlan { } /** - * Public read/write seam for framework-owned testing and tooling that need to + * Controlled cache adoption seam for framework-owned testing and tooling that + * need synchronous helpers to preserve container-owned singleton disposal. + */ +export interface ContainerResolutionCacheOwner { + readonly deleteMultiSingleton: (provider: NormalizedProvider) => void; + readonly deleteSingleton: (token: Token) => void; + readonly setMultiSingleton: (provider: NormalizedProvider, promise: Promise) => void; + readonly setSingleton: (token: Token, promise: Promise) => void; +} + +/** + * Read-only factory resolution diagnostics recorded by container-owned factory + * instantiation paths. + */ +export interface ContainerFactoryResolutionState { + readonly get: (provider: NormalizedProvider) => FactoryResolutionKind | undefined; + readonly has: (provider: NormalizedProvider) => boolean; +} + +/** + * Public read-only seam for framework-owned testing and tooling that need to * inspect a container's resolved provider graph without depending on private * field names or structural casts. */ export interface ContainerResolutionState { + readonly cacheOwner: ContainerResolutionCacheOwner; + readonly factoryResolutionKinds: ContainerFactoryResolutionState; readonly parent?: ContainerResolutionState; - readonly registrations: Map; - readonly multiRegistrations: Map; - readonly multiSingletonCache: Map>; + readonly registrations: ReadonlyMap; + readonly multiRegistrations: ReadonlyMap; + readonly multiSingletonCache: ReadonlyMap>; readonly requestScopeEnabled: boolean; - readonly singletonCache: Map>; + readonly singletonCache: ReadonlyMap>; +} + +class ReadonlyMapView implements ReadonlyMap { + readonly [Symbol.toStringTag] = 'Map'; + + constructor(private readonly source: ReadonlyMap) {} + + get size(): number { + return this.source.size; + } + + entries(): IterableIterator<[K, V]> { + return this.source.entries(); + } + + forEach(callbackfn: (value: V, key: K, map: ReadonlyMap) => void, thisArg?: unknown): void { + for (const [key, value] of this.source) { + callbackfn.call(thisArg, value, key, this); + } + } + + get(key: K): V | undefined { + return this.source.get(key); + } + + has(key: K): boolean { + return this.source.has(key); + } + + keys(): IterableIterator { + return this.source.keys(); + } + + values(): IterableIterator { + return this.source.values(); + } + + [Symbol.iterator](): IterableIterator<[K, V]> { + return this.entries(); + } +} + +class ReadonlyMultiRegistrationMapView implements ReadonlyMap { + readonly [Symbol.toStringTag] = 'Map'; + + constructor(private readonly source: ReadonlyMap) {} + + get size(): number { + return this.source.size; + } + + *entries(): IterableIterator<[Token, readonly NormalizedProvider[]]> { + for (const [token, providers] of this.source) { + yield [token, Object.freeze([...providers])]; + } + } + + forEach(callbackfn: (value: readonly NormalizedProvider[], key: Token, map: ReadonlyMap) => void, thisArg?: unknown): void { + for (const [key, value] of this.entries()) { + callbackfn.call(thisArg, value, key, this); + } + } + + get(key: Token): readonly NormalizedProvider[] | undefined { + const providers = this.source.get(key); + return providers ? Object.freeze([...providers]) : undefined; + } + + has(key: Token): boolean { + return this.source.has(key); + } + + keys(): IterableIterator { + return this.source.keys(); + } + + *values(): IterableIterator { + for (const providers of this.source.values()) { + yield Object.freeze([...providers]); + } + } + + [Symbol.iterator](): IterableIterator<[Token, readonly NormalizedProvider[]]> { + return this.entries(); + } } function isClassConstructor(value: Provider): value is ClassType { return typeof value === 'function'; } -function isValueProvider(value: Provider): value is ValueProvider { - return typeof value === 'object' && value !== null && 'useValue' in value; +function isProviderObject(value: unknown): value is ProviderObjectInput { + return typeof value === 'object' && value !== null; } -function isFactoryProvider(value: Provider): value is FactoryProvider { - return typeof value === 'object' && value !== null && 'useFactory' in value; +function isClassType(value: unknown): value is ClassType { + return typeof value === 'function'; } -function isClassProvider(value: Provider): value is ClassProvider { - return typeof value === 'object' && value !== null && 'useClass' in value; +function isFactoryFunction(value: unknown): value is (...deps: unknown[]) => unknown { + return typeof value === 'function'; } -function isExistingProvider(value: Provider): value is ExistingProvider { - return typeof value === 'object' && value !== null && 'useExisting' in value; +function isPromiseLike(value: unknown): value is PromiseLike { + return ( + (typeof value === 'object' || typeof value === 'function') && + value !== null && + typeof (value as { then?: unknown }).then === 'function' + ); } -function assertProviderToken(provider: ObjectProvider): void { +function assertProviderToken(provider: ProviderObjectInput): asserts provider is ValidatedProviderObject { if (!('provide' in provider) || provider.provide == null) { throw new InvalidProviderError('Provider object must include a non-null provide token.'); } } -function assertProviderStrategy(provider: ObjectProvider): void { +function assertProviderStrategy(provider: ProviderObjectInput): void { const strategyCount = Number('useValue' in provider) + Number('useFactory' in provider) + Number('useClass' in provider) + Number('useExisting' in provider); if (strategyCount !== 1) { @@ -78,7 +199,7 @@ function assertProviderStrategy(provider: ObjectProvider): void { } } -function assertObjectProvider(provider: ObjectProvider): void { +function assertObjectProvider(provider: ProviderObjectInput): asserts provider is ValidatedProviderObject { assertProviderToken(provider); assertProviderStrategy(provider); } @@ -91,87 +212,93 @@ function normalizeInjectToken(token: Token | ForwardRefFn | OptionalToken): Toke return token; } +function freezeNormalizedProvider(provider: NormalizedProvider): NormalizedProvider { + return Object.freeze({ + ...provider, + inject: Object.freeze([...provider.inject]), + }); +} + function normalizeProvider(provider: Provider): NormalizedProvider { if (isClassConstructor(provider)) { const metadata = getClassDiMetadata(provider); - return { + return freezeNormalizedProvider({ inject: (metadata?.inject ?? []).map(normalizeInjectToken), provide: provider, scope: metadata?.scope ?? Scope.DEFAULT, type: 'class', useClass: provider, - }; + }); + } + + if (!isProviderObject(provider)) { + throw new InvalidProviderError('Unsupported provider type.'); } - if (isValueProvider(provider)) { - assertObjectProvider(provider); + const objectProvider: ProviderObjectInput = provider; + assertObjectProvider(objectProvider); - return { + if ('useValue' in objectProvider) { + return freezeNormalizedProvider({ inject: [], - multi: provider.multi, - provide: provider.provide, + multi: objectProvider.multi, + provide: objectProvider.provide, scope: Scope.DEFAULT, type: 'value', - useValue: provider.useValue, - }; + useValue: objectProvider.useValue, + }); } - if (isFactoryProvider(provider)) { - assertObjectProvider(provider); - - if (typeof provider.useFactory !== 'function') { - throw new InvalidProviderError('Factory provider useFactory must be a function.', { token: provider.provide }); + if ('useFactory' in objectProvider) { + if (!isFactoryFunction(objectProvider.useFactory)) { + throw new InvalidProviderError('Factory provider useFactory must be a function.', { token: objectProvider.provide }); } - const metadata = provider.resolverClass ? getClassDiMetadata(provider.resolverClass) : undefined; + const metadata = objectProvider.resolverClass ? getClassDiMetadata(objectProvider.resolverClass) : undefined; - return { - inject: (provider.inject ?? []).map(normalizeInjectToken), - multi: provider.multi, - provide: provider.provide, - scope: provider.scope ?? metadata?.scope ?? Scope.DEFAULT, + return freezeNormalizedProvider({ + inject: (objectProvider.inject ?? []).map(normalizeInjectToken), + multi: objectProvider.multi, + provide: objectProvider.provide, + scope: objectProvider.scope ?? metadata?.scope ?? Scope.DEFAULT, type: 'factory', - useFactory: provider.useFactory, - }; + useFactory: objectProvider.useFactory, + }); } - if (isClassProvider(provider)) { - assertObjectProvider(provider); - - if (typeof provider.useClass !== 'function') { - throw new InvalidProviderError('Class provider useClass must be a constructor.', { token: provider.provide }); + if ('useClass' in objectProvider) { + if (!isClassType(objectProvider.useClass)) { + throw new InvalidProviderError('Class provider useClass must be a constructor.', { token: objectProvider.provide }); } - const metadata = getClassDiMetadata(provider.useClass); + const metadata = getClassDiMetadata(objectProvider.useClass); - return { - inject: (provider.inject ?? metadata?.inject ?? []).map(normalizeInjectToken), - multi: provider.multi, - provide: provider.provide, - scope: provider.scope ?? metadata?.scope ?? Scope.DEFAULT, + return freezeNormalizedProvider({ + inject: (objectProvider.inject ?? metadata?.inject ?? []).map(normalizeInjectToken), + multi: objectProvider.multi, + provide: objectProvider.provide, + scope: objectProvider.scope ?? metadata?.scope ?? Scope.DEFAULT, type: 'class', - useClass: provider.useClass, - }; + useClass: objectProvider.useClass, + }); } - if (isExistingProvider(provider)) { - assertObjectProvider(provider); - - if (provider.useExisting == null) { - throw new InvalidProviderError('Alias provider useExisting must be a non-null token.', { token: provider.provide }); + if ('useExisting' in objectProvider) { + if (objectProvider.useExisting == null) { + throw new InvalidProviderError('Alias provider useExisting must be a non-null token.', { token: objectProvider.provide }); } - return { + return freezeNormalizedProvider({ inject: [], - provide: provider.provide, + provide: objectProvider.provide, scope: Scope.DEFAULT, type: 'existing', - useExisting: provider.useExisting, - }; + useExisting: objectProvider.useExisting, + }); } - throw new InvalidProviderError('Unsupported provider type.'); + throw new InvalidProviderError('Provider object must declare exactly one of useValue, useFactory, useClass, or useExisting.'); } /** @@ -188,6 +315,7 @@ export class Container { private readonly staleDisposalErrors: unknown[] = []; private readonly singletonCache: Map>; private readonly forwardRefTokenCache = new WeakMap(); + private readonly factoryResolutionKinds = new WeakMap(); private readonly providerLookupPlanCache = new Map>(); private readonly multiProviderPlanCache = new Map>(); private readonly requestScopeVerdictPlanCache = new Map>(); @@ -195,7 +323,7 @@ export class Container { private childScopes: Set | undefined; private disposePromise: Promise | undefined; private disposed = false; - private trackedByRoot = false; + private trackedByParent = false; private graphRevision = 0; constructor( @@ -347,22 +475,52 @@ export class Container { * * This method is the supported introspection seam for packages such as * `@fluojs/testing`; callers should prefer ordinary `has(...)` and - * `resolve(...)` unless they need to preserve container cache ownership while - * implementing a framework-level helper. + * `resolve(...)` unless they need read-only graph/cache visibility while + * implementing a framework-level helper. Cache adoption for synchronous + * helpers goes through `cacheOwner`; the returned maps are not mutable + * container internals. * - * @returns Provider registrations and resolution caches for this container scope. + * @returns Read-only provider registrations and resolution caches for this container scope. */ inspectResolutionState(): ContainerResolutionState { return { + cacheOwner: this.createCacheOwner(), + factoryResolutionKinds: this.createFactoryResolutionState(), parent: this.parent?.inspectResolutionState(), - registrations: this.registrations, - multiRegistrations: this.multiRegistrations, - multiSingletonCache: this.multiSingletonCache, + registrations: new ReadonlyMapView(this.registrations), + multiRegistrations: new ReadonlyMultiRegistrationMapView(this.multiRegistrations), + multiSingletonCache: new ReadonlyMapView(this.multiSingletonCache), requestScopeEnabled: this.requestScopeEnabled, - singletonCache: this.singletonCache, + singletonCache: new ReadonlyMapView(this.singletonCache), }; } + private createCacheOwner(): ContainerResolutionCacheOwner { + return Object.freeze({ + deleteMultiSingleton: (provider: NormalizedProvider) => { + this.multiSingletonCache.delete(provider); + }, + deleteSingleton: (token: Token) => { + this.singletonCache.delete(token); + }, + setMultiSingleton: (provider: NormalizedProvider, promise: Promise) => { + this.multiSingletonCache.set(provider, promise); + }, + setSingleton: (token: Token, promise: Promise) => { + this.singletonCache.set(token, promise); + }, + }); + } + + private createFactoryResolutionState(): ContainerFactoryResolutionState { + const root = this.root(); + + return Object.freeze({ + get: (provider: NormalizedProvider) => root.factoryResolutionKinds.get(provider), + has: (provider: NormalizedProvider) => root.factoryResolutionKinds.has(provider), + }); + } + /** * Returns whether resolving a token may require a request-scope container. * @@ -449,8 +607,8 @@ export class Container { const errors: unknown[] = []; try { - // Dispose all live request-scope children first (root only) - if (!this.parent && this.childScopes && this.childScopes.size > 0) { + // Dispose all live request-scope children before tearing down this scope's cache. + if (this.childScopes && this.childScopes.size > 0) { const childResults = await Promise.allSettled(Array.from(this.childScopes).map((child) => child.dispose())); for (const result of childResults) { @@ -470,9 +628,9 @@ export class Container { this.throwDisposalErrors(errors); } finally { - if (this.parent && this.trackedByRoot) { - this.root().childScopes?.delete(this); - this.trackedByRoot = false; + if (this.parent && this.trackedByParent) { + this.parent.childScopes?.delete(this); + this.trackedByParent = false; } } } @@ -591,7 +749,7 @@ export class Container { return true; } - return (metadata?.inject ?? []).some((depEntry) => this.dependencyEntryRequiresRequestScope(depEntry, visited)); + return (metadata?.inject ?? []).some((depEntry: Token | ForwardRefFn | OptionalToken) => this.dependencyEntryRequiresRequestScope(depEntry, visited)); } private normalizedProviderRequiresRequestScope(provider: NormalizedProvider, visited: Set): boolean { @@ -852,14 +1010,14 @@ export class Container { } private ensureTrackedRequestScope(): void { - if (!this.requestScopeEnabled || !this.parent || this.trackedByRoot) { + if (!this.requestScopeEnabled || !this.parent || this.trackedByParent) { return; } - const root = this.root(); - root.childScopes ??= new Set(); - root.childScopes.add(this); - this.trackedByRoot = true; + this.parent.ensureTrackedRequestScope(); + this.parent.childScopes ??= new Set(); + this.parent.childScopes.add(this); + this.trackedByParent = true; } private requestCacheForWrite(): Map> { @@ -1131,9 +1289,11 @@ export class Container { throw new InvariantError('Factory provider is missing useFactory.'); } - const deps = await this.resolveProviderDeps(provider, chain, activeTokens); + const deps = await this.resolveProviderDeps(provider, chain, activeTokens); + const value = provider.useFactory(...deps); + this.root().factoryResolutionKinds.set(provider, isPromiseLike(value) ? 'async' : 'sync'); - return provider.useFactory(...deps); + return value as T; } case 'class': { if (!provider.useClass) { @@ -1305,31 +1465,9 @@ export class Container { private invalidateAffectedCachedEntriesInHierarchy(token: Token): void { this.invalidateAffectedCachedEntries(token); - const childScopes = this.root().childScopes; - - if (!childScopes) { - return; - } - - for (const childScope of childScopes) { - if (this.isAncestorOf(childScope)) { - childScope.invalidateAffectedCachedEntries(token); - } - } - } - - private isAncestorOf(container: Container): boolean { - let current = container.parent; - - while (current) { - if (current === this) { - return true; - } - - current = current.parent; + for (const childScope of this.childScopes ?? []) { + childScope.invalidateAffectedCachedEntriesInHierarchy(token); } - - return false; } private invalidateAffectedCachedEntries(token: Token): void { diff --git a/packages/di/src/types.ts b/packages/di/src/types.ts index aa776b9a1..9f4d56f64 100644 --- a/packages/di/src/types.ts +++ b/packages/di/src/types.ts @@ -114,15 +114,15 @@ export interface RequestScopeContainer { * {@link ValueProvider}, or {@link ExistingProvider}. The container owns construction of normalized records. */ export interface NormalizedProvider { - inject: InjectionToken[]; - provide: Token; - scope: Scope; - type: 'class' | 'factory' | 'value' | 'existing'; - useClass?: ClassType; - useFactory?: (...deps: unknown[]) => MaybePromise; - useValue?: T; - useExisting?: Token; - multi?: boolean; + readonly inject: readonly InjectionToken[]; + readonly provide: Token; + readonly scope: Scope; + readonly type: 'class' | 'factory' | 'value' | 'existing'; + readonly useClass?: ClassType; + readonly useFactory?: (...deps: unknown[]) => MaybePromise; + readonly useValue?: T; + readonly useExisting?: Token; + readonly multi?: boolean; } /** From b309098508f859258b0e9084349dd91fcf73cce4 Mon Sep 17 00:00:00 2001 From: Jinho Ayden Jeong <144667387+ayden94@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:47:17 +0900 Subject: [PATCH 2/4] Resolve #2279: adapt testing introspection cache ownership --- packages/testing/src/module.ts | 65 ++++++++-------------------------- 1 file changed, 15 insertions(+), 50 deletions(-) diff --git a/packages/testing/src/module.ts b/packages/testing/src/module.ts index 9f4a646ec..18ad18c8c 100644 --- a/packages/testing/src/module.ts +++ b/packages/testing/src/module.ts @@ -151,11 +151,12 @@ function isPromiseLike(value: unknown): value is PromiseLike { } interface SyncResolverState { - factoryResolutionKinds: WeakMap; + cacheOwner: ContainerResolutionState['cacheOwner']; + factoryResolutionKinds: ContainerResolutionState['factoryResolutionKinds']; introspection: ContainerIntrospection; - multiSingletonCache: Map>; + multiSingletonCache: ReadonlyMap>; resolutionChain: Set; - singletonCache: Map>; + singletonCache: ReadonlyMap>; syncMultiSingletonValues: Map; syncSingletonValues: Map; } @@ -235,19 +236,19 @@ async function resolveMultiLifecycleProvider( bootstrapped: BootstrapResult, introspection: ContainerIntrospection, ): Promise { - const cache = rootContainerIntrospection(introspection).multiSingletonCache; - const cached = cache.get(provider); + const rootIntrospection = rootContainerIntrospection(introspection); + const cached = rootIntrospection.multiSingletonCache.get(provider); if (cached) { return cached; } const instance = instantiateLifecycleProvider(provider, bootstrapped, introspection).catch((error: unknown) => { - cache.delete(provider); + rootIntrospection.cacheOwner.deleteMultiSingleton(provider); throw error; }); - cache.set(provider, instance); + rootIntrospection.cacheOwner.setMultiSingleton(provider, instance); return instance; } @@ -363,41 +364,6 @@ function dependencyToken(entry: Token | ForwardRefFn | OptionalToken): Token { return entry as Token; } -function trackFactoryResolutionKind( - provider: NormalizedProvider, - factoryResolutionKinds: WeakMap, -): void { - if (provider.type !== 'factory' || !provider.useFactory) { - return; - } - - const originalFactory = provider.useFactory; - provider.useFactory = (...deps: unknown[]) => { - const value = originalFactory(...deps); - factoryResolutionKinds.set(provider, isPromiseLike(value) ? 'async' : 'sync'); - return value; - }; -} - -function installFactoryResolutionTracking( - target: ContainerIntrospection, - factoryResolutionKinds: WeakMap, -): void { - if (target.parent) { - installFactoryResolutionTracking(target.parent, factoryResolutionKinds); - } - - for (const provider of target.registrations.values()) { - trackFactoryResolutionKind(provider, factoryResolutionKinds); - } - - for (const providers of target.multiRegistrations.values()) { - for (const provider of providers) { - trackFactoryResolutionKind(provider, factoryResolutionKinds); - } - } -} - function providerGraphIsSyncResolvable(state: SyncResolverState, token: Token, visited = new Set()): boolean { if (visited.has(token)) { return true; @@ -539,7 +505,7 @@ function resolveSyncProvider(provider: NormalizedProvider, state: SyncResolverSt const instance = instantiateSyncProvider(provider, state); state.syncSingletonValues.set(provider.provide, instance); - state.singletonCache.set(provider.provide, Promise.resolve(instance)); + state.cacheOwner.setSingleton(provider.provide, Promise.resolve(instance)); return instance; } @@ -564,7 +530,7 @@ function resolveSyncMultiProvider(provider: NormalizedProvider, state: SyncResol const instance = instantiateSyncProvider(provider, state); state.syncMultiSingletonValues.set(provider, instance); - state.multiSingletonCache.set(provider, Promise.resolve(instance)); + state.cacheOwner.setMultiSingleton(provider, Promise.resolve(instance)); return instance; } @@ -596,16 +562,15 @@ function resolveSyncToken(token: Token, state: SyncResolverState): unknown { function createSyncResolver(container: BootstrapResult['container']): SyncResolver { const introspection = toContainerIntrospection(container); - const factoryResolutionKinds = new WeakMap(); - - installFactoryResolutionTracking(introspection, factoryResolutionKinds); + const rootIntrospection = rootContainerIntrospection(introspection); const state: SyncResolverState = { - factoryResolutionKinds, + cacheOwner: rootIntrospection.cacheOwner, + factoryResolutionKinds: rootIntrospection.factoryResolutionKinds, introspection, - multiSingletonCache: rootContainerIntrospection(introspection).multiSingletonCache, + multiSingletonCache: rootIntrospection.multiSingletonCache, resolutionChain: new Set(), - singletonCache: rootContainerIntrospection(introspection).singletonCache, + singletonCache: rootIntrospection.singletonCache, syncMultiSingletonValues: new Map(), syncSingletonValues: new Map(), }; From ba22cbee2b46cb3c8f9d25478d1725380b3ee85c Mon Sep 17 00:00:00 2001 From: Jinho Ayden Jeong <144667387+ayden94@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:47:28 +0900 Subject: [PATCH 3/4] Resolve #2279: document DI lifecycle hardening --- .changeset/soft-pandas-guard.md | 6 ++++++ packages/di/README.ko.md | 6 +++--- packages/di/README.md | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 .changeset/soft-pandas-guard.md diff --git a/.changeset/soft-pandas-guard.md b/.changeset/soft-pandas-guard.md new file mode 100644 index 000000000..fc59d8089 --- /dev/null +++ b/.changeset/soft-pandas-guard.md @@ -0,0 +1,6 @@ +--- +"@fluojs/di": patch +"@fluojs/testing": patch +--- + +Harden DI request-scope lifecycle and introspection ownership by recursively disposing nested request scopes from their owners, returning read-only introspection state, and keeping testing cache adoption on controlled container-owned APIs. diff --git a/packages/di/README.ko.md b/packages/di/README.ko.md index 2f4fdc3f6..280aacd82 100644 --- a/packages/di/README.ko.md +++ b/packages/di/README.ko.md @@ -76,7 +76,7 @@ const service = await container.resolve(UserService); - **request**: `createRequestScope()`마다 새로 생성됩니다. - **transient**: resolve할 때마다 새 인스턴스를 만듭니다. -dispose 중에는 루트 컨테이너가 먼저 살아 있는 request scope 자식을 정리한 뒤, 자식 dispose 중 하나 이상이 실패하더라도 루트가 소유한 singleton 정리를 계속 수행합니다. 자식/루트 dispose 실패가 여러 개 발생하면 `dispose()`는 모든 shutdown 실패를 확인할 수 있도록 `AggregateError`로 보고합니다. +dispose 중에는 각 컨테이너가 자신이 소유한 살아 있는 request scope 자식을 먼저 재귀적으로 정리하므로, 루트가 아닌 request scope를 dispose해도 중첩 request scope를 닫은 뒤 자신의 request cache를 정리합니다. 이후 루트 dispose는 자식 dispose 중 하나 이상이 실패하더라도 루트가 소유한 singleton 정리를 계속 수행합니다. 자식/루트 dispose 실패가 여러 개 발생하면 `dispose()`는 모든 shutdown 실패를 확인할 수 있도록 `AggregateError`로 보고합니다. ### provider override @@ -163,7 +163,7 @@ const service = await container.resolve(DataService); | `register(...providers)` | 하나 이상의 프로바이더를 등록합니다. | | `override(...providers)` | 기존 provider를 교체하고 cached instance를 무효화하며 오래된 instance를 dispose합니다. | | `resolve(token)` | 토큰을 인스턴스로 비동기 해석합니다. | -| `inspectResolutionState()` | cache ownership을 보존해야 하는 testing/tooling helper를 위한 지원 대상 framework-owned container introspection seam을 노출합니다. 애플리케이션 코드는 `has(...)`와 `resolve(...)`를 우선 사용하세요. | +| `inspectResolutionState()` | read-only map view, frozen provider record, controlled cache adoption을 통해 cache ownership을 보존해야 하는 testing/tooling helper를 위한 지원 대상 framework-owned container introspection seam을 노출합니다. 애플리케이션 코드는 `has(...)`와 `resolve(...)`를 우선 사용하세요. | | `createRequestScope()` | 요청 스코프 의존성을 위한 자식 컨테이너를 생성합니다. | | `has(token)` | 컨테이너나 부모에 토큰이 등록되어 있는지 확인합니다. | | `hasRequestScopedDependency(token)` | 토큰 해석 시 provider 그래프에 request-scoped 의존성이나 순환이 있어 request-scope 컨테이너가 필요할 수 있는지 확인합니다. | @@ -176,7 +176,7 @@ const service = await container.resolve(DataService); | Provider types | `Provider`, `ClassProvider`, `FactoryProvider`, `ValueProvider`, `ExistingProvider`는 `register(...)`와 `override(...)`가 받는 공개 registration shape를 설명합니다. | | Token wrapper types | `ForwardRefFn`과 `OptionalToken`은 `forwardRef(...)`와 `optional(...)`이 반환하는 wrapper 값을 설명합니다. | | Container helper types | `ClassType`, `Disposable`, `RequestScopeContainer`는 typed provider 선언, teardown hook, request-scope helper 경계를 지원합니다. | -| `ContainerResolutionState` | framework testing/tooling integration을 위해 `inspectResolutionState()`가 반환하는 공개 introspection record입니다. | +| Container introspection helper types | `ContainerResolutionState`, `ContainerResolutionCacheOwner`, `ContainerFactoryResolutionState`는 `inspectResolutionState()`가 반환하는 read-only graph/cache view와 controlled cache adoption helper를 설명합니다. | | `NormalizedProvider` | 컨테이너가 검증한 provider record shape를 위한 compatibility-only 공개 타입입니다. provider를 작성할 때는 `Provider`나 구체 provider interface를 우선 사용하세요. normalized record 생성은 컨테이너가 소유합니다. | | `DiErrorContext` | DI error에 붙는 구조화된 context입니다. 로그와 테스트가 token, scope, module, dependency chain, hint를 검사할 수 있게 합니다. | | 에러 클래스 | `InvalidProviderError`, `ContainerResolutionError`, `RequestScopeResolutionError`, `ScopeMismatchError`, `CircularDependencyError`, `DuplicateProviderError`. | diff --git a/packages/di/README.md b/packages/di/README.md index 5159d0139..81f1a0e99 100644 --- a/packages/di/README.md +++ b/packages/di/README.md @@ -75,7 +75,7 @@ fluo DI supports four provider shapes: - **Request**: Instance is created once per `createRequestScope()` call. - **Transient**: A new instance is created every time it is resolved. -During disposal, the root container first tears down live request-scope children and then continues with root-owned singleton cleanup even if one or more child disposals fail. When multiple child/root disposals fail, `dispose()` reports an `AggregateError` so callers can inspect every shutdown failure without losing cleanup progress. +During disposal, each container first recursively tears down live request-scope children it owns, so disposing a non-root request scope also closes nested request scopes before its own request cache. Root disposal then continues with root-owned singleton cleanup even if one or more child disposals fail. When multiple child/root disposals fail, `dispose()` reports an `AggregateError` so callers can inspect every shutdown failure without losing cleanup progress. ### Provider Overrides @@ -163,7 +163,7 @@ Ensure all required providers are registered in the container. If you use `creat | `register(...providers)` | Registers one or more providers. | | `override(...providers)` | Replaces existing providers, invalidates cached instances, and disposes stale instances. | | `resolve(token)` | Asynchronously resolves a token to an instance. | -| `inspectResolutionState()` | Exposes the supported framework-owned container introspection seam for testing/tooling helpers that must preserve cache ownership. Prefer `has(...)` and `resolve(...)` for application code. | +| `inspectResolutionState()` | Exposes the supported framework-owned container introspection seam for testing/tooling helpers that must preserve cache ownership through read-only map views, frozen provider records, and controlled cache adoption. Prefer `has(...)` and `resolve(...)` for application code. | | `createRequestScope()` | Creates a child container for request-scoped dependencies. | | `has(token)` | Checks if a token is registered in the container or its parents. | | `hasRequestScopedDependency(token)` | Checks whether resolving a token may require a request-scope container because its provider graph contains request-scoped dependencies or is cyclic. | @@ -176,7 +176,7 @@ Ensure all required providers are registered in the container. If you use `creat | Provider types | `Provider`, `ClassProvider`, `FactoryProvider`, `ValueProvider`, and `ExistingProvider` describe the public registration shapes accepted by `register(...)` and `override(...)`. | | Token wrapper types | `ForwardRefFn` and `OptionalToken` describe the wrapper values returned by `forwardRef(...)` and `optional(...)`. | | Container helper types | `ClassType`, `Disposable`, and `RequestScopeContainer` support typed provider declarations, teardown hooks, and request-scope helper boundaries. | -| `ContainerResolutionState` | Public introspection record returned by `inspectResolutionState()` for framework testing/tooling integrations. | +| Container introspection helper types | `ContainerResolutionState`, `ContainerResolutionCacheOwner`, and `ContainerFactoryResolutionState` describe the read-only graph/cache views and controlled cache adoption helpers returned by `inspectResolutionState()`. | | `NormalizedProvider` | Compatibility-only public type for the container's validated provider record shape. Prefer authoring providers with `Provider` or the specific provider interfaces; the container owns normalized record construction. | | `DiErrorContext` | Structured context attached to DI errors so logs and tests can inspect tokens, scopes, modules, dependency chains, and hints. | | Error classes | `InvalidProviderError`, `ContainerResolutionError`, `RequestScopeResolutionError`, `ScopeMismatchError`, `CircularDependencyError`, `DuplicateProviderError`. | From c5206d7da99e90ab7fa2cf132f30b26a8332b309 Mon Sep 17 00:00:00 2001 From: Jinho Ayden Jeong <144667387+ayden94@users.noreply.github.com> Date: Wed, 24 Jun 2026 17:03:38 +0900 Subject: [PATCH 4/4] Resolve #2279: fix DI introspection release blockers --- .changeset/soft-pandas-guard.md | 4 +++- packages/di/src/container.ts | 6 +++++- packages/testing/src/module.test.ts | 31 +++++++++++++++++++++++++++++ packages/testing/src/module.ts | 5 +++-- 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/.changeset/soft-pandas-guard.md b/.changeset/soft-pandas-guard.md index fc59d8089..9ee160f8b 100644 --- a/.changeset/soft-pandas-guard.md +++ b/.changeset/soft-pandas-guard.md @@ -1,6 +1,8 @@ --- -"@fluojs/di": patch +"@fluojs/di": major "@fluojs/testing": patch --- Harden DI request-scope lifecycle and introspection ownership by recursively disposing nested request scopes from their owners, returning read-only introspection state, and keeping testing cache adoption on controlled container-owned APIs. + +Migration note: callers that used `inspectResolutionState()` as a mutable escape hatch must stop mutating returned registration/cache maps or normalized provider records. Framework-owned tooling should use the returned `cacheOwner` helpers for controlled cache adoption instead of writing to the maps directly. diff --git a/packages/di/src/container.ts b/packages/di/src/container.ts index 4c970e95b..a7f8d7cc9 100644 --- a/packages/di/src/container.ts +++ b/packages/di/src/container.ts @@ -33,7 +33,7 @@ type ProviderObjectInput = { type ValidatedProviderObject = ProviderObjectInput & { readonly provide: Token }; -type FactoryResolutionKind = 'async' | 'sync'; +export type FactoryResolutionKind = 'async' | 'sync'; interface CachedResolutionPlan { readonly lineageRevision: string; @@ -47,6 +47,7 @@ interface CachedResolutionPlan { export interface ContainerResolutionCacheOwner { readonly deleteMultiSingleton: (provider: NormalizedProvider) => void; readonly deleteSingleton: (token: Token) => void; + readonly recordFactoryResolution: (provider: NormalizedProvider, kind: FactoryResolutionKind) => void; readonly setMultiSingleton: (provider: NormalizedProvider, promise: Promise) => void; readonly setSingleton: (token: Token, promise: Promise) => void; } @@ -503,6 +504,9 @@ export class Container { deleteSingleton: (token: Token) => { this.singletonCache.delete(token); }, + recordFactoryResolution: (provider: NormalizedProvider, kind: FactoryResolutionKind) => { + this.root().factoryResolutionKinds.set(provider, kind); + }, setMultiSingleton: (provider: NormalizedProvider, promise: Promise) => { this.multiSingletonCache.set(provider, promise); }, diff --git a/packages/testing/src/module.test.ts b/packages/testing/src/module.test.ts index 5854521b8..92ae781ce 100644 --- a/packages/testing/src/module.test.ts +++ b/packages/testing/src/module.test.ts @@ -532,6 +532,37 @@ describe('@fluojs/testing', () => { expect(syncConsumer).toBe(resolved); }); + it('preserves sync factory promotion when the dependency is first materialized through get()', async () => { + const TOKEN = Symbol('sync-first-factory-dependency-token'); + const dependency = { name: 'sync-first-dependency' }; + + @Inject(TOKEN) + class SyncFirstFactoryConsumer { + constructor(readonly value: typeof dependency) {} + } + + @Module({ + providers: [ + { + provide: TOKEN, + useFactory: () => dependency, + }, + SyncFirstFactoryConsumer, + ], + }) + class SyncFirstFactoryConsumerModule {} + + const testingModule = await createTestingModule({ rootModule: SyncFirstFactoryConsumerModule }).compile(); + + const syncDependency = testingModule.get(TOKEN); + const resolved = await testingModule.resolve(SyncFirstFactoryConsumer); + const syncConsumer = testingModule.get(SyncFirstFactoryConsumer); + + expect(syncDependency).toBe(dependency); + expect(resolved.value).toBe(syncDependency); + expect(syncConsumer).toBe(resolved); + }); + it('keeps classes depending on useExisting aliases to async factories resolve-only after resolve()', async () => { const SOURCE = Symbol('async-factory-source-token'); const ALIAS = Symbol('async-factory-alias-token'); diff --git a/packages/testing/src/module.ts b/packages/testing/src/module.ts index 18ad18c8c..ab682386c 100644 --- a/packages/testing/src/module.ts +++ b/packages/testing/src/module.ts @@ -468,6 +468,7 @@ function instantiateSyncProvider(provider: NormalizedProvider, state: SyncResolv ); } + state.cacheOwner.recordFactoryResolution(provider, 'sync'); return value; } case 'class': { @@ -711,8 +712,8 @@ class DefaultTestingModuleBuilder implements TestingModuleBuilder { ...bootstrapped, has: (token) => bootstrapped.container.has(token), get: (token) => syncResolver.get(token), - resolve: async (token) => { - const value = await bootstrapped.container.resolve(token); + resolve: async (token: Token) => { + const value = await bootstrapped.container.resolve(token); await syncResolver.syncFromContainer(); return value; },