From 566f4e3065fd66d9e15e742f5e3b71eec48178de Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Thu, 28 Dec 2023 16:47:02 -0500 Subject: [PATCH 01/17] Add option to detect and error on dependency lifetime mismatch Adds new container option `errorOnShorterLivedDependencies` which enables detection and errors on cases where a longer-lived module resolves a shorter-lived dependency, which is then preserved inside the longer-lived module even after its own lifetime and cached resolution expires. Fixes #348 --- README.md | 15 ++++++++++++++- package-lock.json | 2 +- src/__tests__/container.test.ts | 32 +++++++++++++++++++++++++++++++ src/__tests__/lifetime.test.ts | 15 +++++++++++++++ src/container.ts | 34 +++++++++++++++++++++++++++------ src/errors.ts | 14 +++++--------- src/lifetime.ts | 7 +++++++ 7 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 src/__tests__/lifetime.test.ts diff --git a/README.md b/README.md index 8b020da..df830a4 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,16 @@ app.get('/messages', (req, res) => { ``` **IMPORTANT!** If a singleton is resolved, and it depends on a scoped or -transient registration, those will remain in the singleton for it's lifetime! +transient registration, those will remain in the singleton for its lifetime! +Similarly, if a scoped module is resolved, and it depends on a transient +registration, that remains in the scoped module for its lifetime. +In the example above, if `messageService` was a singleton, it would be cached +in the root container, and would always have the `currentUser` from the first +request. Modules should generally not have a longer lifetime than their +dependencies, as this can cause issues of stale data. + +If you want a mismatched configuration like the above to error, set +`errorOnShorterLivedDependencies` in the container options. ```js const makePrintTime = ({ time }) => () => { @@ -830,6 +839,10 @@ Args: **_must_** be named exactly like they are in the registration. For example, a dependency registered as `repository` cannot be referenced in a class constructor as `repo`. + - `options.errorOnShorterLivedDependencies`: If `true`, will throw an error if + a singleton depends on a scoped or transient registration, or if a scoped + registration depends on a transient registration. Defaults to + `false`. ## `asFunction()` diff --git a/package-lock.json b/package-lock.json index 0856334..83d7461 100644 --- a/package-lock.json +++ b/package-lock.json @@ -41,7 +41,7 @@ "typescript": "^5.2.2" }, "engines": { - "node": ">=12.0.0" + "node": ">=14.0.0" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 6fbd389..50022ff 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -298,6 +298,8 @@ describe('container', () => { expect(scope2.cradle.counterValue === 2).toBe(true) expect(scope1Child.cradle.counterValue === 3).toBe(true) + // assert that the parent scope was not affected + expect(scope1.cradle.counterValue === 1).toBe(true) }) it('supports nested scopes', () => { @@ -438,6 +440,36 @@ describe('container', () => { expect(err.message).toContain('lol') }) + it('allows longer lifetime modules to depend on shorter lifetime dependencies by default', () => { + const container = createContainer() + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction(() => 'hah'), + }) + + expect(container.resolve('first')).toBe('hah') + }) + + it('throws an AwilixResolutionError when longer lifetime modules depend on shorter lifetime dependencies and errorOnShorterLivedDependencies is set', () => { + const container = createContainer({ + errorOnShorterLivedDependencies: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction(() => 'hah'), + }) + + const err = throws(() => container.resolve('first')) + expect(err.message).toContain('first -> second') + expect(err.message).toContain( + "Dependency 'second' has a shorter lifetime than its parent: 'first'", + ) + }) + it('behaves properly when the cradle is returned from an async function', async () => { const container = createContainer() container.register({ value: asValue(42) }) diff --git a/src/__tests__/lifetime.test.ts b/src/__tests__/lifetime.test.ts new file mode 100644 index 0000000..2d2017e --- /dev/null +++ b/src/__tests__/lifetime.test.ts @@ -0,0 +1,15 @@ +import { Lifetime, isLifetimeLonger } from '../lifetime' + +describe('isLifetimeLonger', () => { + it('correctly compares lifetimes', () => { + expect(isLifetimeLonger(Lifetime.TRANSIENT, Lifetime.TRANSIENT)).toBe(false) + expect(isLifetimeLonger(Lifetime.TRANSIENT, Lifetime.SCOPED)).toBe(false) + expect(isLifetimeLonger(Lifetime.TRANSIENT, Lifetime.SINGLETON)).toBe(false) + expect(isLifetimeLonger(Lifetime.SCOPED, Lifetime.TRANSIENT)).toBe(true) + expect(isLifetimeLonger(Lifetime.SCOPED, Lifetime.SCOPED)).toBe(false) + expect(isLifetimeLonger(Lifetime.SCOPED, Lifetime.SINGLETON)).toBe(false) + expect(isLifetimeLonger(Lifetime.SINGLETON, Lifetime.TRANSIENT)).toBe(true) + expect(isLifetimeLonger(Lifetime.SINGLETON, Lifetime.SCOPED)).toBe(true) + expect(isLifetimeLonger(Lifetime.SINGLETON, Lifetime.SINGLETON)).toBe(false) + }) +}) diff --git a/src/container.ts b/src/container.ts index 6a4d64c..53957bf 100644 --- a/src/container.ts +++ b/src/container.ts @@ -15,7 +15,7 @@ import { } from './resolvers' import { last, nameValueToObject, isClass } from './utils' import { InjectionMode, InjectionModeType } from './injection-mode' -import { Lifetime } from './lifetime' +import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' import { AwilixResolutionError, AwilixTypeError } from './errors' import { importModule } from './load-module-native.js' @@ -187,6 +187,7 @@ export type ClassOrFunctionReturning = FunctionReturning | Constructor export interface ContainerOptions { require?: (id: string) => any injectionMode?: InjectionModeType + errorOnShorterLivedDependencies?: boolean } /** @@ -197,7 +198,8 @@ export type RegistrationHash = Record> /** * Resolution stack. */ -export interface ResolutionStack extends Array {} +export interface ResolutionStack + extends Array<{ name: string | symbol; lifetime: LifetimeType }> {} /** * Family tree symbol. @@ -451,7 +453,7 @@ export function createContainer( try { // Grab the registration by name. const resolver = getRegistration(name) - if (resolutionStack.indexOf(name) > -1) { + if (resolutionStack.some(({ name: parentName }) => parentName === name)) { throw new AwilixResolutionError( name, resolutionStack, @@ -497,13 +499,33 @@ export function createContainer( throw new AwilixResolutionError(name, resolutionStack) } - // Pushes the currently-resolving module name onto the stack - resolutionStack.push(name) + const lifetime = resolver.lifetime || Lifetime.TRANSIENT + + // if any of the parents have a shorter lifetime than the one requested, + // and the container is configured to do so, throw an error. + if (options?.errorOnShorterLivedDependencies) { + const maybeLongerLifetimeParentIndex = resolutionStack.findIndex( + ({ lifetime: parentLifetime }) => + isLifetimeLonger(parentLifetime, lifetime), + ) + if (maybeLongerLifetimeParentIndex > -1) { + throw new AwilixResolutionError( + name, + resolutionStack, + `Dependency '${name.toString()}' has a shorter lifetime than its parent: '${resolutionStack[ + maybeLongerLifetimeParentIndex + ].name.toString()}'`, + ) + } + } + + // Pushes the currently-resolving module information onto the stack + resolutionStack.push({ name, lifetime }) // Do the thing let cached: CacheEntry | undefined let resolved - switch (resolver.lifetime || Lifetime.TRANSIENT) { + switch (lifetime) { case Lifetime.TRANSIENT: // Transient lifetime means resolve every time. resolved = resolver.resolve(container) diff --git a/src/errors.ts b/src/errors.ts index c08fd9a..5478f47 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -137,15 +137,11 @@ export class AwilixResolutionError extends AwilixError { resolutionStack: ResolutionStack, message?: string, ) { - if (typeof name === 'symbol') { - name = (name as any).toString() - } - resolutionStack = resolutionStack.map((val) => - typeof val === 'symbol' ? (val as any).toString() : val, - ) - resolutionStack.push(name) - const resolutionPathString = resolutionStack.join(' -> ') - let msg = `Could not resolve '${name as any}'.` + const stringName = name.toString() + const nameStack = resolutionStack.map(({ name: val }) => val.toString()) + nameStack.push(stringName) + const resolutionPathString = nameStack.join(' -> ') + let msg = `Could not resolve '${stringName}'.` if (message) { msg += ` ${message}` } diff --git a/src/lifetime.ts b/src/lifetime.ts index 9ca44b5..39672d5 100644 --- a/src/lifetime.ts +++ b/src/lifetime.ts @@ -25,3 +25,10 @@ export const Lifetime: Record = { */ SCOPED: 'SCOPED', } + +export function isLifetimeLonger(a: LifetimeType, b: LifetimeType): boolean { + return ( + (a === Lifetime.SINGLETON && b !== Lifetime.SINGLETON) || + (a === Lifetime.SCOPED && b === Lifetime.TRANSIENT) + ) +} From 30782b547294077e0d672850b86b82cd10cef2d5 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Thu, 28 Dec 2023 17:24:38 -0500 Subject: [PATCH 02/17] Add additional test with injector proxy Confirms `errorOnShorterLivedDependencies` compatibility with per-module local injections. --- src/__tests__/container.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 50022ff..79feb66 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -470,6 +470,19 @@ describe('container', () => { ) }) + it('does not throw an error when an injector proxy is used and errorOnShorterLivedDependencies is set', () => { + const container = createContainer({ + errorOnShorterLivedDependencies: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.injected, { + lifetime: Lifetime.SCOPED, + }).inject(() => ({ injected: 'hah' })), + }) + + expect(container.resolve('first')).toBe('hah') + }) + it('behaves properly when the cradle is returned from an async function', async () => { const container = createContainer() container.register({ value: asValue(42) }) From ab47584149eb06087a869d6dcf98a9bd6ba0cc81 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Thu, 28 Dec 2023 17:50:29 -0500 Subject: [PATCH 03/17] Add support for lifetimes in asValue, default to scoped --- src/__tests__/container.test.ts | 37 +++++++++++++++++++++++++++++++++ src/resolvers.ts | 7 ++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 79feb66..4d913fe 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -483,6 +483,43 @@ describe('container', () => { expect(container.resolve('first')).toBe('hah') }) + it('allows for asValue() to be used when errorOnShorterLivedDependencies is set', () => { + const container = createContainer({ + errorOnShorterLivedDependencies: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.val, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction((cradle: any) => cradle.secondVal, { + lifetime: Lifetime.SINGLETON, + }), + val: asValue('hah'), + secondVal: asValue('foobar', { lifetime: Lifetime.SINGLETON }), + }) + + expect(container.resolve('first')).toBe('hah') + expect(container.resolve('second')).toBe('foobar') + }) + + it('correctly errors when a singleton parent depends on a scoped value and errorOnShorterLivedDependencies is set', () => { + const container = createContainer({ + errorOnShorterLivedDependencies: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SINGLETON, + }), + second: asValue('hah'), + }) + + const err = throws(() => container.resolve('first')) + expect(err.message).toContain('first -> second') + expect(err.message).toContain( + "Dependency 'second' has a shorter lifetime than its parent: 'first'", + ) + }) + it('behaves properly when the cradle is returned from an async function', async () => { const container = createContainer() container.register({ value: asValue(42) }) diff --git a/src/resolvers.ts b/src/resolvers.ts index 7f35504..e0e4353 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -116,6 +116,7 @@ export type Constructor = { new (...args: any[]): T } /** * Creates a simple value resolver where the given value will always be resolved. + * The lifetime of the resolved value defaults to `Lifetime.SCOPED`. * * @param {string} name * The name to register the value as. @@ -123,12 +124,16 @@ export type Constructor = { new (...args: any[]): T } * @param {*} value * The value to resolve. * + * @param {object} opts + * Additional options for this resolver. + * * @return {object} * The resolver. */ -export function asValue(value: T): Resolver { +export function asValue(value: T, opts?: ResolverOptions): Resolver { return { resolve: () => value, + ...{ lifetime: Lifetime.SCOPED, ...opts }, } } From 8cbdaa0d6161200375175eb2e80487b38a07d709 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Thu, 28 Dec 2023 17:59:44 -0500 Subject: [PATCH 04/17] Correct comment re: scoped and parent cache --- src/container.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/container.ts b/src/container.ts index 53957bf..c8defc0 100644 --- a/src/container.ts +++ b/src/container.ts @@ -543,8 +543,9 @@ export function createContainer( case Lifetime.SCOPED: // Scoped lifetime means that the container // that resolves the registration also caches it. - // When a registration is not found, we travel up - // the family tree until we find one that is cached. + // If this container cache does not have it, + // resolve and cache it rather than using the parent + // container's cache. cached = container.cache.get(name) if (cached !== undefined) { // We found one! From ced24e702f82012dcb603358097d744c7b296e47 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Sat, 30 Dec 2023 08:44:08 -0500 Subject: [PATCH 05/17] Update per review feedback --- README.md | 8 +++++--- src/__tests__/container.test.ts | 4 ++-- src/container.ts | 2 +- src/lifetime.ts | 3 +++ src/resolvers.ts | 3 ++- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index df830a4..87617fe 100644 --- a/README.md +++ b/README.md @@ -297,9 +297,6 @@ in the root container, and would always have the `currentUser` from the first request. Modules should generally not have a longer lifetime than their dependencies, as this can cause issues of stale data. -If you want a mismatched configuration like the above to error, set -`errorOnShorterLivedDependencies` in the container options. - ```js const makePrintTime = ({ time }) => () => { console.log('Time:', time) @@ -324,6 +321,11 @@ container.resolve('printTime')() container.resolve('printTime')() ``` +If you want a mismatched configuration like this to error, set +`errorOnShorterLivedDependencies` in the container options. This will trigger +the following error at runtime when the singleton `printTime` is resolved: +`AwilixResolutionError: Could not resolve 'time'. Dependency 'time' has a shorter lifetime than its ancestor: 'printTime'` + Read the documentation for [`container.createScope()`](#containercreatescope) for more examples. diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 4d913fe..82c880a 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -466,7 +466,7 @@ describe('container', () => { const err = throws(() => container.resolve('first')) expect(err.message).toContain('first -> second') expect(err.message).toContain( - "Dependency 'second' has a shorter lifetime than its parent: 'first'", + "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", ) }) @@ -516,7 +516,7 @@ describe('container', () => { const err = throws(() => container.resolve('first')) expect(err.message).toContain('first -> second') expect(err.message).toContain( - "Dependency 'second' has a shorter lifetime than its parent: 'first'", + "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", ) }) diff --git a/src/container.ts b/src/container.ts index c8defc0..f3d8615 100644 --- a/src/container.ts +++ b/src/container.ts @@ -512,7 +512,7 @@ export function createContainer( throw new AwilixResolutionError( name, resolutionStack, - `Dependency '${name.toString()}' has a shorter lifetime than its parent: '${resolutionStack[ + `Dependency '${name.toString()}' has a shorter lifetime than its ancestor: '${resolutionStack[ maybeLongerLifetimeParentIndex ].name.toString()}'`, ) diff --git a/src/lifetime.ts b/src/lifetime.ts index 39672d5..0ad9cdc 100644 --- a/src/lifetime.ts +++ b/src/lifetime.ts @@ -26,6 +26,9 @@ export const Lifetime: Record = { SCOPED: 'SCOPED', } +/** + * Returns true if and only if the first lifetime is strictly longer than the second. + */ export function isLifetimeLonger(a: LifetimeType, b: LifetimeType): boolean { return ( (a === Lifetime.SINGLETON && b !== Lifetime.SINGLETON) || diff --git a/src/resolvers.ts b/src/resolvers.ts index e0e4353..bad5e78 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -133,7 +133,8 @@ export type Constructor = { new (...args: any[]): T } export function asValue(value: T, opts?: ResolverOptions): Resolver { return { resolve: () => value, - ...{ lifetime: Lifetime.SCOPED, ...opts }, + lifetime: Lifetime.SCOPED, + ...opts, } } From 00f68ae3853444744c1410ec2d732c3529351cd1 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Sat, 30 Dec 2023 13:47:36 -0500 Subject: [PATCH 06/17] Add errorOnShorterLivedDependencies as recommended usage option Also update all examples to use it (and fix examples where necessary) --- README.md | 4 +++- examples/babel/.babelrc | 2 +- examples/babel/src/index.js | 1 + examples/koa/index.js | 10 ++++++---- examples/simple/index.js | 4 +++- examples/typescript/src/index.ts | 3 ++- examples/typescript/tsconfig.json | 5 +++-- 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 87617fe..54ec4c2 100644 --- a/README.md +++ b/README.md @@ -95,8 +95,10 @@ minimum, you need to do 3 things: const awilix = require('awilix') // Create the container and set the injectionMode to PROXY (which is also the default). +// Enable detection and error on mismatched dependency lifetimes (highly recommended). const container = awilix.createContainer({ - injectionMode: awilix.InjectionMode.PROXY + injectionMode: awilix.InjectionMode.PROXY, + errorOnShorterLivedDependencies: true }) // This is our app code... We can use diff --git a/examples/babel/.babelrc b/examples/babel/.babelrc index 002b4aa..c13c5f6 100644 --- a/examples/babel/.babelrc +++ b/examples/babel/.babelrc @@ -1,3 +1,3 @@ { - "presets": ["env"] + "presets": ["es2015"] } diff --git a/examples/babel/src/index.js b/examples/babel/src/index.js index 235b67a..9696385 100644 --- a/examples/babel/src/index.js +++ b/examples/babel/src/index.js @@ -4,6 +4,7 @@ import { DependentService } from './services/dependentService' const container = createContainer({ injectionMode: InjectionMode.CLASSIC, + errorOnShorterLivedDependencies: true, }) container.register({ diff --git a/examples/koa/index.js b/examples/koa/index.js index a2fbdcb..16ee7c7 100644 --- a/examples/koa/index.js +++ b/examples/koa/index.js @@ -13,14 +13,16 @@ const app = new Koa() const router = new KoaRouter() // Create a container. -const container = createContainer() +const container = createContainer({ errorOnShorterLivedDependencies: true }) -// Register usefull stuff +// Register useful stuff const MessageService = require('./services/MessageService') const makeMessageRepository = require('./repositories/messageRepository') container.register({ - // used by the repository. - DB_CONNECTION_STRING: asValue('localhost:1234'), + // used by the repository; registered. + DB_CONNECTION_STRING: asValue('localhost:1234', { + lifetime: awilix.Lifetime.SINGLETON, + }), // resolved for each request. messageService: asClass(MessageService).scoped(), // only resolved once diff --git a/examples/simple/index.js b/examples/simple/index.js index a56bcd3..0902010 100644 --- a/examples/simple/index.js +++ b/examples/simple/index.js @@ -2,7 +2,9 @@ const awilix = require('../..') // Create a container. -const container = awilix.createContainer() +const container = awilix.createContainer({ + errorOnShorterLivedDependencies: true, +}) // Register some value.. We depend on this in `Stuffs.js` container.register('database', awilix.asValue('stuffs_db')) diff --git a/examples/typescript/src/index.ts b/examples/typescript/src/index.ts index 8f6ba34..322c64a 100644 --- a/examples/typescript/src/index.ts +++ b/examples/typescript/src/index.ts @@ -1,5 +1,5 @@ // This is largely for testing, but import what we need -import { createContainer, asClass, InjectionMode } from '../../../src/awilix' +import { createContainer, asClass, InjectionMode } from '../../..' import TestService from './services/TestService' import DependentService from './services/DependentService' @@ -11,6 +11,7 @@ interface ICradle { // Create the container const container = createContainer({ injectionMode: InjectionMode.CLASSIC, + errorOnShorterLivedDependencies: true, }) // Register the classes diff --git a/examples/typescript/tsconfig.json b/examples/typescript/tsconfig.json index c902b97..ffecfb1 100644 --- a/examples/typescript/tsconfig.json +++ b/examples/typescript/tsconfig.json @@ -5,8 +5,9 @@ "outDir": "dist", "sourceMap": false, "target": "es5", - "noImplicitAny": true + "noImplicitAny": true, + "rootDir": "./src", }, - "include": ["src/**/*.ts"], + "include": ["**/*.ts"], "exclude": ["dist", "node_modules"] } From ddc89a723b44a0832b14f9ac349e6f3918b95722 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Mon, 1 Jan 2024 14:41:39 -0500 Subject: [PATCH 07/17] Rename 'errorOnShorterLivedDependencies' to 'strict' --- examples/babel/src/index.js | 2 +- examples/koa/index.js | 2 +- examples/simple/index.js | 2 +- examples/typescript/src/index.ts | 2 +- src/__tests__/container.test.ts | 16 ++++++++-------- src/container.ts | 20 +++++++++++--------- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/examples/babel/src/index.js b/examples/babel/src/index.js index 9696385..f4cced9 100644 --- a/examples/babel/src/index.js +++ b/examples/babel/src/index.js @@ -4,7 +4,7 @@ import { DependentService } from './services/dependentService' const container = createContainer({ injectionMode: InjectionMode.CLASSIC, - errorOnShorterLivedDependencies: true, + strict: true, }) container.register({ diff --git a/examples/koa/index.js b/examples/koa/index.js index 16ee7c7..638c6c2 100644 --- a/examples/koa/index.js +++ b/examples/koa/index.js @@ -13,7 +13,7 @@ const app = new Koa() const router = new KoaRouter() // Create a container. -const container = createContainer({ errorOnShorterLivedDependencies: true }) +const container = createContainer({ strict: true }) // Register useful stuff const MessageService = require('./services/MessageService') diff --git a/examples/simple/index.js b/examples/simple/index.js index 0902010..e551f79 100644 --- a/examples/simple/index.js +++ b/examples/simple/index.js @@ -3,7 +3,7 @@ const awilix = require('../..') // Create a container. const container = awilix.createContainer({ - errorOnShorterLivedDependencies: true, + strict: true, }) // Register some value.. We depend on this in `Stuffs.js` diff --git a/examples/typescript/src/index.ts b/examples/typescript/src/index.ts index 322c64a..38ca872 100644 --- a/examples/typescript/src/index.ts +++ b/examples/typescript/src/index.ts @@ -11,7 +11,7 @@ interface ICradle { // Create the container const container = createContainer({ injectionMode: InjectionMode.CLASSIC, - errorOnShorterLivedDependencies: true, + strict: true, }) // Register the classes diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 82c880a..d505dd0 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -452,9 +452,9 @@ describe('container', () => { expect(container.resolve('first')).toBe('hah') }) - it('throws an AwilixResolutionError when longer lifetime modules depend on shorter lifetime dependencies and errorOnShorterLivedDependencies is set', () => { + it('throws an AwilixResolutionError when longer lifetime modules depend on shorter lifetime dependencies and strict is set', () => { const container = createContainer({ - errorOnShorterLivedDependencies: true, + strict: true, }) container.register({ first: asFunction((cradle: any) => cradle.second, { @@ -470,9 +470,9 @@ describe('container', () => { ) }) - it('does not throw an error when an injector proxy is used and errorOnShorterLivedDependencies is set', () => { + it('does not throw an error when an injector proxy is used and strict is set', () => { const container = createContainer({ - errorOnShorterLivedDependencies: true, + strict: true, }) container.register({ first: asFunction((cradle: any) => cradle.injected, { @@ -483,9 +483,9 @@ describe('container', () => { expect(container.resolve('first')).toBe('hah') }) - it('allows for asValue() to be used when errorOnShorterLivedDependencies is set', () => { + it('allows for asValue() to be used when strict is set', () => { const container = createContainer({ - errorOnShorterLivedDependencies: true, + strict: true, }) container.register({ first: asFunction((cradle: any) => cradle.val, { @@ -502,9 +502,9 @@ describe('container', () => { expect(container.resolve('second')).toBe('foobar') }) - it('correctly errors when a singleton parent depends on a scoped value and errorOnShorterLivedDependencies is set', () => { + it('correctly errors when a singleton parent depends on a scoped value and strict is set', () => { const container = createContainer({ - errorOnShorterLivedDependencies: true, + strict: true, }) container.register({ first: asFunction((cradle: any) => cradle.second, { diff --git a/src/container.ts b/src/container.ts index f3d8615..b55c809 100644 --- a/src/container.ts +++ b/src/container.ts @@ -187,7 +187,7 @@ export type ClassOrFunctionReturning = FunctionReturning | Constructor export interface ContainerOptions { require?: (id: string) => any injectionMode?: InjectionModeType - errorOnShorterLivedDependencies?: boolean + strict?: boolean } /** @@ -219,21 +219,23 @@ const CRADLE_STRING_TAG = 'AwilixContainerCradle' /** * Creates an Awilix container instance. * - * @param {Function} options.require - * The require function to use. Defaults to require. + * @param {Function} options.require The require function to use. Defaults to require. * - * @param {string} options.injectionMode - * The mode used by the container to resolve dependencies. Defaults to 'Proxy'. + * @param {string} options.injectionMode The mode used by the container to resolve dependencies. + * Defaults to 'Proxy'. * - * @return {AwilixContainer} - * The container. + * @param {boolean} options.strict True if the container should run in strict mode with additional + * validation for resolver dependency correctness. Defaults to false. + * + * @return {AwilixContainer} The container. */ export function createContainer( - options?: ContainerOptions, + options: ContainerOptions = {}, parentContainer?: AwilixContainer, ): AwilixContainer { options = { injectionMode: InjectionMode.PROXY, + strict: false, ...options, } @@ -503,7 +505,7 @@ export function createContainer( // if any of the parents have a shorter lifetime than the one requested, // and the container is configured to do so, throw an error. - if (options?.errorOnShorterLivedDependencies) { + if (options?.strict) { const maybeLongerLifetimeParentIndex = resolutionStack.findIndex( ({ lifetime: parentLifetime }) => isLifetimeLonger(parentLifetime, lifetime), From 5ea979aaa16aa9bb68095ff569fccdc35450b0bf Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Mon, 1 Jan 2024 14:59:21 -0500 Subject: [PATCH 08/17] Add documentation for strict mode --- README.md | 243 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 139 insertions(+), 104 deletions(-) diff --git a/README.md b/README.md index 54ec4c2..ae0d829 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ [![JavaScript Style Guide](https://img.shields.io/badge/code%20style-standard-brightgreen.svg)](http://standardjs.com/) Extremely powerful, performant, & battle-tested **Dependency Injection** (DI) container for JavaScript/Node, -written in [TypeScript](http://typescriptlang.org). +written in [TypeScript](http://typescriptlang.org). Awilix enables you to write **composable, testable software** using dependency injection **without special annotations**, which in turn decouples your core application code from the intricacies of the DI mechanism. @@ -24,6 +24,7 @@ Awilix enables you to write **composable, testable software** using dependency i - [Usage](#usage) - [Lifetime management](#lifetime-management) - [Scoped lifetime](#scoped-lifetime) +- [Strict mode](#strict-mode) - [Injection modes](#injection-modes) - [Auto-loading modules](#auto-loading-modules) - [Per-module local injections](#per-module-local-injections) @@ -74,9 +75,9 @@ yarn add awilix You can also use the [UMD](https://github.com/umdjs/umd) build from `unpkg` ```html - ``` @@ -95,10 +96,10 @@ minimum, you need to do 3 things: const awilix = require('awilix') // Create the container and set the injectionMode to PROXY (which is also the default). -// Enable detection and error on mismatched dependency lifetimes (highly recommended). +// Enable strict mode for extra correctness checks (highly recommended). const container = awilix.createContainer({ injectionMode: awilix.InjectionMode.PROXY, - errorOnShorterLivedDependencies: true + strict: true, }) // This is our app code... We can use @@ -120,7 +121,7 @@ class UserController { container.register({ // Here we are telling Awilix how to resolve a // userController: by instantiating a class. - userController: awilix.asClass(UserController) + userController: awilix.asClass(UserController), }) // Let's try with a factory function. @@ -128,16 +129,16 @@ const makeUserService = ({ db }) => { // Notice how we can use destructuring // to access dependencies return { - getUser: id => { + getUser: (id) => { return db.query(`select * from users where id=${id}`) - } + }, } } container.register({ // the `userService` is resolved by // invoking the function. - userService: awilix.asFunction(makeUserService) + userService: awilix.asFunction(makeUserService), }) // Alright, now we need a database. @@ -151,7 +152,7 @@ function Database(connectionString, timeout) { this.conn = connectToYourDatabaseSomehow(connectionString, timeout) } -Database.prototype.query = function(sql) { +Database.prototype.query = function (sql) { // blah.... return this.conn.rawSql(sql) } @@ -161,7 +162,7 @@ Database.prototype.query = function(sql) { // We also want to use `CLASSIC` injection mode for this // registration. Read more about injection modes below. container.register({ - db: awilix.asClass(Database).classic() + db: awilix.asClass(Database).classic(), }) // Lastly we register the connection string and timeout values @@ -171,7 +172,7 @@ container.register({ // limited to strings and numbers, it can be anything, // really - they will be passed through directly. connectionString: awilix.asValue(process.env.CONN_STR), - timeout: awilix.asValue(1000) + timeout: awilix.asValue(1000), }) // We have now wired everything up! @@ -221,17 +222,17 @@ const { asClass, asFunction, asValue } = awilix class MailService {} container.register({ - mailService: asClass(MailService, { lifetime: Lifetime.SINGLETON }) + mailService: asClass(MailService, { lifetime: Lifetime.SINGLETON }), }) // or using the chaining configuration API.. container.register({ - mailService: asClass(MailService).setLifetime(Lifetime.SINGLETON) + mailService: asClass(MailService).setLifetime(Lifetime.SINGLETON), }) // or.. container.register({ - mailService: asClass(MailService).singleton() + mailService: asClass(MailService).singleton(), }) // or....... @@ -262,7 +263,7 @@ class MessageService { } container.register({ - messageService: asClass(MessageService).scoped() + messageService: asClass(MessageService).scoped(), }) // imagine middleware in some web framework.. @@ -272,7 +273,7 @@ app.use((req, res, next) => { // register some request-specific data.. req.scope.register({ - currentUser: asValue(req.user) + currentUser: asValue(req.user), }) next() @@ -281,7 +282,7 @@ app.use((req, res, next) => { app.get('/messages', (req, res) => { // for each request we get a new message service! const messageService = req.scope.resolve('messageService') - messageService.getMessages().then(messages => { + messageService.getMessages().then((messages) => { res.send(200, messages) }) }) @@ -300,15 +301,17 @@ request. Modules should generally not have a longer lifetime than their dependencies, as this can cause issues of stale data. ```js -const makePrintTime = ({ time }) => () => { - console.log('Time:', time) -} +const makePrintTime = + ({ time }) => + () => { + console.log('Time:', time) + } const getTime = () => new Date().toString() container.register({ printTime: asFunction(makePrintTime).singleton(), - time: asFunction(getTime).transient() + time: asFunction(getTime).transient(), }) // Resolving `time` 2 times will @@ -324,13 +327,32 @@ container.resolve('printTime')() ``` If you want a mismatched configuration like this to error, set -`errorOnShorterLivedDependencies` in the container options. This will trigger +`strict` in the container options. This will trigger the following error at runtime when the singleton `printTime` is resolved: `AwilixResolutionError: Could not resolve 'time'. Dependency 'time' has a shorter lifetime than its ancestor: 'printTime'` +In addition, registering a singleton on a scope other than the root container results in +unpredictable behavior. In particular, if two different singletons are registered on two different +scopes, they will share a cache entry and collide with each other. To throw a runtime error when a +singleton is registered on a scope other than the root container, set `strict` to true. + Read the documentation for [`container.createScope()`](#containercreatescope) for more examples. +# Strict mode + +Strict mode is a new feature in Awilix 9.1. It enables additional correctness +checks that can help you catch bugs early. + +In particular, strict mode enables the following checks: + +- When a singleton depends on a scoped or transient registration, or when a scoped registration + depends on a transient registration, an error is thrown. This detects and prevents the issue where + a shorter lifetime dependency can leak outside its intended lifetime due to its preservation in a + longer lifetime module. +- Singleton registrations on any scopes are disabled. This prevents the issue where a singleton is + registered on a scope other than the root container, which results in unpredictable behavior. + # Injection modes The injection mode determines how a function/constructor receives its @@ -364,11 +386,11 @@ modes are available on `awilix.InjectionMode` ``` - `InjectionMode.CLASSIC`: Parses the function/constructor parameters, and - matches them with registrations in the container. `CLASSIC` mode has a - slightly higher initialization cost as it has to parse the function/class - to figure out the dependencies at the time of registration, however resolving - them will be **much faster** than when using `PROXY`. _Don't use `CLASSIC` if - you minify your code!_ We recommend using `CLASSIC` in Node and `PROXY` in + matches them with registrations in the container. `CLASSIC` mode has a + slightly higher initialization cost as it has to parse the function/class + to figure out the dependencies at the time of registration, however resolving + them will be **much faster** than when using `PROXY`. _Don't use `CLASSIC` if + you minify your code!_ We recommend using `CLASSIC` in Node and `PROXY` in environments where minification is needed. ```js @@ -436,8 +458,8 @@ container.register({ const container = createContainer() container.loadModules(['services/**/*.js', 'repositories/**/*.js'], { resolverOptions: { - injectionMode: InjectionMode.CLASSIC - } + injectionMode: InjectionMode.CLASSIC, + }, }) ``` @@ -477,7 +499,7 @@ function database({ connectionString, timeout, logger }) { const db = database({ logger: new LoggerMock(), timeout: 4000, - connectionString: 'localhost:1337;user=123...' + connectionString: 'localhost:1337;user=123...', }) ``` @@ -534,35 +556,38 @@ const awilix = require('awilix') const container = awilix.createContainer() // Load our modules! -container.loadModules([ - // Globs! +container.loadModules( [ - // To have different resolverOptions for specific modules. - 'models/**/*.js', - { - register: awilix.asValue, - lifetime: Lifetime.SINGLETON - } + // Globs! + [ + // To have different resolverOptions for specific modules. + 'models/**/*.js', + { + register: awilix.asValue, + lifetime: Lifetime.SINGLETON, + }, + ], + 'services/**/*.js', + 'repositories/**/*.js', ], - 'services/**/*.js', - 'repositories/**/*.js' -], { - // We want to register `UserService` as `userService` - - // by default loaded modules are registered with the - // name of the file (minus the extension) - formatName: 'camelCase', - // Apply resolver options to all modules. - resolverOptions: { - // We can give these auto-loaded modules - // the deal of a lifetime! (see what I did there?) - // By default it's `TRANSIENT`. - lifetime: Lifetime.SINGLETON, - // We can tell Awilix what to register everything as, - // instead of guessing. If omitted, will inspect the - // module to determine what to register as. - register: awilix.asClass - } -}) + { + // We want to register `UserService` as `userService` - + // by default loaded modules are registered with the + // name of the file (minus the extension) + formatName: 'camelCase', + // Apply resolver options to all modules. + resolverOptions: { + // We can give these auto-loaded modules + // the deal of a lifetime! (see what I did there?) + // By default it's `TRANSIENT`. + lifetime: Lifetime.SINGLETON, + // We can tell Awilix what to register everything as, + // instead of guessing. If omitted, will inspect the + // module to determine what to register as. + register: awilix.asClass, + }, + }, +) // We are now ready! We now have a userService, userRepository and emailService! container.resolve('userService').getUser(1) @@ -588,10 +613,10 @@ export default function userRepository({ db, timeout }) { return Promise.race([ db.query('select * from users'), Promise.delay(timeout).then(() => - Promise.reject(new Error('Timed out')) - ) + Promise.reject(new Error('Timed out')), + ), ]) - } + }, } } ``` @@ -610,22 +635,22 @@ const container = createContainer() // Provide an injection function that returns an object with locals. // The function is called once per resolve of the registration // it is attached to. - .inject(() => ({ timeout: 2000 })) + .inject(() => ({ timeout: 2000 })), }) // Shorthand variants .register({ userRepository: asFunction(createUserRepository, { - injector: () => ({ timeout: 2000 }) - }) + injector: () => ({ timeout: 2000 }), + }), }) // Stringly-typed shorthand .register( 'userRepository', asFunction(createUserRepository, { - injector: () => ({ timeout: 2000 }) - }) + injector: () => ({ timeout: 2000 }), + }), ) // with `loadModules` @@ -658,7 +683,7 @@ export default class AwesomeService { // `RESOLVER` is a Symbol. AwesomeService[RESOLVER] = { lifetime: Lifetime.SCOPED, - injectionMode: InjectionMode.CLASSIC + injectionMode: InjectionMode.CLASSIC, } ``` @@ -669,7 +694,7 @@ import { createContainer, asClass } from 'awilix' import AwesomeService from './services/awesome-service.js' const container = createContainer().register({ - awesomeService: asClass(AwesomeService) + awesomeService: asClass(AwesomeService), }) console.log(container.registrations.awesomeService.lifetime) // 'SCOPED' @@ -733,7 +758,7 @@ function configureContainer() { .singleton() // This is called when the pool is going to be disposed. // If it returns a Promise, it will be awaited by `dispose`. - .disposer(pool => pool.end()) + .disposer((pool) => pool.end()), }) } @@ -814,13 +839,13 @@ pass in an object with the following props: ```js container.register({ - stuff: asClass(MyClass, { injectionMode: InjectionMode.CLASSIC }) + stuff: asClass(MyClass, { injectionMode: InjectionMode.CLASSIC }), }) container.loadModules([['some/path/to/*.js', { register: asClass }]], { resolverOptions: { - lifetime: Lifetime.SCOPED - } + lifetime: Lifetime.SCOPED, + }, }) ``` @@ -843,10 +868,11 @@ Args: **_must_** be named exactly like they are in the registration. For example, a dependency registered as `repository` cannot be referenced in a class constructor as `repo`. - - `options.errorOnShorterLivedDependencies`: If `true`, will throw an error if - a singleton depends on a scoped or transient registration, or if a scoped - registration depends on a transient registration. Defaults to - `false`. + - `options.strict`: Defaults to `false`; if `true`, will enable the following additional + correctness checks: + - Will throw an error if a singleton depends on a scoped or transient registration, or if a + scoped registration depends on a transient registration. + - Will throw an error if a singleton is registered on a scope other than the root container. ## `asFunction()` @@ -886,7 +912,7 @@ Resolves the dependency specified. ```js container.register({ val: asValue(123), - aliasVal: aliasTo('val') + aliasVal: aliasTo('val'), }) container.resolve('aliasVal') === container.resolve('val') @@ -959,7 +985,7 @@ Each scope has it's own cache, and checks the cache of it's ancestors. ```js let counter = 1 container.register({ - count: asFunction(() => counter++).singleton() + count: asFunction(() => counter++).singleton(), }) container.cradle.count === 1 @@ -975,7 +1001,7 @@ Options passed to `createContainer` are stored here. ```js const container = createContainer({ - injectionMode: InjectionMode.CLASSIC + injectionMode: InjectionMode.CLASSIC, }) console.log(container.options.injectionMode) // 'CLASSIC' @@ -991,7 +1017,7 @@ Resolves the registration with the given name. Used by the cradle. ```js container.register({ - leet: asFunction(() => 1337) + leet: asFunction(() => 1337), }) container.resolve('leet') === 1337 @@ -1039,14 +1065,14 @@ container.register('context', asClass(SessionContext)) container.register({ connectionString: asValue('localhost:1433;user=...'), mailService: asFunction(makeMailService, { lifetime: Lifetime.SINGLETON }), - context: asClass(SessionContext, { lifetime: Lifetime.SCOPED }) + context: asClass(SessionContext, { lifetime: Lifetime.SCOPED }), }) // `asClass` and `asFunction` also supports a fluid syntax. // This... container.register( 'mailService', - asFunction(makeMailService).setLifetime(Lifetime.SINGLETON) + asFunction(makeMailService).setLifetime(Lifetime.SINGLETON), ) // .. is the same as this: container.register('context', asClass(SessionContext).singleton()) @@ -1087,9 +1113,9 @@ Args: pass the name through as-is. The 2nd parameter is a full module descriptor. - `opts.resolverOptions`: An `object` passed to the resolvers. Used to configure the lifetime, injection mode and more of the loaded modules. -- `opts.esModules`: Loads modules using Node's native ES modules. - **This makes `container.loadModules` asynchronous, and will therefore return a `Promise`!** - This is only supported on Node 14.0+ and should only be used if you're using +- `opts.esModules`: Loads modules using Node's native ES modules. + **This makes `container.loadModules` asynchronous, and will therefore return a `Promise`!** + This is only supported on Node 14.0+ and should only be used if you're using the [Native Node ES modules](https://nodejs.org/api/esm.html) Example: @@ -1166,7 +1192,7 @@ inside a scope. A scope is basically a "child" container. // Increments the counter every time it is resolved. let counter = 1 container.register({ - counterValue: asFunction(() => counter++).scoped() + counterValue: asFunction(() => counter++).scoped(), }) const scope1 = container.createScope() const scope2 = container.createScope() @@ -1186,7 +1212,7 @@ A _Scope_ maintains it's own cache of `Lifetime.SCOPED` registrations, meaning i ```js let counter = 1 container.register({ - counterValue: asFunction(() => counter++).scoped() + counterValue: asFunction(() => counter++).scoped(), }) const scope1 = container.createScope() const scope2 = container.createScope() @@ -1212,13 +1238,13 @@ that scope and it's children. // that returns the value of the scope-provided dependency. // For this example we could also use scoped lifetime. container.register({ - scopedValue: asFunction(cradle => 'Hello ' + cradle.someValue) + scopedValue: asFunction((cradle) => 'Hello ' + cradle.someValue), }) // Create a scope and register a value. const scope = container.createScope() scope.register({ - someValue: asValue('scope') + someValue: asValue('scope'), }) scope.cradle.scopedValue === 'Hello scope' @@ -1228,27 +1254,36 @@ container.cradle.someValue // of the resolver. ``` -Things registered in the scope take precedence over it's parent. +Things registered in the scope take precedence over registrations in the parent scope(s). This +applies to both the registration directly requested from the scope container, and any dependencies +that the registration uses. ```js // It does not matter when the scope is created, // it will still have anything that is registered -// in it's parent. +// in its parent. const scope = container.createScope() container.register({ value: asValue('root'), - usedValue: asFunction(cradle => cradle.value) + usedValue: asFunction((cradle) => `hello from ${cradle.value}`), }) scope.register({ - value: asValue('scope') + value: asValue('scope'), }) -container.cradle.usedValue === 'root' -scope.cradle.usedValue === 'scope' +container.cradle.value === 'root' +scope.cradle.value === 'scope' +container.cradle.usedValue === 'hello from root' +scope.cradle.usedValue === 'hello from scope' ``` +Registering singletons in a scope results in unpredictable behavior and should be avoided. Having +more than one singleton with the same name in different scopes will result in them sharing a cache +entry and colliding with each other. To disallow such registrations, enable +[strict mode](#strict-mode) in the container options. + ### `container.build()` Builds an instance of a class (or a function) by injecting dependencies, but @@ -1283,11 +1318,11 @@ class MyClass { } const createMyFunc = ({ ping }) => ({ - pong: () => ping + pong: () => ping, }) container.register({ - ping: asValue('pong') + ping: asValue('pong'), }) // Shorthand @@ -1321,9 +1356,9 @@ const pg = require('pg') container.register({ pool: asFunction(() => new pg.Pool()) - .disposer(pool => pool.end()) + .disposer((pool) => pool.end()) // IMPORTANT! Must be either singleton or scoped! - .singleton() + .singleton(), }) const pool = container.resolve('pool') @@ -1365,11 +1400,11 @@ because they depend on Node-specific packages. # Ecosystem -* [`awilix-manager`](https://github.com/kibertoad/awilix-manager): Wrapper that allows eager injection, asynchronous init methods and dependency lookup by tags. -* [`awilix-express`](https://github.com/jeffijoe/awilix-express): Bindings for the Express HTTP library. -* [`awilix-koa`](https://github.com/jeffijoe/awilix-koa): Bindings for the Koa HTTP library. -* [`awilix-router-core`](https://github.com/jeffijoe/awilix-router-core): Library for building HTTP bindings for Awilix with routing. -* [`fastify-awilix`](https://github.com/fastify/fastify-awilix): Bindings for the Fastify framework. +- [`awilix-manager`](https://github.com/kibertoad/awilix-manager): Wrapper that allows eager injection, asynchronous init methods and dependency lookup by tags. +- [`awilix-express`](https://github.com/jeffijoe/awilix-express): Bindings for the Express HTTP library. +- [`awilix-koa`](https://github.com/jeffijoe/awilix-koa): Bindings for the Koa HTTP library. +- [`awilix-router-core`](https://github.com/jeffijoe/awilix-router-core): Library for building HTTP bindings for Awilix with routing. +- [`fastify-awilix`](https://github.com/fastify/fastify-awilix): Bindings for the Fastify framework. # Contributing From 787bf4534037d0db526dd829766d3e8955d74183 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Mon, 1 Jan 2024 15:25:20 -0500 Subject: [PATCH 09/17] Add full strict mode support Update `asValue()` to automatically set lifetime based upon registration container (root vs scope) Add checks to throw an error when a singleton is registered on a scope in strict mode --- src/__tests__/container.test.ts | 208 ++++++++++++++++++++------------ src/container.ts | 31 ++++- src/errors.ts | 21 ++++ src/resolvers.ts | 26 ++-- 4 files changed, 189 insertions(+), 97 deletions(-) diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index d505dd0..df4e397 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -440,86 +440,6 @@ describe('container', () => { expect(err.message).toContain('lol') }) - it('allows longer lifetime modules to depend on shorter lifetime dependencies by default', () => { - const container = createContainer() - container.register({ - first: asFunction((cradle: any) => cradle.second, { - lifetime: Lifetime.SCOPED, - }), - second: asFunction(() => 'hah'), - }) - - expect(container.resolve('first')).toBe('hah') - }) - - it('throws an AwilixResolutionError when longer lifetime modules depend on shorter lifetime dependencies and strict is set', () => { - const container = createContainer({ - strict: true, - }) - container.register({ - first: asFunction((cradle: any) => cradle.second, { - lifetime: Lifetime.SCOPED, - }), - second: asFunction(() => 'hah'), - }) - - const err = throws(() => container.resolve('first')) - expect(err.message).toContain('first -> second') - expect(err.message).toContain( - "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", - ) - }) - - it('does not throw an error when an injector proxy is used and strict is set', () => { - const container = createContainer({ - strict: true, - }) - container.register({ - first: asFunction((cradle: any) => cradle.injected, { - lifetime: Lifetime.SCOPED, - }).inject(() => ({ injected: 'hah' })), - }) - - expect(container.resolve('first')).toBe('hah') - }) - - it('allows for asValue() to be used when strict is set', () => { - const container = createContainer({ - strict: true, - }) - container.register({ - first: asFunction((cradle: any) => cradle.val, { - lifetime: Lifetime.SCOPED, - }), - second: asFunction((cradle: any) => cradle.secondVal, { - lifetime: Lifetime.SINGLETON, - }), - val: asValue('hah'), - secondVal: asValue('foobar', { lifetime: Lifetime.SINGLETON }), - }) - - expect(container.resolve('first')).toBe('hah') - expect(container.resolve('second')).toBe('foobar') - }) - - it('correctly errors when a singleton parent depends on a scoped value and strict is set', () => { - const container = createContainer({ - strict: true, - }) - container.register({ - first: asFunction((cradle: any) => cradle.second, { - lifetime: Lifetime.SINGLETON, - }), - second: asValue('hah'), - }) - - const err = throws(() => container.resolve('first')) - expect(err.message).toContain('first -> second') - expect(err.message).toContain( - "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", - ) - }) - it('behaves properly when the cradle is returned from an async function', async () => { const container = createContainer() container.register({ value: asValue(42) }) @@ -670,6 +590,134 @@ describe('container', () => { expect(theAnswer()).toBe(42) }) }) + + describe('automatic asValue lifetime handling', () => { + it('sets values to singleton lifetime when registered on the root container', () => { + const container = createContainer() + container.register({ + val: asValue(42), + }) + const registration = container.getRegistration('val') + expect(registration).toBeTruthy() + expect(registration!.lifetime).toBe(Lifetime.SINGLETON) + }) + it('sets values to scoped lifetime when registered on a scope container', () => { + const container = createContainer() + const scope = container.createScope() + scope.register({ + val: asValue(42), + }) + const registration = scope.getRegistration('val') + expect(registration).toBeTruthy() + expect(registration!.lifetime).toBe(Lifetime.SCOPED) + }) + }) + + describe('strict mode', () => { + describe('lifetime mismatch check', () => { + it('allows longer lifetime modules to depend on shorter lifetime dependencies by default', () => { + const container = createContainer() + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction(() => 'hah'), + }) + + expect(container.resolve('first')).toBe('hah') + }) + + it('throws an AwilixResolutionError when longer lifetime modules depend on shorter lifetime dependencies and strict is set', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction(() => 'hah'), + }) + + const err = throws(() => container.resolve('first')) + expect(err.message).toContain('first -> second') + expect(err.message).toContain( + "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", + ) + }) + + it('does not throw an error when an injector proxy is used and strict is set', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.injected, { + lifetime: Lifetime.SCOPED, + }).inject(() => ({ injected: 'hah' })), + }) + + expect(container.resolve('first')).toBe('hah') + }) + + it('allows for asValue() to be used when strict is set', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.val, { + lifetime: Lifetime.SCOPED, + }), + second: asFunction((cradle: any) => cradle.secondVal, { + lifetime: Lifetime.SINGLETON, + }), + val: asValue('hah'), + secondVal: asValue('foobar'), + }) + + expect(container.resolve('first')).toBe('hah') + expect(container.resolve('second')).toBe('foobar') + }) + + it('correctly errors when a singleton parent depends on a scoped value and strict is set', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SINGLETON, + }), + second: asValue('hah'), + }) + const scope = container.createScope() + scope.register({ + second: asValue('foobar'), + }) + + const err = throws(() => scope.resolve('first')) + expect(err.message).toContain('first -> second') + expect(err.message).toContain( + "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", + ) + }) + }) + + describe('singleton registration on scope check', () => { + it('detects and errors when a singleton is registered on a scope', () => { + const container = createContainer({ + strict: true, + }) + const scope = container.createScope() + const err = throws(() => + scope.register({ + test: asFunction(() => 42, { lifetime: Lifetime.SINGLETON }), + }), + ) + expect(err.message).toContain("Could not register 'test'") + expect(err.message).toContain( + 'Cannot register a singleton on a scoped container', + ) + }) + }) + }) }) describe('setting a name on the registration options', () => { diff --git a/src/container.ts b/src/container.ts index b55c809..0709a77 100644 --- a/src/container.ts +++ b/src/container.ts @@ -16,7 +16,11 @@ import { import { last, nameValueToObject, isClass } from './utils' import { InjectionMode, InjectionModeType } from './injection-mode' import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' -import { AwilixResolutionError, AwilixTypeError } from './errors' +import { + AwilixRegistrationError, + AwilixResolutionError, + AwilixTypeError, +} from './errors' import { importModule } from './load-module-native.js' /** @@ -403,8 +407,29 @@ export function createContainer( const keys = [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)] for (const key of keys) { - const value = obj[key as any] as Resolver - registrations[key as any] = value + const resolver = obj[key as any] as Resolver + // If strict mode is enabled, check to ensure we are not registering a singleton on a non-root + // container. + if (options?.strict && resolver.lifetime === Lifetime.SINGLETON) { + if (parentContainer) { + throw new AwilixRegistrationError( + key, + 'Cannot register a singleton on a scoped container.', + ) + } + } + + // If this is a value resolver, set the lifetime to `Lifetime.SCOPED` if this is a scoped + // container, or `Lifetime.SINGLETON` if this is the root container. + if (resolver.isValue) { + if (parentContainer != null) { + resolver.lifetime = Lifetime.SCOPED + } else { + resolver.lifetime = Lifetime.SINGLETON + } + } + + registrations[key as any] = resolver } return container diff --git a/src/errors.ts b/src/errors.ts index 5478f47..282f93c 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -151,3 +151,24 @@ export class AwilixResolutionError extends AwilixError { super(msg) } } + +/** + * A nice error class so we can do an instanceOf check. + */ +export class AwilixRegistrationError extends AwilixError { + /** + * Constructor, takes the registered modules and unresolved tokens + * to create a message. + * + * @param {string|symbol} name + * The name of the module that could not be registered. + */ + constructor(name: string | symbol, message?: string) { + const stringName = name.toString() + let msg = `Could not register '${stringName}'.` + if (message) { + msg += ` ${message}` + } + super(msg) + } +} diff --git a/src/resolvers.ts b/src/resolvers.ts index bad5e78..9dc75dd 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -30,6 +30,7 @@ export type InjectorFunction = ( */ export interface Resolver extends ResolverOptions { resolve(container: AwilixContainer): T + isValue: boolean } /** @@ -115,26 +116,20 @@ export interface BuildResolverOptions export type Constructor = { new (...args: any[]): T } /** - * Creates a simple value resolver where the given value will always be resolved. - * The lifetime of the resolved value defaults to `Lifetime.SCOPED`. + * Creates a simple value resolver where the given value will always be resolved. The lifetime of + * the resolved value defaults to `Lifetime.SCOPED` if registered on a scoped container, and + * `Lifetime.SINGLETON` if registered on the root container. * - * @param {string} name - * The name to register the value as. + * @param {string} name The name to register the value as. * - * @param {*} value - * The value to resolve. - * - * @param {object} opts - * Additional options for this resolver. + * @param {*} value The value to resolve. * - * @return {object} - * The resolver. + * @return {object} The resolver. */ -export function asValue(value: T, opts?: ResolverOptions): Resolver { +export function asValue(value: T): Resolver { return { resolve: () => value, - lifetime: Lifetime.SCOPED, - ...opts, + isValue: true, } } @@ -171,6 +166,7 @@ export function asFunction( const resolve = generateResolve(fn) const result = { resolve, + isValue: false, ...opts, } @@ -215,6 +211,7 @@ export function asClass( return createDisposableResolver( createBuildResolver({ ...opts, + isValue: false, resolve, }), ) @@ -230,6 +227,7 @@ export function aliasTo( resolve(container) { return container.resolve(name) }, + isValue: false, } } From 258476ed5306cb41fba37e7a756f66b35a8efc52 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Tue, 2 Jan 2024 10:24:05 -0500 Subject: [PATCH 10/17] Update per review - Add `isLeakSafe` to resolver to support excluding alias registrations from lifetime leak checking (since their targets will be checked) - Add tests for alias registration lifetime leak checking - Add `ResolverInternal` interface to include optional `isLeakSafe` and `isValue` params, which should not be exposed in the public library interface --- README.md | 6 +--- src/__tests__/container.test.ts | 40 +++++++++++++++++++++++++- src/container.ts | 50 +++++++++++++++++++-------------- src/resolvers.ts | 18 ++++++------ src/types.ts | 16 +++++++++++ 5 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 src/types.ts diff --git a/README.md b/README.md index ae0d829..a0ee487 100644 --- a/README.md +++ b/README.md @@ -868,11 +868,7 @@ Args: **_must_** be named exactly like they are in the registration. For example, a dependency registered as `repository` cannot be referenced in a class constructor as `repo`. - - `options.strict`: Defaults to `false`; if `true`, will enable the following additional - correctness checks: - - Will throw an error if a singleton depends on a scoped or transient registration, or if a - scoped registration depends on a transient registration. - - Will throw an error if a singleton is registered on a scope other than the root container. + - `options.strict`: Enables [strict mode](#strict-mode). Defaults to `false`. ## `asFunction()` diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index df4e397..c175558 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -3,7 +3,7 @@ import * as util from 'util' import { createContainer, AwilixContainer } from '../container' import { Lifetime } from '../lifetime' import { AwilixResolutionError } from '../errors' -import { asClass, asFunction, asValue } from '../resolvers' +import { aliasTo, asClass, asFunction, asValue } from '../resolvers' import { InjectionMode } from '../injection-mode' class Test { @@ -698,6 +698,44 @@ describe('container', () => { "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", ) }) + + it('allows aliasTo to be used when strict is set', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SINGLETON, + }), + second: aliasTo('val'), + val: asValue('hah'), + }) + + expect(container.resolve('first')).toBe('hah') + }) + + it('detects when an aliasTo resolution violates lifetime constraints', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + first: asFunction((cradle: any) => cradle.second, { + lifetime: Lifetime.SINGLETON, + }), + second: aliasTo('val'), + val: asValue('hah'), + }) + const scope = container.createScope() + scope.register({ + val: asValue('foobar'), + }) + + const err = throws(() => scope.resolve('first')) + expect(err.message).toContain('first -> second -> val') + expect(err.message).toContain( + "Dependency 'val' has a shorter lifetime than its ancestor: 'first'", + ) + }) }) describe('singleton registration on scope check', () => { diff --git a/src/container.ts b/src/container.ts index 0709a77..2fb957c 100644 --- a/src/container.ts +++ b/src/container.ts @@ -1,27 +1,28 @@ import * as util from 'util' +import { + AwilixRegistrationError, + AwilixResolutionError, + AwilixTypeError, +} from './errors' +import { InjectionMode, InjectionModeType } from './injection-mode' +import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' import { GlobWithOptions, listModules } from './list-modules' +import { importModule } from './load-module-native.js' import { LoadModulesOptions, - loadModules as realLoadModules, LoadModulesResult, + loadModules as realLoadModules, } from './load-modules' import { - Resolver, + BuildResolverOptions, Constructor, + DisposableResolver, + Resolver, asClass, asFunction, - DisposableResolver, - BuildResolverOptions, } from './resolvers' -import { last, nameValueToObject, isClass } from './utils' -import { InjectionMode, InjectionModeType } from './injection-mode' -import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' -import { - AwilixRegistrationError, - AwilixResolutionError, - AwilixTypeError, -} from './errors' -import { importModule } from './load-module-native.js' +import { ResolverInternal } from './types' +import { isClass, last, nameValueToObject } from './utils' /** * The container returned from createContainer has some methods and properties. @@ -70,7 +71,7 @@ export interface AwilixContainer { /** * Adds a single registration that using a pre-constructed resolver. */ - register(name: string | symbol, registration: Resolver): this + register(name: string | symbol, registration: ResolverInternal): this /** * Pairs resolvers to registration names and registers them. */ @@ -114,14 +115,18 @@ export interface AwilixContainer { * * @param name {string | symbol} The registration name. */ - getRegistration(name: K): Resolver | null + getRegistration( + name: K, + ): ResolverInternal | null /** * Recursively gets a registration by name if it exists in the * current container or any of its' parents. * * @param name {string | symbol} The registration name. */ - getRegistration(name: string | symbol): Resolver | null + getRegistration( + name: string | symbol, + ): ResolverInternal | null /** * Given a resolver, class or function, builds it up and returns it. * Does not cache it, this means that any lifetime configured in case of passing @@ -160,7 +165,7 @@ export interface CacheEntry { /** * The resolver that resolved the value. */ - resolver: Resolver + resolver: ResolverInternal /** * The resolved value. */ @@ -172,7 +177,7 @@ export interface CacheEntry { * @interface NameAndRegistrationPair */ export type NameAndRegistrationPair = { - [U in keyof T]?: Resolver + [U in keyof T]?: ResolverInternal } /** @@ -197,7 +202,10 @@ export interface ContainerOptions { /** * Contains a hash of registrations where the name is the key. */ -export type RegistrationHash = Record> +export type RegistrationHash = Record< + string | symbol | number, + ResolverInternal +> /** * Resolution stack. @@ -407,7 +415,7 @@ export function createContainer( const keys = [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)] for (const key of keys) { - const resolver = obj[key as any] as Resolver + const resolver = obj[key as any] as ResolverInternal // If strict mode is enabled, check to ensure we are not registering a singleton on a non-root // container. if (options?.strict && resolver.lifetime === Lifetime.SINGLETON) { @@ -530,7 +538,7 @@ export function createContainer( // if any of the parents have a shorter lifetime than the one requested, // and the container is configured to do so, throw an error. - if (options?.strict) { + if (options?.strict && !resolver.isLeakSafe) { const maybeLongerLifetimeParentIndex = resolutionStack.findIndex( ({ lifetime: parentLifetime }) => isLifetimeLonger(parentLifetime, lifetime), diff --git a/src/resolvers.ts b/src/resolvers.ts index 9dc75dd..23d8b0f 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -1,9 +1,10 @@ -import { Lifetime, LifetimeType } from './lifetime' +import { AwilixContainer, FunctionReturning, ResolveOptions } from './container' +import { AwilixTypeError } from './errors' import { InjectionMode, InjectionModeType } from './injection-mode' +import { Lifetime, LifetimeType } from './lifetime' +import { Parameter, parseParameterList } from './param-parser' +import { ResolverInternal } from './types' import { isFunction, uniq } from './utils' -import { parseParameterList, Parameter } from './param-parser' -import { AwilixTypeError } from './errors' -import { AwilixContainer, FunctionReturning, ResolveOptions } from './container' // We parse the signature of any `Function`, so we want to allow `Function` types. /* eslint-disable @typescript-eslint/ban-types */ @@ -30,7 +31,6 @@ export type InjectorFunction = ( */ export interface Resolver extends ResolverOptions { resolve(container: AwilixContainer): T - isValue: boolean } /** @@ -126,7 +126,7 @@ export type Constructor = { new (...args: any[]): T } * * @return {object} The resolver. */ -export function asValue(value: T): Resolver { +export function asValue(value: T): ResolverInternal { return { resolve: () => value, isValue: true, @@ -166,7 +166,6 @@ export function asFunction( const resolve = generateResolve(fn) const result = { resolve, - isValue: false, ...opts, } @@ -211,7 +210,6 @@ export function asClass( return createDisposableResolver( createBuildResolver({ ...opts, - isValue: false, resolve, }), ) @@ -222,12 +220,12 @@ export function asClass( */ export function aliasTo( name: Parameters[0], -): Resolver { +): ResolverInternal { return { resolve(container) { return container.resolve(name) }, - isValue: false, + isLeakSafe: true, } } diff --git a/src/types.ts b/src/types.ts new file mode 100644 index 0000000..ad7a376 --- /dev/null +++ b/src/types.ts @@ -0,0 +1,16 @@ +import { Resolver } from './resolvers' + +export interface ResolverInternal extends Resolver { + /** + * True if this resolver is a value resolver. Used to implicit set the lifetime of a value + * resolver to either {@link Lifetime.SCOPED} or {@link Lifetime.SINGLETON} depending on whether + * this value is registered to a root or a scope container. + */ + isValue?: boolean + + /** + * True if this resolver should be excluded from lifetime leak checking. Used to exclude alias + * resolvers since any lifetime issues will be caught in the resolution of the alias target. + */ + isLeakSafe?: boolean +} From 31c464954e79faa34cf25a64c5b0d8369528e478 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Tue, 2 Jan 2024 10:52:42 -0500 Subject: [PATCH 11/17] Remove internal ResolutionStack from library external interface --- src/container.ts | 20 +++++++------------- src/errors.ts | 2 +- src/resolvers.ts | 4 ++-- src/types.ts | 4 ++++ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/container.ts b/src/container.ts index 2fb957c..a0df486 100644 --- a/src/container.ts +++ b/src/container.ts @@ -5,7 +5,7 @@ import { AwilixTypeError, } from './errors' import { InjectionMode, InjectionModeType } from './injection-mode' -import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' +import { Lifetime, isLifetimeLonger } from './lifetime' import { GlobWithOptions, listModules } from './list-modules' import { importModule } from './load-module-native.js' import { @@ -21,7 +21,7 @@ import { asClass, asFunction, } from './resolvers' -import { ResolverInternal } from './types' +import { ResolutionStack, ResolverInternal } from './types' import { isClass, last, nameValueToObject } from './utils' /** @@ -207,12 +207,6 @@ export type RegistrationHash = Record< ResolverInternal > -/** - * Resolution stack. - */ -export interface ResolutionStack - extends Array<{ name: string | symbol; lifetime: LifetimeType }> {} - /** * Family tree symbol. */ @@ -251,10 +245,10 @@ export function createContainer( ...options, } - // The resolution stack is used to keep track - // of what modules are being resolved, so when - // an error occurs, we have something to present - // to the poor developer who fucked up. + /** + * Tracks the names and lifetimes of the modules being resolved. Used to detect circular + * dependencies and, in strict mode, lifetime leakage issues. + */ let resolutionStack: ResolutionStack = [] // Internal registration store for this container. @@ -627,7 +621,7 @@ export function createContainer( * Does not cache it, this means that any lifetime configured in case of passing * a registration will not be used. * - * @param {Resolver|Class|Function} targetOrResolver + * @param {Resolver|Constructor|Function} targetOrResolver * @param {ResolverOptions} opts */ function build( diff --git a/src/errors.ts b/src/errors.ts index 282f93c..1847fc8 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,4 +1,4 @@ -import { ResolutionStack } from './container' +import { ResolutionStack } from './types' /** * Newline. diff --git a/src/resolvers.ts b/src/resolvers.ts index 23d8b0f..ea0d1f9 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -239,7 +239,7 @@ export function aliasTo( * @return {object} * The interface. */ -export function createBuildResolver>( +function createBuildResolver>( obj: B, ): BuildResolver & B { function setLifetime(this: any, value: LifetimeType) { @@ -280,7 +280,7 @@ export function createBuildResolver>( * function. * @param obj */ -export function createDisposableResolver>( +function createDisposableResolver>( obj: B, ): DisposableResolver & B { function disposer(this: any, dispose: Disposer) { diff --git a/src/types.ts b/src/types.ts index ad7a376..84f4ccd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,3 +1,4 @@ +import { LifetimeType } from './lifetime' import { Resolver } from './resolvers' export interface ResolverInternal extends Resolver { @@ -14,3 +15,6 @@ export interface ResolverInternal extends Resolver { */ isLeakSafe?: boolean } + +export interface ResolutionStack + extends Array<{ name: string | symbol; lifetime: LifetimeType }> {} From d8a2680f9715542c6321f597780e5c3f188dbf22 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 11:07:57 -0500 Subject: [PATCH 12/17] Update strict mode to only resolve singletons in root context Remove `asValue` lifetime support, use `isLeakSafe` instead. --- README.md | 11 +++-- src/__tests__/container.test.ts | 83 +++++++++++++++------------------ src/container.ts | 69 ++++++++++++++------------- src/resolvers.ts | 21 +++++---- src/types.ts | 22 ++------- 5 files changed, 98 insertions(+), 108 deletions(-) diff --git a/README.md b/README.md index a0ee487..091ac1a 100644 --- a/README.md +++ b/README.md @@ -346,12 +346,13 @@ checks that can help you catch bugs early. In particular, strict mode enables the following checks: -- When a singleton depends on a scoped or transient registration, or when a scoped registration - depends on a transient registration, an error is thrown. This detects and prevents the issue where - a shorter lifetime dependency can leak outside its intended lifetime due to its preservation in a - longer lifetime module. +- When a singleton or scoped registration depends on a transient non-value registration, an error is + thrown. This detects and prevents the issue where a shorter lifetime dependency can leak outside + its intended lifetime due to its preservation in a longer lifetime module. - Singleton registrations on any scopes are disabled. This prevents the issue where a singleton is registered on a scope other than the root container, which results in unpredictable behavior. +- Singleton resolution is performed using registrations from the root container only, which prevents + potential leaks in which scoped registrations are preserved in singletons. # Injection modes @@ -834,6 +835,8 @@ pass in an object with the following props: [Per-module local injections](#per-module-local-injections) - `register`: Only used in `loadModules`, determines how to register a loaded module explicitly +- `isLeakSafe`: true if this resolver should be excluded from lifetime-leak checking performed in + [strict mode](#strict-mode). Defaults to false. **Examples of usage:** diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index c175558..0600afb 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -591,28 +591,6 @@ describe('container', () => { }) }) - describe('automatic asValue lifetime handling', () => { - it('sets values to singleton lifetime when registered on the root container', () => { - const container = createContainer() - container.register({ - val: asValue(42), - }) - const registration = container.getRegistration('val') - expect(registration).toBeTruthy() - expect(registration!.lifetime).toBe(Lifetime.SINGLETON) - }) - it('sets values to scoped lifetime when registered on a scope container', () => { - const container = createContainer() - const scope = container.createScope() - scope.register({ - val: asValue(42), - }) - const registration = scope.getRegistration('val') - expect(registration).toBeTruthy() - expect(registration!.lifetime).toBe(Lifetime.SCOPED) - }) - }) - describe('strict mode', () => { describe('lifetime mismatch check', () => { it('allows longer lifetime modules to depend on shorter lifetime dependencies by default', () => { @@ -677,7 +655,7 @@ describe('container', () => { expect(container.resolve('second')).toBe('foobar') }) - it('correctly errors when a singleton parent depends on a scoped value and strict is set', () => { + it('allows aliasTo to be used when strict is set', () => { const container = createContainer({ strict: true, }) @@ -685,56 +663,71 @@ describe('container', () => { first: asFunction((cradle: any) => cradle.second, { lifetime: Lifetime.SINGLETON, }), - second: asValue('hah'), - }) - const scope = container.createScope() - scope.register({ - second: asValue('foobar'), + second: aliasTo('val'), + val: asValue('hah'), }) - const err = throws(() => scope.resolve('first')) - expect(err.message).toContain('first -> second') - expect(err.message).toContain( - "Dependency 'second' has a shorter lifetime than its ancestor: 'first'", - ) + expect(container.resolve('first')).toBe('hah') }) - it('allows aliasTo to be used when strict is set', () => { + it('detects when an aliasTo resolution violates lifetime constraints', () => { const container = createContainer({ strict: true, }) container.register({ first: asFunction((cradle: any) => cradle.second, { - lifetime: Lifetime.SINGLETON, + lifetime: Lifetime.SCOPED, }), second: aliasTo('val'), val: asValue('hah'), }) + const scope = container.createScope() + scope.register({ + val: asFunction(() => 'foobar'), + }) - expect(container.resolve('first')).toBe('hah') + const err = throws(() => scope.resolve('first')) + expect(err.message).toContain('first -> second -> val') + expect(err.message).toContain( + "Dependency 'val' has a shorter lifetime than its ancestor: 'first'", + ) }) + }) - it('detects when an aliasTo resolution violates lifetime constraints', () => { + describe('singleton resolution using only root container', () => { + it('resolves singletons using root container only, even if called from scope', () => { const container = createContainer({ strict: true, }) container.register({ - first: asFunction((cradle: any) => cradle.second, { + scoped: asFunction((cradle: any) => cradle.val, { + lifetime: Lifetime.SCOPED, + }), + singleton: asFunction((cradle: any) => cradle.val, { lifetime: Lifetime.SINGLETON, }), - second: aliasTo('val'), - val: asValue('hah'), }) const scope = container.createScope() + + let err = throws(() => scope.resolve('scoped')) + expect(err).toBeInstanceOf(AwilixResolutionError) + expect(err.message).toContain('scoped -> val') + scope.register({ val: asValue('foobar'), }) - const err = throws(() => scope.resolve('first')) - expect(err.message).toContain('first -> second -> val') - expect(err.message).toContain( - "Dependency 'val' has a shorter lifetime than its ancestor: 'first'", - ) + expect(scope.resolve('scoped')).toBe('foobar') + + err = throws(() => scope.resolve('singleton')) + expect(err).toBeInstanceOf(AwilixResolutionError) + expect(err.message).toContain('singleton -> val') + + container.register({ + val: asValue('hah'), + }) + + expect(scope.resolve('singleton')).toBe('hah') }) }) diff --git a/src/container.ts b/src/container.ts index a0df486..6ac5c5a 100644 --- a/src/container.ts +++ b/src/container.ts @@ -21,7 +21,7 @@ import { asClass, asFunction, } from './resolvers' -import { ResolutionStack, ResolverInternal } from './types' +import { ResolutionStack } from './types' import { isClass, last, nameValueToObject } from './utils' /** @@ -71,7 +71,7 @@ export interface AwilixContainer { /** * Adds a single registration that using a pre-constructed resolver. */ - register(name: string | symbol, registration: ResolverInternal): this + register(name: string | symbol, registration: Resolver): this /** * Pairs resolvers to registration names and registers them. */ @@ -115,18 +115,14 @@ export interface AwilixContainer { * * @param name {string | symbol} The registration name. */ - getRegistration( - name: K, - ): ResolverInternal | null + getRegistration(name: K): Resolver | null /** * Recursively gets a registration by name if it exists in the * current container or any of its' parents. * * @param name {string | symbol} The registration name. */ - getRegistration( - name: string | symbol, - ): ResolverInternal | null + getRegistration(name: string | symbol): Resolver | null /** * Given a resolver, class or function, builds it up and returns it. * Does not cache it, this means that any lifetime configured in case of passing @@ -165,7 +161,7 @@ export interface CacheEntry { /** * The resolver that resolved the value. */ - resolver: ResolverInternal + resolver: Resolver /** * The resolved value. */ @@ -177,7 +173,7 @@ export interface CacheEntry { * @interface NameAndRegistrationPair */ export type NameAndRegistrationPair = { - [U in keyof T]?: ResolverInternal + [U in keyof T]?: Resolver } /** @@ -202,10 +198,7 @@ export interface ContainerOptions { /** * Contains a hash of registrations where the name is the key. */ -export type RegistrationHash = Record< - string | symbol | number, - ResolverInternal -> +export type RegistrationHash = Record> /** * Family tree symbol. @@ -231,13 +224,24 @@ const CRADLE_STRING_TAG = 'AwilixContainerCradle' * Defaults to 'Proxy'. * * @param {boolean} options.strict True if the container should run in strict mode with additional - * validation for resolver dependency correctness. Defaults to false. + * validation for resolver configuration correctness. Defaults to false. * * @return {AwilixContainer} The container. */ export function createContainer( options: ContainerOptions = {}, parentContainer?: AwilixContainer, +): AwilixContainer { + return createContainerInternal(options, parentContainer) +} + +function createContainerInternal< + T extends object = any, + U extends object = any, +>( + options: ContainerOptions = {}, + parentContainer?: AwilixContainer, + parentResolutionStack?: ResolutionStack, ): AwilixContainer { options = { injectionMode: InjectionMode.PROXY, @@ -249,7 +253,7 @@ export function createContainer( * Tracks the names and lifetimes of the modules being resolved. Used to detect circular * dependencies and, in strict mode, lifetime leakage issues. */ - let resolutionStack: ResolutionStack = [] + const resolutionStack: ResolutionStack = parentResolutionStack ?? [] // Internal registration store for this container. const registrations: RegistrationHash = {} @@ -398,7 +402,11 @@ export function createContainer( * The scoped container. */ function createScope

