Skip to content

Commit

Permalink
fix(injector): resolve deps of exported providers correctly
Browse files Browse the repository at this point in the history 
When a provider is exported its exporting chain needs to be set so injector knows how to resolve dependencies in the right order.

Also add configureProvider to App abstraction.
  • Loading branch information
@marcj
marcj committed Feb 1, 2024
1 parent 6c3c057 commit c185b38
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 45 deletions.
24 changes: 16 additions & 8 deletions packages/app/src/app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import { ClassType, ExtractClassType, isFunction, isObject, pathBasename, setPathValue } from '@deepkit/core';
import { ConfigLoader, ServiceContainer } from './service-container.js';
import { InjectorContext, ResolveToken, Token } from '@deepkit/injector';
import { ConfigureProviderOptions, InjectorContext, ResolveToken, Token } from '@deepkit/injector';
import { AppModule, RootModuleDefinition } from './module.js';
import { EnvConfiguration } from './configuration.js';
import { DataEventToken, EventDispatcher, EventListener, EventListenerCallback, EventOfEventToken, EventToken } from '@deepkit/event';
Expand Down Expand Up @@ -59,7 +59,7 @@ function parseEnv(
incomingDotPath: string,
incomingEnvPath: string,
namingStrategy: EnvNamingStrategy,
envContainer: { [name: string]: any }
envContainer: { [name: string]: any },
) {
for (const property of schema.getProperties()) {
const name = convertNameStrategy(namingStrategy, property.name);
Expand All @@ -72,7 +72,7 @@ function parseEnv(
(incomingDotPath ? incomingDotPath + '.' : '') + property.name,
(incomingEnvPath ? incomingEnvPath + '_' : '') + name,
namingStrategy,
envContainer
envContainer,
);
} else {
const dotPath = (incomingDotPath ? incomingDotPath + '.' : '') + property.name;
Expand Down Expand Up @@ -113,7 +113,7 @@ interface EnvConfigOptions {
const defaultEnvConfigOptions: Required<EnvConfigOptions> = {
prefix: 'APP_',
envFilePath: ['.env'],
namingStrategy: 'upper'
namingStrategy: 'upper',
};

class EnvConfigLoader {
Expand All @@ -124,7 +124,7 @@ class EnvConfigLoader {
constructor(options?: EnvConfigOptions) {
const normalizedOptions = {
...defaultEnvConfigOptions,
...options
...options,
};

const { prefix, envFilePath, namingStrategy } = normalizedOptions;
Expand Down Expand Up @@ -215,7 +215,7 @@ export class App<T extends RootModuleDefinition> {
constructor(
appModuleOptions: T,
serviceContainer?: ServiceContainer,
appModule?: AppModule<any>
appModule?: AppModule<any>,
) {
this.appModule = appModule || new RootAppModule(appModuleOptions) as any;
this.serviceContainer = serviceContainer || new ServiceContainer(this.appModule);
Expand Down Expand Up @@ -320,7 +320,7 @@ export class App<T extends RootModuleDefinition> {
} catch (error) {
throw new Error(`Invalid JSON in env variable ${variableName}. Parse error: ${error}`);
}
}
},
});
return this;
}
Expand All @@ -338,6 +338,14 @@ export class App<T extends RootModuleDefinition> {
return this.serviceContainer.getInjectorContext();
}

/**
* @see InjectorModule.configureProvider
*/
configureProvider<T>(configure: (instance: T, ...args: any[]) => any, options: Partial<ConfigureProviderOptions> = {}, type?: ReceiveType<T>): this {
this.appModule.configureProvider<T>(configure, options, type);
return this;
}

public async execute(argv?: string[], bin?: string[] | string): Promise<number> {
const eventDispatcher = this.get(EventDispatcher);
const logger = this.get(Logger);
Expand All @@ -362,7 +370,7 @@ export class App<T extends RootModuleDefinition> {
bin, argv || getArgsFromEnvironment(),
eventDispatcher, logger,
scopedInjectorContext,
this.serviceContainer.cliControllerRegistry.controllers
this.serviceContainer.cliControllerRegistry.controllers,
);
}
}
39 changes: 34 additions & 5 deletions packages/http/tests/module.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { HttpKernel } from '../src/kernel.js';
import { HttpRequest } from '../src/model.js';
import { http } from '../src/decorator.js';
import { httpWorkflow } from '../src/http.js';
import { HttpRouterRegistry } from '../src/router.js';
import { HttpRouterRegistry, RouteConfig } from '../src/router.js';

test('module basic functionality', async () => {
class Controller {
Expand All @@ -20,8 +20,8 @@ test('module basic functionality', async () => {
Controller,
],
imports: [
new HttpModule()
]
new HttpModule(),
],
});

const httpKernel = app.get(HttpKernel);
Expand Down Expand Up @@ -53,7 +53,7 @@ test('functional listener', async () => {
],
imports: [
new HttpModule(),
]
],
});

const httpKernel = app.get(HttpKernel);
Expand Down Expand Up @@ -82,7 +82,7 @@ test('functional routes using use()', async () => {

const app = new App({
providers: [MyService],
imports: [new HttpModule()]
imports: [new HttpModule()],
});

function userController(router: HttpRouterRegistry, service: MyService) {
Expand All @@ -106,3 +106,32 @@ test('functional routes using use()', async () => {
expect(response.json).toEqual({ id: 2, username: 'marie' });
}
});

test('dynamic route', async () => {
const app = new App({
providers: [],
imports: [new HttpModule()],
});

app.configureProvider<HttpRouterRegistry>(router => {
router.addRoute(new RouteConfig('name', ['GET'], '/users/:id', {
type: 'function', fn: (id: number) => {
return {id};
},
}));

router.get('/users', () => {
return [{id: 1}, {id: 2}];
});
});

const httpKernel = app.get(HttpKernel);

const response = await httpKernel.request(HttpRequest.GET('/users/2'));
expect(response.statusCode).toBe(200);
expect(response.json).toEqual({ id: 2 });

const response2 = await httpKernel.request(HttpRequest.GET('/users'));
expect(response2.statusCode).toBe(200);
expect(response2.json).toEqual([{ id: 1 }, { id: 2 }]);
});
54 changes: 31 additions & 23 deletions packages/injector/src/injector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ import {
Token,
} from './provider.js';
import { AbstractClassType, ClassType, CompilerContext, CustomError, getClassName, getPathValue, isArray, isClass, isFunction, isPrototypeOfBase } from '@deepkit/core';
import { findModuleForConfig, getScope, InjectorModule, PreparedProvider, ConfigurationProviderRegistry } from './module.js';
import { ConfigurationProviderRegistry, ConfigureProviderEntry, findModuleForConfig, getScope, InjectorModule, PreparedProvider } from './module.js';
import {
isExtendable,
isOptional,
isType,
isWithAnnotations,
Expand Down Expand Up @@ -58,7 +57,7 @@ function functionParameterNotFound(ofName: string, name: string, position: numbe
argsCheck.push('?');

throw new DependenciesUnmetError(
`Unknown function argument '${name}: ${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`
`Unknown function argument '${name}: ${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`,
);
}

Expand All @@ -68,13 +67,13 @@ function constructorParameterNotFound(ofName: string, name: string, position: nu
argsCheck.push('?');

throw new DependenciesUnmetError(
`Unknown constructor argument '${name}: ${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`
`Unknown constructor argument '${name}: ${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`,
);
}

function serviceNotfoundError(token: any, moduleName: string) {
throw new ServiceNotFoundError(
`Service '${tokenLabel(token)}' in ${moduleName} not found. Make sure it is provided.`
`Service '${tokenLabel(token)}' in ${moduleName} not found. Make sure it is provided.`,
);
}

Expand All @@ -85,20 +84,20 @@ function factoryDependencyNotFound(ofName: string, name: string, position: numbe

for (const reset of CircularDetectorResets) reset();
throw new DependenciesUnmetError(
`Unknown factory dependency argument '${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`
`Unknown factory dependency argument '${tokenLabel(token)}' of ${ofName}(${argsCheck.join(', ')}). Make sure '${tokenLabel(token)}' is provided.`,
);
}

function propertyParameterNotFound(ofName: string, name: string, position: number, token: any) {
for (const reset of CircularDetectorResets) reset();
throw new DependenciesUnmetError(
`Unknown property parameter ${name} of ${ofName}. Make sure '${tokenLabel(token)}' is provided.`
`Unknown property parameter ${name} of ${ofName}. Make sure '${tokenLabel(token)}' is provided.`,
);
}

function transientInjectionTargetUnavailable(ofName: string, name: string, position: number, token: any) {
throw new DependenciesUnmetError(
`${TransientInjectionTarget.name} is not available for ${name} of ${ofName}. ${TransientInjectionTarget.name} is only available when injecting into other providers`
`${TransientInjectionTarget.name} is not available for ${name} of ${ofName}. ${TransientInjectionTarget.name} is only available when injecting into other providers`,
);
}

Expand Down Expand Up @@ -422,7 +421,7 @@ export class Injector implements InjectorInterface {
args.push(this.createFactoryProperty({
name: parameter.name,
type: tokenType || parameter.getType() as Type,
optional: !parameter.isValueRequired()
optional: !parameter.isValueRequired(),
}, provider, compiler, resolveDependenciesFrom, ofName, args.length, 'factoryDependencyNotFound'));
}

Expand All @@ -433,7 +432,10 @@ export class Injector implements InjectorInterface {

const configureProvider: string[] = [];

const configurations = resolveDependenciesFrom[0].configurationProviderRegistry?.get(token);
const configurations: ConfigureProviderEntry[] = [];
for (const module of resolveDependenciesFrom) {
configurations.push(...module.configurationProviderRegistry.get(token));
}
configurations.push(...buildContext.globalConfigurationProviderRegistry.get(token));

if (configurations?.length) {
Expand All @@ -451,7 +453,7 @@ export class Injector implements InjectorInterface {
args.push(this.createFactoryProperty({
name: parameter.name,
type: tokenType || parameter.getType() as Type,
optional: !parameter.isValueRequired()
optional: !parameter.isValueRequired(),
}, provider, compiler, resolveDependenciesFrom, ofName, args.length, 'functionParameterNotFound'));
}

Expand Down Expand Up @@ -496,7 +498,7 @@ export class Injector implements InjectorInterface {
resolvedName: string,
compiler: CompilerContext,
classType: ClassType,
resolveDependenciesFrom: InjectorModule[]
resolveDependenciesFrom: InjectorModule[],
): { code: string, dependencies: number } {
if (!classType) throw new Error('Can not create factory for undefined ClassType');
const reflectionClass = ReflectionClass.from(classType);
Expand All @@ -514,7 +516,7 @@ export class Injector implements InjectorInterface {
args.push(this.createFactoryProperty({
name: parameter.name,
type: tokenType || parameter.getType() as Type,
optional: !parameter.isValueRequired()
optional: !parameter.isValueRequired(),
}, provider, compiler, resolveDependenciesFrom, getClassName(classType), args.length, 'constructorParameterNotFound'));
}
}
Expand All @@ -528,7 +530,7 @@ export class Injector implements InjectorInterface {
const resolveProperty = this.createFactoryProperty({
name: property.name,
type: tokenType,
optional: !property.isValueRequired()
optional: !property.isValueRequired(),
}, provider, compiler, resolveDependenciesFrom, getClassName(classType), -1, 'propertyParameterNotFound');
propertyAssignment.push(`${resolvedName}.${String(property.getName())} = ${resolveProperty};`);
} catch (error: any) {
Expand All @@ -538,7 +540,7 @@ export class Injector implements InjectorInterface {

return {
code: `${resolvedName} = new ${classTypeVar}(${args.join(',')});\n${propertyAssignment.join('\n')}`,
dependencies
dependencies,
};
}

Expand All @@ -549,7 +551,7 @@ export class Injector implements InjectorInterface {
resolveDependenciesFrom: InjectorModule[],
ofName: string,
argPosition: number,
notFoundFunction: string
notFoundFunction: string,
): string {
let of = `${ofName}.${options.name}`;
const destinationVar = compiler.reserveConst({ token: fromProvider.provide });
Expand All @@ -575,8 +577,12 @@ export class Injector implements InjectorInterface {
return compiler.reserveVariable('tagRegistry', this.buildContext.tagRegistry);
}

if (options.type.kind === ReflectionKind.class && resolveDependenciesFrom[0] instanceof options.type.classType) {
return compiler.reserveConst(resolveDependenciesFrom[0], 'module');
if (options.type.kind === ReflectionKind.class) {
for (const module of resolveDependenciesFrom) {
if (module instanceof options.type.classType) {
return compiler.reserveConst(module, 'module');
}
}
}

if (options.type.kind === ReflectionKind.class && isPrototypeOfBase(options.type.classType, Tag)) {
Expand Down Expand Up @@ -683,7 +689,7 @@ export class Injector implements InjectorInterface {
const type = stringifyType(options.type, { showFullDefinition: false }).replace(/\n/g, '').replace(/\s\s+/g, ' ').replace(' & InjectMeta', '');
if (options.optional) return 'undefined';
throw new DependenciesUnmetError(
`Undefined dependency "${options.name}: ${type}" of ${of}. Type has no provider${fromScope ? ' in scope ' + fromScope : ''}.`
`Undefined dependency "${options.name}: ${type}" of ${of}. Type has no provider${fromScope ? ' in scope ' + fromScope : ''}.`,
);
}

Expand All @@ -696,7 +702,7 @@ export class Injector implements InjectorInterface {
const t = stringifyType(options.type, { showFullDefinition: false });
throw new DependenciesUnmetError(
`Dependency '${options.name}: ${t}' of ${of} can not be injected into ${fromScope ? 'scope ' + fromScope : 'no scope'}, ` +
`since ${foundProviderLabel} only exists in scope${allPossibleScopes.length === 1 ? '' : 's'} ${allPossibleScopes.join(', ')}.`
`since ${foundProviderLabel} only exists in scope${allPossibleScopes.length === 1 ? '' : 's'} ${allPossibleScopes.join(', ')}.`,
);
}

Expand Down Expand Up @@ -730,8 +736,10 @@ export class Injector implements InjectorInterface {

if (type.kind === ReflectionKind.class && type.classType === TagRegistry) return () => this.buildContext.tagRegistry;

if (type.kind === ReflectionKind.class && resolveDependenciesFrom[0] instanceof type.classType) {
return () => resolveDependenciesFrom[0];
if (type.kind === ReflectionKind.class) {
for (const module of resolveDependenciesFrom) {
if (module instanceof type.classType) return () => module;
}
}

if (type.kind === ReflectionKind.class && isPrototypeOfBase(type.classType, Tag)) {
Expand Down Expand Up @@ -827,7 +835,7 @@ export class Injector implements InjectorInterface {
if (optional) return () => undefined;
const t = stringifyType(type, { showFullDefinition: false });
throw new ServiceNotFoundError(
`Undefined service "${label ? label + ': ' : ''}${t}". Type has no matching provider in ${fromScope ? 'scope ' + fromScope : 'no scope'}.`
`Undefined service "${label ? label + ': ' : ''}${t}". Type has no matching provider in ${fromScope ? 'scope ' + fromScope : 'no scope'}.`,
);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/injector/src/module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,9 @@ export class InjectorModule<C extends { [name: string]: any } = any, IMPORT = In
//we add our module as additional source for potential dependencies
registerPreparedProvider(parentProviders, preparedProvider.modules, preparedProvider.providers, false);
} else {
parentProviders.push({ token: preparedProvider.token, modules: [this], providers: preparedProvider.providers.slice() });
// [this, to] is used so that this service resolves dependencies from the target first (so it can overwrite them)
// and falls back to the module it was defined in. This includes configureProvider() calls.
parentProviders.push({ token: preparedProvider.token, modules: [to, this], providers: preparedProvider.providers.slice() });
}
};

Expand Down
Loading

0 comments on commit c185b38

Please sign in to comment.