Skip to content

Commit

Permalink
fix(platform-express): prevent loosing context with express middleware
Browse files Browse the repository at this point in the history
  • Loading branch information
Romakita committed Aug 21, 2022
1 parent 80ee899 commit 4fd0ec3
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 25 deletions.
8 changes: 3 additions & 5 deletions packages/di/src/services/InjectorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class InjectorService extends Container {
switch (this.scopeOf(provider)) {
case ProviderScope.SINGLETON:
if (provider.hooks && !options.rebuild) {
this.registerHooks(provider);
this.registerHooks(provider, instance);
}

if (!provider.isAsync()) {
Expand Down Expand Up @@ -589,8 +589,6 @@ export class InjectorService extends Container {

async destroy() {
await this.emit("$onDestroy");
this.#cache.clear();
this.#hooks.destroy();
}

protected ensureProvider(token: TokenProvider): Provider | undefined {
Expand Down Expand Up @@ -740,10 +738,10 @@ export class InjectorService extends Container {
};
}

private registerHooks(provider: Provider) {
private registerHooks(provider: Provider, instance: any) {
if (provider.hooks) {
Object.entries(provider.hooks).forEach(([event, cb]) => {
const callback = (...args: any[]) => cb(this.get(provider.token), ...args);
const callback = (...args: any[]) => cb(this.get(provider.token) || instance, ...args);

this.#hooks.on(event, callback);
});
Expand Down
6 changes: 6 additions & 0 deletions packages/platform/platform-express/coverage.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"statements": 100,
"branches": 80.64,
"functions": 100,
"lines": 100
}
7 changes: 1 addition & 6 deletions packages/platform/platform-express/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
module.exports = {
...require("@tsed/jest-config")(__dirname, "platform-express"),
coverageThreshold: {
global: {
statements: 100,
branches: 78.94,
functions: 100,
lines: 100
}
global: require("./coverage.json")
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,10 @@ export class PlatformExpress implements PlatformAdapter<Express.Application, Exp

app() {
const app = this.injector.settings.get("express.app") || Express();
const requestHandler = (req: IncomingMessage, res: ServerResponse) => runInContext(undefined, () => app(req, res), this.injector);

return {
app,
callback: () => requestHandler
callback: () => app
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe("PlatformExpressHandler", () => {
const handler = platformHandler.createHandler(handlerMetadata);

// THEN
expect(handler).toEqual(handlerMetadata.handler);
expect(handler).toEqual(expect.any(Function));
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,37 @@
import {HandlerMetadata, HandlerType, OnRequestOptions, PlatformHandler} from "@tsed/common";
import {HandlerMetadata, HandlerType, OnRequestOptions, PlatformHandler, runInContext} from "@tsed/common";

/**
* @platform
* @express
*/
export class PlatformExpressHandler extends PlatformHandler {
protected createRawHandler(metadata: HandlerMetadata): Function {
if ([HandlerType.RAW_ERR_FN, HandlerType.RAW_FN].includes(metadata.type)) {
return metadata.handler;
}

const handler = this.compileHandler(metadata);

if (metadata.type === HandlerType.ERR_MIDDLEWARE) {
const handler = this.compileHandler(metadata);
return async (err: any, req: any, res: any, next: any) =>
runInContext(req.$ctx, () =>
handler({
err,
next,
$ctx: req.$ctx
})
);
}

return async (req: any, res: any, next: any) => {
return runInContext(req.$ctx, () =>
handler({
err,
next,
$ctx: req.$ctx
});
}

return super.createRawHandler(metadata);
})
);
};
}

protected async onCtxRequest(requestOptions: OnRequestOptions): Promise<any> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ describe("ViteServer", () => {
middlewareMode: true
}
});

// @ts-ignore
await viteDevServer.$onDestroy();
});
it("should load and close server", async () => {
const viteDevServer = PlatformTest.get<VITE_SERVER>(VITE_SERVER);
Expand All @@ -50,8 +47,8 @@ describe("ViteServer", () => {
}
});

// @ts-ignore
await viteDevServer.$onDestroy();
await PlatformTest.injector.destroy();

expect(viteDevServer.close).toHaveBeenCalledWith();
});
});
Expand Down

0 comments on commit 4fd0ec3

Please sign in to comment.