(): AwilixContainer

{ - return createContainer(options, container as AwilixContainer) + return createContainerInternal( + options, + container as AwilixContainer, + resolutionStack, + ) } /** @@ -409,7 +417,7 @@ export function createContainer( const keys = [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)] for (const key of keys) { - const resolver = obj[key as any] as ResolverInternal + const resolver = obj[key as any] as Resolver // If strict mode is enabled, check to ensure we are not registering a singleton on a non-root // container. if (options?.strict && resolver.lifetime === Lifetime.SINGLETON) { @@ -421,16 +429,6 @@ export function createContainer( } } - // If this is a value resolver, set the lifetime to `Lifetime.SCOPED` if this is a scoped - // container, or `Lifetime.SINGLETON` if this is the root container. - if (resolver.isValue) { - if (parentContainer != null) { - resolver.lifetime = Lifetime.SCOPED - } else { - resolver.lifetime = Lifetime.SINGLETON - } - } - registrations[key as any] = resolver } @@ -530,8 +528,8 @@ export function createContainer( const lifetime = resolver.lifetime || Lifetime.TRANSIENT - // if any of the parents have a shorter lifetime than the one requested, - // and the container is configured to do so, throw an error. + // if we are running in strict mode, this resolver is not explicitly marked leak-safe, and any + // of the parents have a shorter lifetime than the one requested, throw an error. if (options?.strict && !resolver.isLeakSafe) { const maybeLongerLifetimeParentIndex = resolutionStack.findIndex( ({ lifetime: parentLifetime }) => @@ -563,7 +561,11 @@ export function createContainer( // Singleton lifetime means cache at all times, regardless of scope. cached = rootContainer.cache.get(name) if (!cached) { - resolved = resolver.resolve(container) + // if we are running in strict mode, perform singleton resolution using the root + // container only. + resolved = resolver.resolve( + options.strict ? rootContainer : container, + ) rootContainer.cache.set(name, { resolver, value: resolved }) } else { resolved = cached.value @@ -597,8 +599,9 @@ export function createContainer( resolutionStack.pop() return resolved } catch (err) { - // When we get an error we need to reset the stack. - resolutionStack = [] + // When we get an error we need to reset the stack. Mutate the existing array rather than + // updating the reference to ensure all parent containers' stacks are also updated. + resolutionStack.length = 0 throw err } } diff --git a/src/resolvers.ts b/src/resolvers.ts index ea0d1f9..741d381 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -3,7 +3,6 @@ import { AwilixTypeError } from './errors' import { InjectionMode, InjectionModeType } from './injection-mode' import { Lifetime, LifetimeType } from './lifetime' import { Parameter, parseParameterList } from './param-parser' -import { ResolverInternal } from './types' import { isFunction, uniq } from './utils' // We parse the signature of any `Function`, so we want to allow `Function` types. @@ -87,6 +86,11 @@ export interface ResolverOptions { * Registration function to use. Only used for inline configuration with `loadModules`. */ register?: (...args: any[]) => Resolver + /** + * True if this resolver should be excluded from lifetime leak checking. Used by resolvers that + * wish to uphold the anti-leakage contract themselves. Defaults to false. + */ + isLeakSafe?: boolean } /** @@ -116,9 +120,9 @@ export interface BuildResolverOptions export type Constructor = { new (...args: any[]): T } /** - * Creates a simple value resolver where the given value will always be resolved. The lifetime of - * the resolved value defaults to `Lifetime.SCOPED` if registered on a scoped container, and - * `Lifetime.SINGLETON` if registered on the root container. + * Creates a simple value resolver where the given value will always be resolved. The value is + * marked as leak-safe since in strict mode, the value will only be resolved when it is not leaking + * upwards from a child scope to a parent singleton. * * @param {string} name The name to register the value as. * @@ -126,10 +130,10 @@ export type Constructor = { new (...args: any[]): T } * * @return {object} The resolver. */ -export function asValue(value: T): ResolverInternal { +export function asValue(value: T): Resolver { return { resolve: () => value, - isValue: true, + isLeakSafe: true, } } @@ -216,11 +220,12 @@ export function asClass( } /** - * Resolves to the specified registration. + * Resolves to the specified registration. Marked as leak-safe since the alias target is what should + * be checked for lifetime leaks. */ export function aliasTo( name: Parameters[0], -): ResolverInternal { +): Resolver { return { resolve(container) { return container.resolve(name) diff --git a/src/types.ts b/src/types.ts index 84f4ccd..4a55876 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,20 +1,6 @@ import { LifetimeType } from './lifetime' -import { Resolver } from './resolvers' -export interface ResolverInternal extends Resolver { - /** - * True if this resolver is a value resolver. Used to implicit set the lifetime of a value - * resolver to either {@link Lifetime.SCOPED} or {@link Lifetime.SINGLETON} depending on whether - * this value is registered to a root or a scope container. - */ - isValue?: boolean - - /** - * True if this resolver should be excluded from lifetime leak checking. Used to exclude alias - * resolvers since any lifetime issues will be caught in the resolution of the alias target. - */ - isLeakSafe?: boolean -} - -export interface ResolutionStack - extends Array<{ name: string | symbol; lifetime: LifetimeType }> {} +export type ResolutionStack = Array<{ + name: string | symbol + lifetime: LifetimeType +}> From f0bc0583965686be310ff35a3e2b0397bf1db7c0 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 11:18:53 -0500 Subject: [PATCH 13/17] Add explicit test for parent/scope resolution stack sharing --- src/__tests__/container.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index 0600afb..f084a5b 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -729,6 +729,25 @@ describe('container', () => { expect(scope.resolve('singleton')).toBe('hah') }) + + it('preserves the resolution stack when resolving a singleton using a parent container, from a scope', () => { + const container = createContainer({ + strict: true, + }) + container.register({ + scoped: asFunction((cradle: any) => cradle.singleton, { + lifetime: Lifetime.SCOPED, + }), + singleton: asFunction((cradle: any) => cradle.val, { + lifetime: Lifetime.SINGLETON, + }), + }) + const scope = container.createScope() + + const err = throws(() => scope.resolve('scoped')) + expect(err).toBeInstanceOf(AwilixResolutionError) + expect(err.message).toContain('scoped -> singleton -> val') + }) }) describe('singleton registration on scope check', () => { From d1af9b4dc3fadad04809da3d7837cec2d7ba7863 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 12:34:35 -0500 Subject: [PATCH 14/17] Add explicit test for injector + definition in strict mode --- src/__tests__/container.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/__tests__/container.test.ts b/src/__tests__/container.test.ts index f084a5b..c9f7c54 100644 --- a/src/__tests__/container.test.ts +++ b/src/__tests__/container.test.ts @@ -631,6 +631,7 @@ describe('container', () => { first: asFunction((cradle: any) => cradle.injected, { lifetime: Lifetime.SCOPED, }).inject(() => ({ injected: 'hah' })), + injected: asFunction(() => 'foobar'), }) expect(container.resolve('first')).toBe('hah') From bd38a1b1d783f5578df8003415ebd5c201d1f6ac Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 12:48:14 -0500 Subject: [PATCH 15/17] Clean up and limit public exports --- README.md | 6 ++++++ src/awilix.ts | 30 ++++++++++++++++++++++++------ src/container.ts | 13 ++++++++----- src/errors.ts | 2 +- src/load-modules.ts | 10 +++++----- src/types.ts | 6 ------ 6 files changed, 44 insertions(+), 23 deletions(-) delete mode 100644 src/types.ts diff --git a/README.md b/README.md index 091ac1a..c40b95a 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,7 @@ Awilix enables you to write **composable, testable software** using dependency i - [`aliasTo()`](#aliasto) - [`listModules()`](#listmodules) - [`AwilixResolutionError`](#awilixresolutionerror) + - [`AwilixRegistrationError`](#awilixregistrationerror) - [The `AwilixContainer` object](#the-awilixcontainer-object) - [`container.cradle`](#containercradle) - [`container.registrations`](#containerregistrations) @@ -955,6 +956,11 @@ This is a special error thrown when Awilix is unable to resolve all dependencies `err instanceof AwilixResolutionError` if you wish. It will tell you what dependencies it could not find or which ones caused a cycle. +## `AwilixRegistrationError` + +This is a special error thrown when Awilix is unable to register a dependency due to a strict mode +violation. You can catch this error and use `err instanceof AwilixRegistrationError` if you wish. + ## The `AwilixContainer` object The container returned from `createContainer` has some methods and properties. diff --git a/src/awilix.ts b/src/awilix.ts index 563e53d..c81f6c1 100644 --- a/src/awilix.ts +++ b/src/awilix.ts @@ -1,6 +1,24 @@ -export * from './errors' -export * from './list-modules' -export * from './container' -export * from './resolvers' -export * from './injection-mode' -export * from './lifetime' +export { AwilixContainer, ContainerOptions, createContainer } from './container' +export { + AwilixError, + AwilixRegistrationError, + AwilixResolutionError, + AwilixTypeError, +} from './errors' +export { InjectionMode, InjectionModeType } from './injection-mode' +export { Lifetime, LifetimeType } from './lifetime' +export { + GlobWithOptions, + ListModulesOptions, + ModuleDescriptor, + listModules, +} from './list-modules' +export { + BuildResolverOptions, + Disposer, + InjectorFunction, + aliasTo, + asClass, + asFunction, + asValue, +} from './resolvers' diff --git a/src/container.ts b/src/container.ts index 6ac5c5a..2fe6bbc 100644 --- a/src/container.ts +++ b/src/container.ts @@ -5,7 +5,7 @@ import { AwilixTypeError, } from './errors' import { InjectionMode, InjectionModeType } from './injection-mode' -import { Lifetime, isLifetimeLonger } from './lifetime' +import { Lifetime, LifetimeType, isLifetimeLonger } from './lifetime' import { GlobWithOptions, listModules } from './list-modules' import { importModule } from './load-module-native.js' import { @@ -21,7 +21,6 @@ import { asClass, asFunction, } from './resolvers' -import { ResolutionStack } from './types' import { isClass, last, nameValueToObject } from './utils' /** @@ -200,6 +199,11 @@ export interface ContainerOptions { */ export type RegistrationHash = Record> +export type ResolutionStack = Array<{ + name: string | symbol + lifetime: LifetimeType +}> + /** * Family tree symbol. */ @@ -228,11 +232,10 @@ const CRADLE_STRING_TAG = 'AwilixContainerCradle' * * @return {AwilixContainer} The container. */ -export function createContainer( +export function createContainer( options: ContainerOptions = {}, - parentContainer?: AwilixContainer, ): AwilixContainer { - return createContainerInternal(options, parentContainer) + return createContainerInternal(options) } function createContainerInternal< diff --git a/src/errors.ts b/src/errors.ts index 1847fc8..282f93c 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,4 +1,4 @@ -import { ResolutionStack } from './types' +import { ResolutionStack } from './container' /** * Newline. diff --git a/src/load-modules.ts b/src/load-modules.ts index f4c7d1a..d43016b 100644 --- a/src/load-modules.ts +++ b/src/load-modules.ts @@ -1,16 +1,16 @@ +import { camelCase } from 'camel-case' import { pathToFileURL } from 'url' -import { ModuleDescriptor, GlobWithOptions, listModules } from './list-modules' +import { AwilixContainer } from './container' import { Lifetime } from './lifetime' +import { GlobWithOptions, ModuleDescriptor, listModules } from './list-modules' import { + BuildResolver, + BuildResolverOptions, RESOLVER, asClass, asFunction, - BuildResolverOptions, } from './resolvers' -import { AwilixContainer } from './container' import { isClass, isFunction } from './utils' -import { BuildResolver } from './awilix' -import { camelCase } from 'camel-case' /** * Metadata of the module as well as the loaded module itself. diff --git a/src/types.ts b/src/types.ts deleted file mode 100644 index 4a55876..0000000 --- a/src/types.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { LifetimeType } from './lifetime' - -export type ResolutionStack = Array<{ - name: string | symbol - lifetime: LifetimeType -}> From 5fd24895e1b6cf6fd5febdc09a6e9b7ecb71d116 Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 13:22:18 -0500 Subject: [PATCH 16/17] Update README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c40b95a..a5c93b2 100644 --- a/README.md +++ b/README.md @@ -335,15 +335,15 @@ the following error at runtime when the singleton `printTime` is resolved: In addition, registering a singleton on a scope other than the root container results in unpredictable behavior. In particular, if two different singletons are registered on two different scopes, they will share a cache entry and collide with each other. To throw a runtime error when a -singleton is registered on a scope other than the root container, set `strict` to true. +singleton is registered on a scope other than the root container, enable [strict mode](#strict-mode). Read the documentation for [`container.createScope()`](#containercreatescope) for more examples. # Strict mode -Strict mode is a new feature in Awilix 9.1. It enables additional correctness -checks that can help you catch bugs early. +Strict mode is a new feature in Awilix 10. It enables additional correctness checks that can help +you catch bugs early. In particular, strict mode enables the following checks: From 6d1832b7ed1ddfaedea7d7cb11a551d58095491c Mon Sep 17 00:00:00 2001 From: Francis Nimick Date: Wed, 3 Jan 2024 17:45:31 -0500 Subject: [PATCH 17/17] Add changelog entry and bump package to v10.0.0 --- CHANGELOG.md | 7 +++++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9014be9..a23543c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# v10.0.0 + +* Add (optional, off by default) strict mode to enforce extra correctness checks in both resolution + and registration +* Reduce the publicly accessible API surface to only that which is needed to use Awilix. This is + potentially a breaking change if you were using any of the internal type definitions. + # v9.0.0 * Upgrade packages diff --git a/package-lock.json b/package-lock.json index 83d7461..8ea85cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "awilix", - "version": "9.0.0", + "version": "10.0.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "awilix", - "version": "9.0.0", + "version": "10.0.0", "license": "MIT", "dependencies": { "camel-case": "^4.1.2", diff --git a/package.json b/package.json index e43c28d..5bd4cc7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "awilix", - "version": "9.0.0", + "version": "10.0.0", "description": "Extremely powerful dependency injection container.", "main": "lib/awilix.js", "module": "lib/awilix.module.mjs",