-
-
Notifications
You must be signed in to change notification settings - Fork 291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(platform-http): update the adapter configuration to be aligned with DI convention #2999
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,39 @@ | ||
import {Type} from "@tsed/core"; | ||
import {refValue} from "@tsed/di"; | ||
import {GlobalProviders, injector, ProviderOpts, refValue} from "@tsed/di"; | ||
|
||
import type {PlatformAdapter} from "../services/PlatformAdapter.js"; | ||
import {PlatformAdapter} from "../services/PlatformAdapter.js"; | ||
|
||
const ADAPTER = "platform.adapter"; | ||
|
||
let globalAdapter: Type<PlatformAdapter<any>>; | ||
|
||
/** | ||
* Set the platform adapter | ||
* Set the platform adapter token and his dependencies | ||
*/ | ||
export function adapter(adapter?: Type<PlatformAdapter<any>>): Type<PlatformAdapter<any>> { | ||
export function adapter(adapter: Type<PlatformAdapter<any>>, imports?: ProviderOpts[]): Type<PlatformAdapter<any>>; | ||
/** | ||
* Get the platform adapter token | ||
*/ | ||
export function adapter(): Type<PlatformAdapter<any>>; | ||
export function adapter(adapter?: Type<PlatformAdapter<any>>, imports: ProviderOpts[] = []): Type<PlatformAdapter<any>> { | ||
Comment on lines
+11
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Clarify function overload documentation |
||
const ref = refValue<Type<PlatformAdapter<any>>>(ADAPTER); | ||
|
||
if (adapter) { | ||
globalAdapter ||= adapter; | ||
|
||
imports?.forEach(({token, useClass}) => { | ||
const provider = GlobalProviders.get(token); | ||
if (useClass && provider) { | ||
provider.useClass = useClass; | ||
injector().set(token, provider); | ||
} | ||
}); | ||
|
||
injector() | ||
.addProvider(PlatformAdapter, { | ||
useClass: adapter | ||
}) | ||
.alias(PlatformAdapter, "PlatformAdapter"); | ||
Comment on lines
+24
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Handle missing providers and possible concurrency side effects |
||
} | ||
|
||
ref.value ||= globalAdapter; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import type {MulterError} from "multer"; | |
|
||
import {PlatformMulterField, PlatformMulterSettings} from "../config/interfaces/PlatformMulterSettings.js"; | ||
import {PlatformContext} from "../domain/PlatformContext.js"; | ||
import {PlatformAdapter} from "../services/PlatformAdapter.js"; | ||
import {PlatformApplication} from "../services/PlatformApplication.js"; | ||
|
||
export interface MulterInputOptions { | ||
|
@@ -24,8 +25,6 @@ export class MulterException extends BadRequest { | |
* @middleware | ||
*/ | ||
export class PlatformMulterMiddleware implements MiddlewareMethods { | ||
protected app = inject(PlatformApplication); | ||
|
||
async use(@Context() ctx: PlatformContext) { | ||
try { | ||
const {fields, options = {}} = ctx.endpoint.get(PlatformMulterMiddleware); | ||
|
@@ -39,7 +38,7 @@ export class PlatformMulterMiddleware implements MiddlewareMethods { | |
delete settings.dest; | ||
} | ||
|
||
const multer = this.app.multer(settings); | ||
const multer = inject(PlatformAdapter).multipart(settings); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Refactor injection usage. |
||
|
||
if (multer) { | ||
const middleware: any = multer.fields(this.getFields({fields})); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import {IncomingMessage, ServerResponse} from "node:http"; | ||
|
||
import {Type} from "@tsed/core"; | ||
import {injectable, ProviderOpts} from "@tsed/di"; | ||
import {configuration, constant, inject, injectable} from "@tsed/di"; | ||
import {$on} from "@tsed/hooks"; | ||
import {PlatformHandlerMetadata, PlatformLayer} from "@tsed/platform-router"; | ||
|
||
import {PlatformMulter, PlatformMulterSettings} from "../config/interfaces/PlatformMulterSettings.js"; | ||
|
@@ -10,17 +11,25 @@ import {application} from "../fn/application.js"; | |
import {createHttpServer} from "../utils/createHttpServer.js"; | ||
import {createHttpsServer} from "../utils/createHttpsServer.js"; | ||
import {CreateServerReturn} from "../utils/createServer.js"; | ||
import type {PlatformApplication} from "./PlatformApplication.js"; | ||
import {Platform} from "./Platform.js"; | ||
import {PlatformApplication} from "./PlatformApplication.js"; | ||
import {PlatformHandler} from "./PlatformHandler.js"; | ||
|
||
export abstract class PlatformAdapter<App = TsED.Application> { | ||
abstract readonly NAME: string; | ||
/** | ||
* Load providers in top priority | ||
*/ | ||
providers: ProviderOpts[] = []; | ||
|
||
get app(): PlatformApplication<App> { | ||
return application(); | ||
readonly app = inject<PlatformApplication<App>>(PlatformApplication); | ||
|
||
constructor() { | ||
const platformApp = inject(PlatformApplication); | ||
|
||
const {app, callback} = this.createApp(); | ||
platformApp.rawApp = app as any; | ||
platformApp.rawCallback = callback; | ||
|
||
$on("$afterInvoke", PlatformAdapter, () => { | ||
configuration().set("PLATFORM_NAME", constant("PLATFORM_NAME") || this.NAME); | ||
}); | ||
Comment on lines
+21
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Unify usage of injected references and confirm |
||
} | ||
|
||
getServers(): CreateServerReturn[] { | ||
|
@@ -87,4 +96,4 @@ export interface PlatformBuilderSettings<App = TsED.Application> extends Partial | |
adapter?: Type<PlatformAdapter<App>>; | ||
} | ||
|
||
injectable(PlatformAdapter).alias("PlatformAdapter"); | ||
injectable(PlatformAdapter).imports([PlatformApplication, Platform, PlatformHandler]).alias("PlatformAdapter"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,8 @@ | ||
import {IncomingMessage, ServerResponse} from "node:http"; | ||
|
||
import {inject, injectable, ProviderScope} from "@tsed/di"; | ||
import {injectable, ProviderScope} from "@tsed/di"; | ||
import {PlatformRouter} from "@tsed/platform-router"; | ||
|
||
import {PlatformMulterSettings} from "../config/interfaces/PlatformMulterSettings.js"; | ||
import {PlatformAdapter} from "./PlatformAdapter.js"; | ||
|
||
declare global { | ||
namespace TsED { | ||
// @ts-ignore | ||
|
@@ -19,28 +16,14 @@ declare global { | |
* @platform | ||
*/ | ||
export class PlatformApplication<App = TsED.Application> extends PlatformRouter { | ||
adapter: PlatformAdapter<App> = inject(PlatformAdapter<App>); | ||
|
||
rawApp: App; | ||
rawCallback: () => any; | ||
|
||
constructor() { | ||
super(); | ||
|
||
const {app, callback} = this.adapter.createApp(); | ||
|
||
this.rawApp = app; | ||
this.rawCallback = callback; | ||
} | ||
rawCallback(): any {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Empty method body may lead to unhandled callbacks. |
||
|
||
getApp(): App { | ||
return this.rawApp; | ||
} | ||
|
||
multer(options: PlatformMulterSettings) { | ||
return this.adapter.multipart(options); | ||
} | ||
|
||
callback(): (req: IncomingMessage, res: ServerResponse) => any; | ||
callback(req: IncomingMessage, res: ServerResponse): any; | ||
callback(req?: IncomingMessage, res?: ServerResponse) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use consistent ESM extension
The import statements with ".js" extension are valid for ESM usage. Just ensure your bundler or environment supports it consistently across the platform.