From 942e09a74828f4fc6077ef68beee7f408d7f13b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Fri, 8 Nov 2024 10:52:54 +0100 Subject: [PATCH 1/3] chore: add devEngines constraints --- packages/container/libraries/common/package.json | 4 ++++ packages/container/libraries/core/package.json | 4 ++++ .../foundation/libraries/reflect-metadata-utils/package.json | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/packages/container/libraries/common/package.json b/packages/container/libraries/common/package.json index f3827c6d..84e56ad9 100644 --- a/packages/container/libraries/common/package.json +++ b/packages/container/libraries/common/package.json @@ -21,6 +21,10 @@ "ts-node": "10.9.2", "typescript": "5.6.3" }, + "devEngines": { + "node": "^20.18.0", + "pnpm": "^9.12.1" + }, "homepage": "https://inversify.io", "keywords": [ "dependency injection", diff --git a/packages/container/libraries/core/package.json b/packages/container/libraries/core/package.json index b35306e8..ead5e94e 100644 --- a/packages/container/libraries/core/package.json +++ b/packages/container/libraries/core/package.json @@ -36,6 +36,10 @@ "node", "typescript" ], + "devEngines": { + "node": "^20.18.0", + "pnpm": "^9.12.1" + }, "license": "MIT", "main": "lib/cjs/index.js", "module": "lib/esm/index.js", diff --git a/packages/foundation/libraries/reflect-metadata-utils/package.json b/packages/foundation/libraries/reflect-metadata-utils/package.json index 58beb9d1..a60f8b2b 100644 --- a/packages/foundation/libraries/reflect-metadata-utils/package.json +++ b/packages/foundation/libraries/reflect-metadata-utils/package.json @@ -47,6 +47,10 @@ "ts-node": "10.9.2", "typescript": "5.6.3" }, + "devEngines": { + "node": "^20.18.0", + "pnpm": "^9.12.1" + }, "homepage": "https://inversify.io", "peerDependencies": { "reflect-metadata": "0.2.2" From bb39031d4a5cf66c5a6642f4ad468a68deffc491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Fri, 8 Nov 2024 10:59:24 +0100 Subject: [PATCH 2/3] refactor(core): add assertConstructorMetadataArrayFilled --- ...sertConstructorMetadataArrayFilled.spec.ts | 73 +++++++++++++++++++ .../assertConstructorMetadataArrayFilled.ts | 32 ++++++++ 2 files changed, 105 insertions(+) create mode 100644 packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.spec.ts create mode 100644 packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.ts diff --git a/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.spec.ts b/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.spec.ts new file mode 100644 index 00000000..11579adc --- /dev/null +++ b/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.spec.ts @@ -0,0 +1,73 @@ +import { beforeAll, describe, expect, it } from '@jest/globals'; + +import { InversifyCoreError } from '../../error/models/InversifyCoreError'; +import { InversifyCoreErrorKind } from '../../error/models/InversifyCoreErrorKind'; +import { ClassElementMetadata } from '../models/ClassElementMetadata'; +import { assertConstructorMetadataArrayFilled } from './assertConstructorMetadataArrayFilled'; + +describe(assertConstructorMetadataArrayFilled.name, () => { + describe('having an array with no empty values', () => { + class Foo {} + + let arrayFixture: [ClassElementMetadata]; + + beforeAll(() => { + arrayFixture = [Symbol() as unknown as ClassElementMetadata]; + }); + + describe('when called', () => { + let result: unknown; + + beforeAll(() => { + try { + assertConstructorMetadataArrayFilled(Foo, arrayFixture); + } catch (error: unknown) { + result = error; + } + }); + + it('should not throw an error', () => { + expect(result).toBeUndefined(); + }); + }); + }); + + describe('having an array with empty values', () => { + class Foo {} + + let arrayFixture: (ClassElementMetadata | undefined)[]; + + beforeAll(() => { + arrayFixture = new Array(3); + + arrayFixture[1] = Symbol() as unknown as ClassElementMetadata; + }); + + describe('when called', () => { + let result: unknown; + + beforeAll(() => { + try { + assertConstructorMetadataArrayFilled(Foo, arrayFixture); + } catch (error: unknown) { + result = error; + } + }); + + it('should throw an error', () => { + const expectedErrorProperties: Partial = { + kind: InversifyCoreErrorKind.missingInjectionDecorator, + message: `Found unexpected missing metadata on type "Foo" at constructor indexes "0", "2". + +Are you using @inject, @multiInject or @unmanaged decorators at those indexes? + +If you're using typescript and want to rely on auto injection, set "emitDecoratorMetadata" compiler option to true`, + }; + + expect(result).toStrictEqual( + expect.objectContaining(expectedErrorProperties), + ); + }); + }); + }); +}); diff --git a/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.ts b/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.ts new file mode 100644 index 00000000..f6c3916b --- /dev/null +++ b/packages/container/libraries/core/src/metadata/calculations/assertConstructorMetadataArrayFilled.ts @@ -0,0 +1,32 @@ +import { Newable } from '@inversifyjs/common'; + +import { InversifyCoreError } from '../../error/models/InversifyCoreError'; +import { InversifyCoreErrorKind } from '../../error/models/InversifyCoreErrorKind'; +import { ClassElementMetadata } from '../models/ClassElementMetadata'; + +export function assertConstructorMetadataArrayFilled( + type: Newable, + value: (ClassElementMetadata | undefined)[], +): asserts value is ClassElementMetadata[] { + const undefinedIndexes: number[] = []; + + // Using a for loop to ensure empty values are traversed as well + for (let i: number = 0; i < value.length; ++i) { + const element: ClassElementMetadata | undefined = value[i]; + + if (element === undefined) { + undefinedIndexes.push(i); + } + } + + if (undefinedIndexes.length > 0) { + throw new InversifyCoreError( + InversifyCoreErrorKind.missingInjectionDecorator, + `Found unexpected missing metadata on type "${type.name}" at constructor indexes "${undefinedIndexes.join('", "')}". + +Are you using @inject, @multiInject or @unmanaged decorators at those indexes? + +If you're using typescript and want to rely on auto injection, set "emitDecoratorMetadata" compiler option to true`, + ); + } +} From 0e347ab1cdea4f1fd07ff525589bb4967ff40b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Fri, 8 Nov 2024 11:06:25 +0100 Subject: [PATCH 3/3] feat(core): update getClassMetadataConstructorArguments with better error descriptions --- .changeset/fresh-bulldogs-tan.md | 5 ++++ ...tClassMetadataConstructorArguments.spec.ts | 26 +++++++++++++++++++ .../getClassMetadataConstructorArguments.ts | 5 +++- ...tructorArgumentsFromMetadataReader.spec.ts | 26 +++++++++++++++++++ ...aConstructorArgumentsFromMetadataReader.ts | 5 +++- 5 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 .changeset/fresh-bulldogs-tan.md diff --git a/.changeset/fresh-bulldogs-tan.md b/.changeset/fresh-bulldogs-tan.md new file mode 100644 index 00000000..4a888290 --- /dev/null +++ b/.changeset/fresh-bulldogs-tan.md @@ -0,0 +1,5 @@ +--- +"@inversifyjs/core": patch +--- + +Updated get metadata flow to provide better error messages when missing metadata. diff --git a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.spec.ts b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.spec.ts index c7647020..3ee6686f 100644 --- a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.spec.ts +++ b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.spec.ts @@ -5,6 +5,7 @@ jest.mock('@inversifyjs/reflect-metadata-utils'); import { Newable } from '@inversifyjs/common'; import { getReflectMetadata } from '@inversifyjs/reflect-metadata-utils'; +jest.mock('./assertConstructorMetadataArrayFilled'); jest.mock('./getClassElementMetadataFromNewable'); jest.mock('./getConstructorArgumentMetadataFromLegacyMetadata'); @@ -13,6 +14,7 @@ import { ClassElementMetadata } from '../models/ClassElementMetadata'; import { ClassElementMetadataKind } from '../models/ClassElementMetadataKind'; import { LegacyMetadata } from '../models/LegacyMetadata'; import { LegacyMetadataMap } from '../models/LegacyMetadataMap'; +import { assertConstructorMetadataArrayFilled } from './assertConstructorMetadataArrayFilled'; import { getClassElementMetadataFromNewable } from './getClassElementMetadataFromNewable'; import { getClassMetadataConstructorArguments } from './getClassMetadataConstructorArguments'; import { getConstructorArgumentMetadataFromLegacyMetadata } from './getConstructorArgumentMetadataFromLegacyMetadata'; @@ -73,6 +75,14 @@ describe(getClassMetadataConstructorArguments.name, () => { ); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); @@ -147,6 +157,14 @@ describe(getClassMetadataConstructorArguments.name, () => { ).toHaveBeenCalledWith(typeFixture, 0, legacyMetadataListFixture); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); @@ -229,6 +247,14 @@ describe(getClassMetadataConstructorArguments.name, () => { expect(getClassElementMetadataFromNewable).not.toHaveBeenCalled(); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); diff --git a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.ts b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.ts index dba2678f..fabb9864 100644 --- a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.ts +++ b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArguments.ts @@ -4,6 +4,7 @@ import { getReflectMetadata } from '@inversifyjs/reflect-metadata-utils'; import { DESIGN_PARAM_TYPES, TAGGED } from '../../reflectMetadata/data/keys'; import { ClassElementMetadata } from '../models/ClassElementMetadata'; import { LegacyMetadataMap } from '../models/LegacyMetadataMap'; +import { assertConstructorMetadataArrayFilled } from './assertConstructorMetadataArrayFilled'; import { getClassElementMetadataFromNewable } from './getClassElementMetadataFromNewable'; import { getConstructorArgumentMetadataFromLegacyMetadata } from './getConstructorArgumentMetadataFromLegacyMetadata'; @@ -18,7 +19,7 @@ export function getClassMetadataConstructorArguments( const constructorParametersLegacyMetadata: LegacyMetadataMap | undefined = getReflectMetadata(type, TAGGED); - const constructorArgumentsMetadata: ClassElementMetadata[] = []; + const constructorArgumentsMetadata: (ClassElementMetadata | undefined)[] = []; if (constructorParametersLegacyMetadata !== undefined) { for (const [stringifiedIndex, metadataList] of Object.entries( @@ -48,5 +49,7 @@ export function getClassMetadataConstructorArguments( } } + assertConstructorMetadataArrayFilled(type, constructorArgumentsMetadata); + return constructorArgumentsMetadata; } diff --git a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.spec.ts b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.spec.ts index 36908960..5910e5c2 100644 --- a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.spec.ts +++ b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.spec.ts @@ -2,6 +2,7 @@ import { afterAll, beforeAll, describe, expect, it, jest } from '@jest/globals'; import { Newable } from '@inversifyjs/common'; +jest.mock('./assertConstructorMetadataArrayFilled'); jest.mock('./getClassElementMetadataFromNewable'); jest.mock('./getConstructorArgumentMetadataFromLegacyMetadata'); @@ -10,6 +11,7 @@ import { ClassElementMetadataKind } from '../models/ClassElementMetadataKind'; import { LegacyMetadata } from '../models/LegacyMetadata'; import { LegacyMetadataMap } from '../models/LegacyMetadataMap'; import { LegacyMetadataReader } from '../models/LegacyMetadataReader'; +import { assertConstructorMetadataArrayFilled } from './assertConstructorMetadataArrayFilled'; import { getClassElementMetadataFromNewable } from './getClassElementMetadataFromNewable'; import { getClassMetadataConstructorArgumentsFromMetadataReader } from './getClassMetadataConstructorArgumentsFromMetadataReader'; import { getConstructorArgumentMetadataFromLegacyMetadata } from './getConstructorArgumentMetadataFromLegacyMetadata'; @@ -75,6 +77,14 @@ describe(getClassMetadataConstructorArgumentsFromMetadataReader.name, () => { ); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); @@ -153,6 +163,14 @@ describe(getClassMetadataConstructorArgumentsFromMetadataReader.name, () => { ).toHaveBeenCalledWith(typeFixture, 0, legacyMetadataListFixture); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); @@ -239,6 +257,14 @@ describe(getClassMetadataConstructorArgumentsFromMetadataReader.name, () => { expect(getClassElementMetadataFromNewable).not.toHaveBeenCalled(); }); + it('should call assertConstructorMetadataArrayFilled()', () => { + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledTimes(1); + expect(assertConstructorMetadataArrayFilled).toHaveBeenCalledWith( + typeFixture, + [classElementMetadataFixture], + ); + }); + it('should return ClassElementMetadata[]', () => { expect(result).toStrictEqual([classElementMetadataFixture]); }); diff --git a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.ts b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.ts index 243069d8..dc4034b3 100644 --- a/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.ts +++ b/packages/container/libraries/core/src/metadata/calculations/getClassMetadataConstructorArgumentsFromMetadataReader.ts @@ -3,6 +3,7 @@ import { Newable } from '@inversifyjs/common'; import { ClassElementMetadata } from '../models/ClassElementMetadata'; import { LegacyConstructorMetadata } from '../models/LegacyConstructorMetadata'; import { LegacyMetadataReader } from '../models/LegacyMetadataReader'; +import { assertConstructorMetadataArrayFilled } from './assertConstructorMetadataArrayFilled'; import { getClassElementMetadataFromNewable } from './getClassElementMetadataFromNewable'; import { getConstructorArgumentMetadataFromLegacyMetadata } from './getConstructorArgumentMetadataFromLegacyMetadata'; @@ -13,7 +14,7 @@ export function getClassMetadataConstructorArgumentsFromMetadataReader( const legacyConstructorMetadata: LegacyConstructorMetadata = metadataReader.getConstructorMetadata(type); - const constructorArgumentsMetadata: ClassElementMetadata[] = []; + const constructorArgumentsMetadata: (ClassElementMetadata | undefined)[] = []; for (const [stringifiedIndex, metadataList] of Object.entries( legacyConstructorMetadata.userGeneratedMetadata, @@ -44,5 +45,7 @@ export function getClassMetadataConstructorArgumentsFromMetadataReader( } } + assertConstructorMetadataArrayFilled(type, constructorArgumentsMetadata); + return constructorArgumentsMetadata; }