Skip to content

Commit

Permalink
Merge pull request #14175 from micalevisk/fix-issue-14154
Browse files Browse the repository at this point in the history
fix(common,core): align the logic of optional provider
  • Loading branch information
kamilmysliwiec authored Nov 21, 2024
2 parents 45f4612 + aabde1a commit a1732f5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 21 deletions.
17 changes: 10 additions & 7 deletions packages/common/module-utils/utils/get-injection-providers.util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isUndefined } from '../../utils/shared.utils';
import {
InjectionToken,
Provider,
Expand All @@ -6,15 +7,17 @@ import {
} from '../../interfaces';

/**
* check if x is OptionalFactoryDependency, based on prototype presence
* (to avoid classes with a static 'token' field)
* @param x
* @returns x is OptionalFactoryDependency
* @param value
* @returns `true` if value is `OptionalFactoryDependency`
*/
function isOptionalFactoryDependency(
x: InjectionToken | OptionalFactoryDependency,
): x is OptionalFactoryDependency {
return !!((x as any)?.token && !(x as any)?.prototype);
value: InjectionToken | OptionalFactoryDependency,
): value is OptionalFactoryDependency {
return (
!isUndefined((value as OptionalFactoryDependency).token) &&
!isUndefined((value as OptionalFactoryDependency).optional) &&
!(value as any).prototype
);
}

const mapInjectToTokens = (t: InjectionToken | OptionalFactoryDependency) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,30 @@ import { getInjectionProviders } from '../../../module-utils/utils/get-injection
describe('getInjectionProviders', () => {
it('should take only required providers', () => {
class C {
static token = 'a';
static token = 'anything';
}
const p: Provider[] = [
class G {
static token = 'anything';
static optional = true;
}
class H {
static token = 'anything';
static optional = false;
}
const providers: Provider[] = [
{
//0
provide: 'a',
useValue: 'a',
},
{
//1
provide: 'b',
useValue: 'b',
},
C,
C, //2
{
//3
provide: 'd',
useFactory: (c, b) => [c, b],
inject: [
Expand All @@ -27,23 +38,36 @@ describe('getInjectionProviders', () => {
optional: true,
},
'x',
G,
H,
],
},
{
//4
provide: 'e',
useFactory: (d, b) => [d, b],
inject: ['d', 'b'],
},
{
//5
provide: 'f',
useValue: 'f',
},
G, //6
H, //7
];

const expected = [
providers[1],
providers[2],
providers[3],
providers[4],
providers[6],
providers[7],
];
// should not include 'a' and 'f'
const expected = p.slice(1, -1);
const result = getInjectionProviders(p, ['e']);
expect(result).to.have.length(expected.length);

const result = getInjectionProviders(providers, ['e']);

expect(result).to.have.members(expected);
expect(result).not.to.have.members([p[0], p[5]]);
});
});
21 changes: 15 additions & 6 deletions packages/core/injector/injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,20 @@ export class Injector {
wrapper: InstanceWrapper<T>,
): [InjectorDependency[], number[]] {
const optionalDependenciesIds = [];
const isOptionalFactoryDep = (
item: InjectionToken | OptionalFactoryDependency,
): item is OptionalFactoryDependency =>
!isUndefined((item as OptionalFactoryDependency).token) &&
!isUndefined((item as OptionalFactoryDependency).optional);

/**
* Same as the internal utility function `isOptionalFactoryDependency` from `@nestjs/common`.
* We are duplicating it here because that one is not supposed to be exported.
*/
function isOptionalFactoryDependency(
value: InjectionToken | OptionalFactoryDependency,
): value is OptionalFactoryDependency {
return (
!isUndefined((value as OptionalFactoryDependency).token) &&
!isUndefined((value as OptionalFactoryDependency).optional) &&
!(value as any).prototype
);
}

const mapFactoryProviderInjectArray = (
item: InjectionToken | OptionalFactoryDependency,
Expand All @@ -356,7 +365,7 @@ export class Injector {
if (typeof item !== 'object') {
return item;
}
if (isOptionalFactoryDep(item)) {
if (isOptionalFactoryDependency(item)) {
if (item.optional) {
optionalDependenciesIds.push(index);
}
Expand Down

0 comments on commit a1732f5

Please sign in to comment.