Skip to content
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

feat: support variadic middleware for routes #85

Merged
merged 1 commit into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/koa-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ export type EndpointImplementation<
context: ContextOfEndpoint<RouteName, Request, RouterType>,
) => Response | Promise<Response>;

export type OneSchemaRouterMiddleware<Context> = (
context: Context,
next: () => Promise<any>,
) => any | Promise<any>;

export const implementRoute = <
Route extends string,
Request,
Expand All @@ -103,6 +108,9 @@ export const implementRoute = <
route: Route,
router: R,
parse: (ctx: ContextOfEndpoint<Route, Request, R>, data: unknown) => Request,
middlewares: OneSchemaRouterMiddleware<
ContextOfEndpoint<Route, Request, R>
>[],
implementation: EndpointImplementation<Route, Request, Response, R>,
) => {
// Separate method and path. e.g. 'POST my/route' => ['POST', 'my/route']
Expand Down Expand Up @@ -163,19 +171,19 @@ export const implementRoute = <
// Register the route + handler on the router.
switch (method) {
case 'POST':
router.post(path, handler);
router.post(path, ...middlewares, handler);
break;
case 'GET':
router.get(path, handler);
router.get(path, ...middlewares, handler);
break;
case 'PUT':
router.put(path, handler);
router.put(path, ...middlewares, handler);
break;
case 'PATCH':
router.patch(path, handler);
router.patch(path, ...middlewares, handler);
break;
case 'DELETE':
router.delete(path, handler);
router.delete(path, ...middlewares, handler);
break;
default:
throw new Error(`Unsupported method detected: ${route}`);
Expand Down
1 change: 1 addition & 0 deletions src/koa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export const implementSchema = <
data,
});
},
[],
routeHandler,
);
}
Expand Down
106 changes: 103 additions & 3 deletions src/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,11 @@ describe('output validation', () => {
request: z.object({}),
response: z.object({ message: z.string() }),
})
.implement(`${method} /items`, () => ({
.implement(
`${method} /items`,
// @ts-expect-error Intentionally writing incorrect TS here
message: 123,
})),
() => ({ message: 123 }),
),
);

const { status } = await client.request({
Expand Down Expand Up @@ -509,6 +510,105 @@ describe('implementations', () => {
});
});

describe('using middleware', () => {
test('type errors are caught when using middleware', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is actually making the jest assertions here? Or are we just relying on the @ts-expect-error directives to act like a unit test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just the ts-expect-error directives. This pattern is used in many tests in this project.

type CustomState = { message: string };

setup(() =>
OneSchemaRouter.create({
using: new Router<CustomState>(),
introspection: undefined,
})
.declare({
name: 'putItem',
route: 'PUT /items/:id',
request: z.object({ message: z.string() }),
response: z.object({ id: z.string(), message: z.string() }),
})
.implement(
'PUT /items/:id',
// @ts-expect-error
async (ctx, next) => {
ctx.state.message = ctx.request.body.message + '-response';
await next();
},
// We're leaving out the id value here, which should cause the TS error.
(ctx) => ({ message: ctx.state.message }),
)
.implement(
'PUT /items/:id',
async (ctx, next) => {
ctx.params.id;

ctx.request.body.message;

// @ts-expect-error The params should be well-typed.
ctx.params.bogus;

// @ts-expect-error The body should be well-typed.
ctx.request.body.bogus;

await next();
},
(ctx) => ({ id: 'test-id', message: ctx.state.message }),
)
.implement(
'PUT /items/:id',
(ctx, next) => {
// This call implicitly tests that `message` is a string.
ctx.state.message.endsWith('');

// @ts-expect-error The state should be well-typed.
ctx.state.bogus;

return next();
},
(ctx) => ({ id: 'test-id', message: ctx.state.message }),
),
);
});

test('middlewares are actually executed', async () => {
const mock = jest.fn();
const { typed: client } = setup((router) =>
router
.declare({
name: 'putItem',
route: 'PUT /items/:id',
request: z.object({ message: z.string() }),
response: z.object({ id: z.string(), message: z.string() }),
})
.implement(
'PUT /items/:id',
async (ctx, next) => {
mock('middleware 1', ctx.state.message);
ctx.state.message = 'message 1';
await next();
},
(ctx, next) => {
mock('middleware 2', ctx.state.message);
ctx.state.message = 'message 2';
return next();
},
(ctx) => ({ id: ctx.params.id, message: ctx.state.message }),
),
);

const { status, data } = await client.putItem({
id: 'test-id-bleh',
message: 'test-message',
});

expect(status).toStrictEqual(200);
expect(data).toStrictEqual({ id: 'test-id-bleh', message: 'message 2' });

expect(mock.mock.calls).toEqual([
['middleware 1', undefined],
['middleware 2', 'message 1'],
]);
});
});

describe('introspection', () => {
test('introspecting + generating a client', async () => {
const { client } = serve(
Expand Down
27 changes: 20 additions & 7 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { fromZodError } from 'zod-validation-error';
import zodToJsonSchema from 'zod-to-json-schema';
import compose = require('koa-compose');
import {
ContextOfEndpoint,
EndpointImplementation,
implementRoute,
OneSchemaRouterMiddleware,
PathParamsOf,
} from './koa-utils';
import { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios';
Expand Down Expand Up @@ -97,15 +99,24 @@ export class OneSchemaRouter<

implement<Route extends keyof Schema & string>(
route: Route,
implementation: EndpointImplementation<
Route,
z.output<Schema[Route]['request']>,
z.infer<Schema[Route]['response']>,
R
>,
) {
...middlewares: [
...OneSchemaRouterMiddleware<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, TIL you can spread a type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this is a rest, not a spread.

ContextOfEndpoint<Route, z.output<Schema[Route]['request']>, R>
>[],
EndpointImplementation<
Route,
z.output<Schema[Route]['request']>,
z.infer<Schema[Route]['response']>,
R
>,
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this above type for middlewares will require 0 to many OneSchemaRouterMiddleware parameters, followed by exactly 1 required EndpointImplementation parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epeters3 Exactly! Previously, I thought this was impossible to model using TS. I was wrong :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way cool. I think the fact that Typescript is fundamentally a correctness checker and DX tool and not just a necessary part of code compilation like with Java or C, has enabled the Typescript maintainers and community to do some pretty incredible things.

): this {
const endpoint = this.schema[route];

const mws = middlewares.slice(0, -1) as any[];

const implementation = middlewares.at(-1) as any;
Comment on lines +116 to +118
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for having to cast as any here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

middlewares doesn't narrow enough -- it's typed as roughly Array<Middleware | Implementation>


implementRoute(
route,
this.router,
Expand All @@ -119,6 +130,8 @@ export class OneSchemaRouter<
}
return res.data;
},
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
mws,
async (ctx) => {
const result = await implementation(ctx);
const res = endpoint.response.safeParse(result);
Expand Down