Skip to content

Commit

Permalink
[APM] Explicit return types for routes (#123266)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgieselaar authored Jan 25, 2022
1 parent 0c6edb4 commit 7f5e6c3
Show file tree
Hide file tree
Showing 144 changed files with 2,304 additions and 1,350 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@
"terser-webpack-plugin": "^4.2.3",
"tough-cookie": "^4.0.0",
"ts-loader": "^7.0.5",
"ts-morph": "^11.0.0",
"ts-morph": "^13.0.2",
"tsd": "^0.13.1",
"typescript": "4.5.3",
"unlazy-loader": "^0.1.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export function buildApiDeclaration(node: Node, opts: BuildApiDecOpts): ApiDecla
Node.isVariableDeclaration(node)
) {
return buildVariableDec(node, opts);
} else if (Node.isTypeLiteralNode(node)) {
} else if (Node.isTypeLiteral(node)) {
return buildTypeLiteralDec(node, opts);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function maybeCollectReferences({
apiDec,
captureReferences,
}: MaybeCollectReferencesOpt): ApiReference[] | undefined {
if (Node.isReferenceFindableNode(node)) {
if (Node.isReferenceFindable(node)) {
return captureReferences || apiDec.deprecated
? getReferences({ node, plugins, currentPluginId, log })
: undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ export function getCommentsFromNode(node: Node): TextWithLinks | undefined {
}

export function getJSDocs(node: Node): JSDoc[] | undefined {
if (Node.isJSDocableNode(node)) {
if (Node.isJSDocable(node)) {
return node.getJsDocs();
} else if (Node.isVariableDeclaration(node)) {
const gparent = node.getParent()?.getParent();
if (Node.isJSDocableNode(gparent)) {
if (Node.isJSDocable(gparent)) {
return gparent.getJsDocs();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ import { isNamedNode } from '../tsmorph_utils';
export const pathsOutsideScopes: { [key: string]: string } = {};

export function isPrivate(node: ParameterDeclaration | ClassMemberTypes): boolean {
return node.getModifiers().find((mod) => mod.getText() === 'private') !== undefined;
if (Node.isModifierable(node)) {
return node.getModifiers().find((mod) => mod.getText() === 'private') !== undefined;
}
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ export function createServerRouteFactory<
TReturnType,
TRouteCreateOptions
>
) => ServerRoute<
) => Record<
TEndpoint,
TRouteParamsRT,
TRouteHandlerResources,
TReturnType,
TRouteCreateOptions
ServerRoute<TEndpoint, TRouteParamsRT, TRouteHandlerResources, TReturnType, TRouteCreateOptions>
> {
return (route) => route;
return (route) => ({ [route.endpoint]: route } as any);
}

This file was deleted.

1 change: 0 additions & 1 deletion packages/kbn-server-route-repository/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

export { createServerRouteRepository } from './create_server_route_repository';
export { createServerRouteFactory } from './create_server_route_factory';
export { formatRequest } from './format_request';
export { parseEndpoint } from './parse_endpoint';
Expand Down
58 changes: 22 additions & 36 deletions packages/kbn-server-route-repository/src/test_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,17 @@
* Side Public License, v 1.
*/
import * as t from 'io-ts';
import { createServerRouteRepository } from './create_server_route_repository';
import { createServerRouteFactory } from './create_server_route_factory';
import { decodeRequestParams } from './decode_request_params';
import { EndpointOf, ReturnOf, RouteRepositoryClient } from './typings';

function assertType<TShape = never>(value: TShape) {
return value;
}

// Generic arguments for createServerRouteRepository should be set,
// if not, registering routes should not be allowed
createServerRouteRepository().add({
// @ts-expect-error
endpoint: 'any_endpoint',
// @ts-expect-error
handler: async ({ params }) => {},
});

// If a params codec is not set, its type should not be available in
// the request handler.
createServerRouteRepository<{}, {}>().add({
createServerRouteFactory<{}, {}>()({
endpoint: 'endpoint_without_params',
handler: async (resources) => {
// @ts-expect-error Argument of type '{}' is not assignable to parameter of type '{ params: any; }'.
Expand All @@ -35,7 +26,7 @@ createServerRouteRepository<{}, {}>().add({

// If a params codec is set, its type _should_ be available in the
// request handler.
createServerRouteRepository<{}, {}>().add({
createServerRouteFactory<{}, {}>()({
endpoint: 'endpoint_with_params',
params: t.type({
path: t.type({
Expand All @@ -48,7 +39,7 @@ createServerRouteRepository<{}, {}>().add({
});

// Resources should be passed to the request handler.
createServerRouteRepository<{ context: { getSpaceId: () => string } }, {}>().add({
createServerRouteFactory<{ context: { getSpaceId: () => string } }, {}>()({
endpoint: 'endpoint_with_params',
params: t.type({
path: t.type({
Expand All @@ -62,7 +53,7 @@ createServerRouteRepository<{ context: { getSpaceId: () => string } }, {}>().add
});

// Create options are available when registering a route.
createServerRouteRepository<{}, { options: { tags: string[] } }>().add({
createServerRouteFactory<{}, { options: { tags: string[] } }>()({
endpoint: 'endpoint_with_params',
params: t.type({
path: t.type({
Expand All @@ -77,16 +68,18 @@ createServerRouteRepository<{}, { options: { tags: string[] } }>().add({
},
});

const repository = createServerRouteRepository<{}, {}>()
.add({
const createServerRoute = createServerRouteFactory<{}, {}>();

const repository = {
...createServerRoute({
endpoint: 'endpoint_without_params',
handler: async () => {
return {
noParamsForMe: true,
};
},
})
.add({
}),
...createServerRoute({
endpoint: 'endpoint_with_params',
params: t.type({
path: t.type({
Expand All @@ -98,8 +91,8 @@ const repository = createServerRouteRepository<{}, {}>()
yesParamsForMe: true,
};
},
})
.add({
}),
...createServerRoute({
endpoint: 'endpoint_with_optional_params',
params: t.partial({
query: t.partial({
Expand All @@ -111,7 +104,8 @@ const repository = createServerRouteRepository<{}, {}>()
someParamsForMe: true,
};
},
});
}),
};

type TestRepository = typeof repository;

Expand Down Expand Up @@ -146,26 +140,21 @@ const client: TestClient = {} as any;
// It should respect any additional create options.

// @ts-expect-error Property 'timeout' is missing
client({
endpoint: 'endpoint_without_params',
});
client('endpoint_without_params', {});

client({
endpoint: 'endpoint_without_params',
client('endpoint_without_params', {
timeout: 1,
});

// It does not allow params for routes without a params codec
client({
endpoint: 'endpoint_without_params',
client('endpoint_without_params', {
// @ts-expect-error Object literal may only specify known properties, and 'params' does not exist in type
params: {},
timeout: 1,
});

// It requires params for routes with a params codec
client({
endpoint: 'endpoint_with_params',
client('endpoint_with_params', {
params: {
// @ts-expect-error property 'serviceName' is missing in type '{}'
path: {},
Expand All @@ -174,14 +163,12 @@ client({
});

// Params are optional if the codec has no required keys
client({
endpoint: 'endpoint_with_optional_params',
client('endpoint_with_optional_params', {
timeout: 1,
});

// If optional, an error will still occur if the params do not match
client({
endpoint: 'endpoint_with_optional_params',
client('endpoint_with_optional_params', {
timeout: 1,
params: {
// @ts-expect-error Object literal may only specify known properties, and 'path' does not exist in type
Expand All @@ -190,8 +177,7 @@ client({
});

// The return type is correctly inferred
client({
endpoint: 'endpoint_with_params',
client('endpoint_with_params', {
params: {
path: {
serviceName: '',
Expand Down
Loading

0 comments on commit 7f5e6c3

Please sign in to comment.