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

refactor(ts): strict mode #696

Merged
merged 9 commits into from
Sep 14, 2023
Merged
3 changes: 2 additions & 1 deletion packages/api/src/cli/codegen/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ export default abstract class CodeGeneratorLanguage {

userAgent: string;

requiredPackages: Record<string, { reason: string; url: string }>;
requiredPackages!: Record<string, { reason: string; url: string }>;

constructor(spec: Oas, specPath: string, identifier: string) {
this.spec = spec;
this.specPath = specPath;
this.identifier = identifier;

// User agents should be contextual to the spec in question and the version of `api` that was
// used to generate the SDK. For example, this'll look like `petstore/1.0.0 (api/4.2.0)` for
Expand Down
33 changes: 17 additions & 16 deletions packages/api/src/cli/codegen/languages/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { InstallerOptions } from '../language';
import type Oas from 'oas';
import type { Operation } from 'oas';
import type { HttpMethods, SchemaObject } from 'oas/dist/rmoas.types';
import type { SemVer } from 'semver';
import type {
ClassDeclaration,
JSDocStructure,
Expand Down Expand Up @@ -53,9 +54,9 @@ export default class TSGenerator extends CodeGeneratorLanguage {

types: Map<string, string>;

files: Record<string, string>;
files?: Record<string, string>;

sdk: ClassDeclaration;
sdk!: ClassDeclaration;

schemas: Record<
string,
Expand Down Expand Up @@ -138,7 +139,7 @@ export default class TSGenerator extends CodeGeneratorLanguage {
if (!pkgVersion) {
// If the version that's in `info.version` isn't compatible with semver NPM won't be able to
// handle it properly so we need to fallback to something it can.
pkgVersion = semver.coerce('0.0.0');
pkgVersion = semver.coerce('0.0.0') as SemVer;
}

const pkg: PackageJson = {
Expand Down Expand Up @@ -221,7 +222,7 @@ export default class TSGenerator extends CodeGeneratorLanguage {
sdkSource
.getImportDeclarations()
.find(id => id.getText() === "import type * as types from './types';")
.remove();
?.remove();
}

// If this SDK doesn't use the `HTTPMethodRange` interface for handling `2XX` response status
Expand All @@ -230,7 +231,7 @@ export default class TSGenerator extends CodeGeneratorLanguage {
sdkSource
.getImportDeclarations()
.find(id => id.getText().includes('HTTPMethodRange'))
.replaceWithText("import type { ConfigOptions, FetchResponse } from '@api/core'");
?.replaceWithText("import type { ConfigOptions, FetchResponse } from '@api/core'");
Copy link
Member

Choose a reason for hiding this comment

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

You originally had these as the following but I think optionally chaining these is a little nicer.

const declaration = sdkSource.getImportDeclarations(...).find(...);
if (declaration) declaration.replaceWithText(...);

Cool with the original solution if you want me to revert it though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I suppose this just returns undefined and doesn't actually touch the original sdkSource variable if it can't find anything right? Works for me!

}

if (this.outputJS) {
Expand Down Expand Up @@ -566,10 +567,10 @@ sdk.server('https://eu.api.example.com/v14');`),

let hasOptionalBody = false;
let hasOptionalMetadata = false;
const parameters: {
body?: OptionalKind<ParameterDeclarationStructure>;
metadata?: OptionalKind<ParameterDeclarationStructure>;
} = {};
const parameters = {} as {
body: OptionalKind<ParameterDeclarationStructure>;
metadata: OptionalKind<ParameterDeclarationStructure>;
};
Comment on lines +570 to +573
Copy link
Member

Choose a reason for hiding this comment

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

This slight reshuffling ended up fixing all of those ts-morph typing issues. When we use parameters.body and parameters.metadata they will be present so marking them as optional here is wrong. I believe I had originally done it this way to satisfy setting parameters to an empty object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotta love strict mode


if (paramTypes) {
// If an operation has a request body payload it will only ever have `body` or `formData`,
Expand Down Expand Up @@ -656,7 +657,7 @@ sdk.server('https://eu.api.example.com/v14');`),
// we should only add a docblock to the first overload we create because IDE Intellisense will
// always use that and adding a docblock to all three will bloat the SDK with unused and
// unsurfaced method documentation.
docs: shouldAddAltTypedOverloads ? null : Object.keys(docblock).length ? [docblock] : null,
docs: shouldAddAltTypedOverloads ? undefined : Object.keys(docblock).length ? [docblock] : undefined,
statements: writer => {
/**
* @example return this.core.fetch('/pet/findByStatus', 'get', body, metadata);
Expand Down Expand Up @@ -698,7 +699,7 @@ sdk.server('https://eu.api.example.com/v14');`),
{ ...parameters.metadata, hasQuestionToken: false },
],
returnType,
docs: Object.keys(docblock).length ? [docblock] : null,
docs: Object.keys(docblock).length ? [docblock] : undefined,
});

// Create an overload that just has a single `metadata` parameter.
Expand Down Expand Up @@ -739,13 +740,13 @@ sdk.server('https://eu.api.example.com/v14');`),
*/
loadOperationsAndMethods() {
const operations: Record</* operationId */ string, OperationTypeHousing> = {};
const methods = new Set();
const methods = new Set<HttpMethods>();

// Prepare all of the schemas that we need to process for every operation within this API
// definition.
Object.entries(this.spec.getPaths()).forEach(([, ops]) => {
Object.entries(ops).forEach(([method, operation]: [HttpMethods, Operation]) => {
methods.add(method);
Object.entries(ops).forEach(([method, operation]: [string, Operation]) => {
methods.add(method as HttpMethods);

const operationId = operation.getOperationId({
// This `camelCase` option will clean up any weird characters that might be present in
Expand Down Expand Up @@ -786,7 +787,7 @@ sdk.server('https://eu.api.example.com/v14');`),
transformer: (s: SchemaObject) => {
// As our schemas are dereferenced in the `oas` library we don't want to pollute our
// codegen'd schemas file with duplicate schemas.
if ('x-readme-ref-name' in s) {
if ('x-readme-ref-name' in s && typeof s['x-readme-ref-name'] !== 'undefined') {
const typeName = generateTypeName(s['x-readme-ref-name']);
this.addSchemaToExport(s, typeName, typeName);

Expand Down Expand Up @@ -844,7 +845,7 @@ sdk.server('https://eu.api.example.com/v14');`),
transformer: (s: SchemaObject) => {
// As our schemas are dereferenced in the `oas` library we don't want to pollute our
// codegen'd schemas file with duplicate schemas.
if ('x-readme-ref-name' in s) {
if ('x-readme-ref-name' in s && typeof s['x-readme-ref-name'] !== 'undefined') {
const typeName = generateTypeName(s['x-readme-ref-name']);
this.addSchemaToExport(s, typeName, `${typeName}`);

Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/cli/fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default class Fetcher {
return undefined;
}

return matches.groups.project;
return matches.groups?.project;
}

async load() {
Expand Down
10 changes: 7 additions & 3 deletions packages/api/src/cli/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default class Storage {
*/
source: string;

identifier: string;
identifier!: string;

fetcher: Fetcher;

Expand All @@ -32,7 +32,9 @@ export default class Storage {
this.fetcher = new Fetcher(source);

this.source = source;
this.identifier = identifier;
if (identifier) {
this.identifier = identifier;
}

// This should default to false so we have awareness if we've looked at the lockfile yet.
Storage.lockfile = false;
Expand Down Expand Up @@ -117,7 +119,9 @@ export default class Storage {
if (!isValidForNPM.validForNewPackages) {
// `prompts` doesn't support surfacing multiple errors in a `validate` call so we can only
// surface the first to the user.
throw new Error(`Identifier cannot be used for an NPM package: ${isValidForNPM.errors[0]}`);
throw new Error(
`Identifier cannot be used for an NPM package: ${isValidForNPM?.errors?.[0] || '[error unavailable]'}`,
);
}

return true;
Expand Down
4 changes: 3 additions & 1 deletion packages/api/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
"esModuleInterop": true,
"lib": ["DOM", "DOM.Iterable", "ES2020"],
"noImplicitAny": true,
"outDir": "dist/"
"outDir": "dist/",
"strict": true,
"useUnknownInCatchVariables": false
Copy link
Member

Choose a reason for hiding this comment

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

Allows us to not have to do catch (err: any) nonsense that adds no value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh hell yeah I've been running into this a bunch in rdme, gonna steal this. Thanks!

},
"include": ["./src/**/*"]
}
10 changes: 5 additions & 5 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,24 @@ export type HTTPMethodRange<F extends number, T extends number> = Exclude<Enumer
export { getJSONSchemaDefaults, parseResponse, prepareAuth, prepareParams, prepareServer };

export default class APICore {
spec: Oas;
spec!: Oas;

private auth: (number | string)[] = [];

private server:
| false
| {
url?: string;
url: string;
variables?: Record<string, string | number>;
} = false;

private config: ConfigOptions = {};

private userAgent: string;
private userAgent!: string;

constructor(spec?: Oas, userAgent?: string) {
this.spec = spec;
this.userAgent = userAgent;
if (spec) this.spec = spec;
if (userAgent) this.userAgent = userAgent;
}

setSpec(spec: Oas) {
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/lib/getJSONSchemaDefaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ export default function getJSONSchemaDefaults(jsonSchemas: SchemaWrapper[]) {
schema: SchemaObject,
pointer: string,
rootSchema: SchemaObject,
parentPointer: string,
parentKeyword: string,
parentSchema: SchemaObject,
indexProperty: string,
parentPointer?: string,
parentKeyword?: string,
parentSchema?: SchemaObject,
indexProperty?: string | number,
) => {
if (!pointer.startsWith('/properties/')) {
return;
}

if (Array.isArray(parentSchema?.required) && parentSchema.required.includes(indexProperty)) {
if (Array.isArray(parentSchema?.required) && parentSchema?.required.includes(String(indexProperty))) {
if (schema.type === 'object' && indexProperty) {
defaults[indexProperty] = {};
}
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/lib/prepareAuth.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable no-underscore-dangle */
import type { Operation } from 'oas';
import type { KeyedSecuritySchemeObject } from 'oas/dist/rmoas.types';

export default function prepareAuth(authKey: (number | string)[], operation: Operation) {
if (authKey.length === 0) {
Expand Down Expand Up @@ -58,7 +59,7 @@ export default function prepareAuth(authKey: (number | string)[], operation: Ope
);
}

const scheme = schemes.shift();
const scheme = schemes.shift() as KeyedSecuritySchemeObject;
preparedAuth[scheme._key] = {
user: authKey[0],
pass: authKey.length === 2 ? authKey[1] : '',
Expand All @@ -76,7 +77,7 @@ export default function prepareAuth(authKey: (number | string)[], operation: Ope
.map(([, ps]) => ps.filter(s => usableScheme === s._key))
.reduce((prev, next) => prev.concat(next), []);

const scheme = schemes.shift();
const scheme = schemes.shift() as KeyedSecuritySchemeObject;
switch (scheme.type) {
case 'http':
if (scheme.scheme === 'basic') {
Expand Down
54 changes: 31 additions & 23 deletions packages/core/src/lib/prepareParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ function merge(src: any, target: any) {
*
*/
function processFile(
paramName: string,
paramName: string | undefined,
file: string | ReadStream,
): Promise<{ base64: string; buffer: Buffer; filename: string; paramName: string }> {
): Promise<{ base64?: string; buffer?: Buffer; filename: string; paramName?: string } | undefined> {
if (typeof file === 'string') {
// In order to support relative pathed files, we need to attempt to resolve them.
const resolvedFile = path.resolve(file);
Expand All @@ -103,7 +103,7 @@ function processFile(

return resolve({
paramName,
base64: fileMetadata.content.replace(';base64', `;name=${payloadFilename};base64`),
base64: fileMetadata?.content?.replace(';base64', `;name=${payloadFilename};base64`),
filename: payloadFilename,
buffer: fileMetadata.buffer,
});
Expand All @@ -118,7 +118,7 @@ function processFile(

return {
paramName,
base64: base64.replace(';base64', `;name=${payloadFilename};base64`),
base64: base64?.replace(';base64', `;name=${payloadFilename};base64`),
filename: payloadFilename,
buffer,
};
Expand Down Expand Up @@ -228,7 +228,7 @@ export default async function prepareParams(operation: Operation, body?: unknown
}
});

const intersection = Object.keys(body).filter(value => {
const intersection = Object.keys(body as NonNullable<unknown>).filter(value => {
if (Object.keys(digestedParameters).includes(value)) {
return true;
} else if (headerParams.has(value)) {
Expand All @@ -238,10 +238,10 @@ export default async function prepareParams(operation: Operation, body?: unknown
return false;
}).length;

if (intersection && intersection / Object.keys(body).length > 0.25) {
// If more than 25% of the body intersects with the parameters that we've got on hand, then
// we should treat it as a metadata object and organize into parameters.
if (intersection && intersection / Object.keys(body as NonNullable<unknown>).length > 0.25) {
/* eslint-disable no-param-reassign */
// If more than 25% of the body intersects with the parameters that we've got on hand,
// then we should treat it as a metadata object and organize into parameters.
metadataIntersected = true;
metadata = merge(params.body, body) as Record<string, unknown>;
body = undefined;
Expand Down Expand Up @@ -304,7 +304,9 @@ export default async function prepareParams(operation: Operation, body?: unknown
params.body = fileMetadata.base64;
}

params.files[fileMetadata.filename] = fileMetadata.buffer;
if (fileMetadata.buffer && params?.files) {
params.files[fileMetadata.filename] = fileMetadata.buffer;
}
});
});
}
Expand Down Expand Up @@ -336,7 +338,7 @@ export default async function prepareParams(operation: Operation, body?: unknown
} else if (param.in === 'header') {
// Headers are sent case-insensitive so we need to make sure that we're properly
// matching them when detecting what our incoming payload looks like.
metadataHeaderParam = Object.keys(metadata).find(k => k.toLowerCase() === paramName.toLowerCase());
metadataHeaderParam = Object.keys(metadata).find(k => k.toLowerCase() === paramName.toLowerCase()) || '';
value = metadata[metadataHeaderParam];
}
}
Expand All @@ -348,20 +350,20 @@ export default async function prepareParams(operation: Operation, body?: unknown
/* eslint-disable no-param-reassign */
switch (param.in) {
case 'path':
params.path[paramName] = value;
delete metadata[paramName];
(params.path as NonNullable<typeof params.path>)[paramName] = value;
if (metadata?.[paramName]) delete metadata[paramName];
break;
case 'query':
params.query[paramName] = value;
delete metadata[paramName];
(params.query as NonNullable<typeof params.query>)[paramName] = value;
if (metadata?.[paramName]) delete metadata[paramName];
break;
case 'header':
params.header[paramName.toLowerCase()] = value;
delete metadata[metadataHeaderParam];
(params.header as NonNullable<typeof params.header>)[paramName.toLowerCase()] = value;
if (metadataHeaderParam && metadata?.[metadataHeaderParam]) delete metadata[metadataHeaderParam];
break;
case 'cookie':
params.cookie[paramName] = value;
delete metadata[paramName];
(params.cookie as NonNullable<typeof params.cookie>)[paramName] = value;
if (metadata?.[paramName]) delete metadata[paramName];
break;
default: // no-op
}
Expand All @@ -387,11 +389,17 @@ export default async function prepareParams(operation: Operation, body?: unknown
// or specify a custom auth header (maybe we can't handle their auth case right) this is the
// only way with this library that they can do that.
specialHeaders.forEach(headerName => {
const headerParam = Object.keys(metadata).find(m => m.toLowerCase() === headerName);
const headerParam = Object.keys(metadata || {}).find(m => m.toLowerCase() === headerName);
if (headerParam) {
params.header[headerName] = metadata[headerParam] as string;
// eslint-disable-next-line no-param-reassign
delete metadata[headerParam];
// this if-statement below is a typeguard
if (typeof metadata === 'object') {
// this if-statement below is a typeguard
if (typeof params.header === 'object') {
params.header[headerName] = metadata[headerParam] as string;
}
// eslint-disable-next-line no-param-reassign
delete metadata[headerParam];
}
}
});
}
Expand All @@ -405,7 +413,7 @@ export default async function prepareParams(operation: Operation, body?: unknown
}
}

['body', 'cookie', 'files', 'formData', 'header', 'path', 'query'].forEach((type: keyof typeof params) => {
(['body', 'cookie', 'files', 'formData', 'header', 'path', 'query'] as const).forEach((type: keyof typeof params) => {
if (type in params && isEmpty(params[type])) {
delete params[type];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"lib": ["DOM", "DOM.Iterable", "ES2020"],
"noImplicitAny": true,
"outDir": "dist/",
"strict": false
"strict": true
},
"include": ["./src/**/*"]
}
Loading