From b6481b5466d10dd4b9f6b95bab9c9aa9a80b719b Mon Sep 17 00:00:00 2001 From: Iku-turso Date: Fri, 2 Sep 2022 10:15:26 +0300 Subject: [PATCH] fix: Make decorator injectables behave consistently with cycles --- ...Container.context-of-instantiation.test.js | 4 + .../createContainer.global-decoration.test.js | 138 +++++++++++++++++- .../createContainer.js | 45 +++--- 3 files changed, 165 insertions(+), 22 deletions(-) diff --git a/packages/injectable/src/dependency-injection-container/createContainer.context-of-instantiation.test.js b/packages/injectable/src/dependency-injection-container/createContainer.context-of-instantiation.test.js index 775633df..0508c352 100644 --- a/packages/injectable/src/dependency-injection-container/createContainer.context-of-instantiation.test.js +++ b/packages/injectable/src/dependency-injection-container/createContainer.context-of-instantiation.test.js @@ -289,6 +289,7 @@ describe('createContainer.context-of-instantiation', () => { it('decorator has context', () => { expect(contextOfDecorator.map(get('injectable.id'))).toEqual([ 'some-container', + 'some-injectable', 'instantiate-decorator-token', 'some-decorator', ]); @@ -342,6 +343,7 @@ describe('createContainer.context-of-instantiation', () => { it('decorator has context', () => { expect(contextOfDecorator.map(get('injectable.id'))).toEqual([ 'some-container', + 'some-injectable', 'injection-decorator-token', 'some-decorator', ]); @@ -434,6 +436,7 @@ describe('createContainer.context-of-instantiation', () => { it('parent decorator has root as context without knowledge of other decorators', () => { expect(contextOfParentDecorator.map(get('injectable.id'))).toEqual([ 'some-container', + 'some-parent-injectable', 'injection-decorator-token', 'some-parent-decorator', ]); @@ -442,6 +445,7 @@ describe('createContainer.context-of-instantiation', () => { it('child decorator has root as context without knowledge of other decorators', () => { expect(contextOfChildDecorator.map(get('injectable.id'))).toEqual([ 'some-container', + 'some-parent-injectable', 'injection-decorator-token', 'some-child-decorator', ]); diff --git a/packages/injectable/src/dependency-injection-container/createContainer.global-decoration.test.js b/packages/injectable/src/dependency-injection-container/createContainer.global-decoration.test.js index 45b66562..2b3e46ec 100644 --- a/packages/injectable/src/dependency-injection-container/createContainer.global-decoration.test.js +++ b/packages/injectable/src/dependency-injection-container/createContainer.global-decoration.test.js @@ -1,9 +1,7 @@ -import { noop } from 'lodash/fp'; +import { identity } from 'lodash/fp'; import getInjectable from '../getInjectable/getInjectable'; -import createContainer, { - injectionDecoratorToken, - registrationCallbackToken, -} from './createContainer'; +import createContainer, { injectionDecoratorToken } from './createContainer'; +import { instantiationDecoratorToken } from '../../index'; describe('createContainer.global-decoration', () => { it('given global decorator and child injectable, when parent is injected, decorates instances and instantiation parameters of both parent and child', () => { @@ -164,4 +162,134 @@ describe('createContainer.global-decoration', () => { expect(actual).toBe('decorated-with-some-override(some-undecorated-value)'); }); + + it('given injectable and infinitely recursing injection decorator, when injected, throws', () => { + const someInjectable = getInjectable({ + id: 'some-injectable-to-be-decorated', + + instantiate: () => 'irrelevant', + }); + + const someDecoratorInjectable = getInjectable({ + id: 'some-decorator', + injectionToken: injectionDecoratorToken, + decorable: false, + + instantiate: di => ({ + decorate: + toBeDecorated => + (...args) => { + // Note: injection of decorated injectable within the decorator + // itself causes the recursion. + di.inject(someInjectable); + + return toBeDecorated(...args); + }, + }), + }); + + const di = createContainer('some-container'); + + di.register(someInjectable, someDecoratorInjectable); + + expect(() => { + di.inject(someInjectable); + }).toThrow( + 'Cycle of injectables encountered: "some-container" -> "some-injectable-to-be-decorated" -> "injection-decorator-token" -> "some-decorator" -> "some-injectable-to-be-decorated"', + ); + }); + + it('given injectable and injection decorator which decorates itself, when injected, throws', () => { + const someInjectable = getInjectable({ + id: 'some-injectable-to-be-decorated', + + instantiate: () => 'irrelevant', + }); + + const someDecoratorInjectable = getInjectable({ + id: 'some-decorator', + injectionToken: injectionDecoratorToken, + // Note: decorator being decorable itself causes the infinite recursion. + // decorable: false, + + instantiate: () => ({ + decorate: identity, + }), + }); + + const di = createContainer('some-container'); + + di.register(someInjectable, someDecoratorInjectable); + + expect(() => { + di.inject(someInjectable); + }).toThrow( + 'Cycle of injectables encountered: "some-container" -> "some-injectable-to-be-decorated" -> "injection-decorator-token" -> "some-decorator" -> "injection-decorator-token" -> "some-decorator"', + ); + }); + + it('given injectable and instantiation decorator which decorates itself, when injected, throws', () => { + const someInjectable = getInjectable({ + id: 'some-injectable-to-be-decorated', + + instantiate: () => 'irrelevant', + }); + + const someDecoratorInjectable = getInjectable({ + id: 'some-decorator', + injectionToken: instantiationDecoratorToken, + // Note: decorator being decorable itself causes the infinite recursion. + // decorable: false, + + instantiate: () => ({ + decorate: identity, + }), + }); + + const di = createContainer('some-container'); + + di.register(someInjectable, someDecoratorInjectable); + + expect(() => { + di.inject(someInjectable); + }).toThrow( + 'Cycle of injectables encountered: "some-container" -> "some-injectable-to-be-decorated" -> "instantiate-decorator-token" -> "some-decorator" -> "instantiate-decorator-token" -> "some-decorator"', + ); + }); + + it('given injectable and infinitely recursing instantiation decorator, when injected, throws', () => { + const someInjectable = getInjectable({ + id: 'some-injectable-to-be-decorated', + + instantiate: () => 'irrelevant', + }); + + const someDecoratorInjectable = getInjectable({ + id: 'some-decorator', + injectionToken: instantiationDecoratorToken, + decorable: false, + + instantiate: di => ({ + decorate: + toBeDecorated => + (...args) => { + // Note: injection of decorated injectable within the decorator + // itself causes the recursion. + di.inject(someInjectable); + + return toBeDecorated(...args); + }, + }), + }); + + const di = createContainer('some-container'); + + di.register(someInjectable, someDecoratorInjectable); + + expect(() => { + di.inject(someInjectable); + }).toThrow( + 'Cycle of injectables encountered: "some-container" -> "some-injectable-to-be-decorated" -> "instantiate-decorator-token" -> "some-decorator" -> "some-injectable-to-be-decorated"', + ); + }); }); diff --git a/packages/injectable/src/dependency-injection-container/createContainer.js b/packages/injectable/src/dependency-injection-container/createContainer.js index bd51cc9b..54c4b83a 100644 --- a/packages/injectable/src/dependency-injection-container/createContainer.js +++ b/packages/injectable/src/dependency-injection-container/createContainer.js @@ -352,21 +352,6 @@ const getInstance = ({ }, ]; - const injectableCausingCycle = oldContext - .filter(contextItem => !contextItem.injectable.cannotCauseCycles) - .find( - contextItem => - contextItem.injectable.id === injectableToBeInstantiated.id, - ); - - if (injectableCausingCycle) { - throw new Error( - `Cycle of injectables encountered: "${newContext - .map(x => x.injectable.id) - .join('" -> "')}"`, - ); - } - const instanceMap = instancesByInjectableMap.get( injectableToBeInstantiated.id, ); @@ -454,7 +439,13 @@ const withInstantiationDecoratorsFor = ({ injectMany, injectable }) => { return toBeDecorated(...args); } - const decorators = injectMany(instantiationDecoratorToken) + const [{ context }] = args; + + const decorators = injectMany( + instantiationDecoratorToken, + undefined, + context, + ) .filter(isRelevantDecorator) .map(x => x.decorate); @@ -472,9 +463,29 @@ const withInjectionDecoratorsFor = return toBeDecorated(alias, ...args); } + const [, oldContext] = args; + + const injectableCausingCycle = oldContext + .filter(contextItem => !contextItem.injectable.cannotCauseCycles) + .find(contextItem => contextItem.injectable.id === alias.id); + + const newContext = [...oldContext, { injectable: alias }]; + + if (injectableCausingCycle) { + throw new Error( + `Cycle of injectables encountered: "${newContext + .map(x => x.injectable.id) + .join('" -> "')}"`, + ); + } + const isRelevantDecorator = isRelevantDecoratorFor(alias); - const decorators = injectMany(injectionDecoratorToken) + const decorators = injectMany( + injectionDecoratorToken, + undefined, + newContext, + ) .filter(isRelevantDecorator) .map(x => x.decorate);