Skip to content

Commit

Permalink
Fix null property access at getBaseClassDependencyCount (#1665)
Browse files Browse the repository at this point in the history
* refactor: add getBaseType

* fix: update getBaseClassDependencyCount to rely on getBaseType

* fix: update rollup config to patch wrong sourcemap links on ESM build

* fix: update Prototype with mandatory constructor
  • Loading branch information
notaphplover authored Dec 3, 2024
1 parent f7dd2c8 commit d787b8f
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

### Fixed
- Fixed unexpected property access while running planning checks on injected base types.

## [6.1.5]

Expand Down
8 changes: 8 additions & 0 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ export default [
dir: './lib/esm',
format: 'esm',
sourcemap: true,
sourcemapPathTransform: (relativeSourcePath) => {
// Rollup seems to generate source maps pointing to the wrong directory. Ugly patch to fix it
if (relativeSourcePath.startsWith('../')) {
return relativeSourcePath.slice(3);
} else {
return relativeSourcePath;
}
},
},
],
plugins: [
Expand Down
62 changes: 30 additions & 32 deletions src/planning/reflection_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getTargets } from '@inversifyjs/core';

import * as METADATA_KEY from '../constants/metadata_keys';
import { interfaces } from '../interfaces/interfaces';
import { getBaseType } from '../utils/get_base_type';
import { getFunctionName } from '../utils/serialization';
import { Metadata } from './metadata';

Expand All @@ -17,40 +18,37 @@ function getBaseClassDependencyCount(
metadataReader: interfaces.MetadataReader,
func: NewableFunction,
): number {
const baseConstructor: NewableFunction =
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Object.getPrototypeOf(func.prototype).constructor as NewableFunction;
if (baseConstructor !== (Object as unknown as NewableFunction)) {
// get targets for base class

const targets: interfaces.Target[] = getTargets(metadataReader)(
baseConstructor as Newable,
);

// get unmanaged metadata
const metadata: interfaces.Metadata[][] = targets.map(
(t: interfaces.Target) =>
t.metadata.filter(
(m: interfaces.Metadata) => m.key === METADATA_KEY.UNMANAGED_TAG,
),
);

// Compare the number of constructor arguments with the number of
// unmanaged dependencies unmanaged dependencies are not required
const unmanagedCount: number = ([] as Metadata[]).concat.apply(
[],
metadata,
).length;
const dependencyCount: number = targets.length - unmanagedCount;

if (dependencyCount > 0) {
return dependencyCount;
} else {
return getBaseClassDependencyCount(metadataReader, baseConstructor);
}
} else {
const baseConstructor: Newable | undefined = getBaseType(func);

if (baseConstructor === undefined || baseConstructor === Object) {
return 0;
}

// get targets for base class
const targets: interfaces.Target[] =
getTargets(metadataReader)(baseConstructor);

// get unmanaged metadata
const metadata: interfaces.Metadata[][] = targets.map(
(t: interfaces.Target) =>
t.metadata.filter(
(m: interfaces.Metadata) => m.key === METADATA_KEY.UNMANAGED_TAG,
),
);

// Compare the number of constructor arguments with the number of
// unmanaged dependencies unmanaged dependencies are not required
const unmanagedCount: number = ([] as Metadata[]).concat.apply(
[],
metadata,
).length;
const dependencyCount: number = targets.length - unmanagedCount;

if (dependencyCount > 0) {
return dependencyCount;
} else {
return getBaseClassDependencyCount(metadataReader, baseConstructor);
}
}

export { getDependencies, getBaseClassDependencyCount, getFunctionName };
50 changes: 50 additions & 0 deletions src/test/utils/get_base_type.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { Newable } from '@inversifyjs/common';
import { expect } from 'chai';

import { getBaseType } from '../../utils/get_base_type';

describe(getBaseType.name, () => {
describe('having a type with base type', () => {
let baseTypeFixture: Newable;
let typeFixture: Newable;

before(() => {
class BaseType {}

baseTypeFixture = BaseType;
typeFixture = class extends BaseType {};
});

describe('when called', () => {
let result: unknown;

before(() => {
result = getBaseType(typeFixture);
});

it('should return base type', () => {
expect(result).to.eq(baseTypeFixture);
});
});
});

describe('having a type with no base type', () => {
let typeFixture: Newable;

before(() => {
typeFixture = Object;
});

describe('when called', () => {
let result: unknown;

before(() => {
result = getBaseType(typeFixture);
});

it('should return undefined', () => {
expect(result).to.eq(undefined);
});
});
});
});
16 changes: 16 additions & 0 deletions src/utils/get_base_type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Newable } from '@inversifyjs/common';

interface Prototype {
constructor: Newable;
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
export function getBaseType(type: Function): Newable | undefined {
const prototype: Prototype | null = Object.getPrototypeOf(
type.prototype,
) as Prototype | null;

const baseType: Newable | undefined = prototype?.constructor;

return baseType;
}

0 comments on commit d787b8f

Please sign in to comment.