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

Add support for JSDoc descriptions from object types #3

Merged
merged 5 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Changes to be included in the next upcoming release

## v0.11.0
- Add support for parallel execution of readonly functions ([#2](https://github.com/hasura/ndc-nodejs-lambda/pull/2))
- Add support for JSDoc descriptions from object types ([#3](https://github.com/hasura/ndc-nodejs-lambda/pull/3))
Copy link

Choose a reason for hiding this comment

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

👍


## v0.10.0
- Add missing query.variables capability
Expand Down
36 changes: 36 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,42 @@ export async function test(statusCode: number): Promise<string> {

Non-readonly functions are not invoked in parallel within the same mutation request to the connector, so it is invalid to use the @paralleldegree JSDoc tag on those functions.

### Documentation
*Note: this feature is still in development.*

JSDoc comments on your functions and types are used to provide descriptions for types exposed in your GraphQL schema. For example:

```typescript
/** Different types of greetings */
interface Greeting {
/** A greeting if you want to be polite */
polite: string
/** A casual-toned greeting */
casual: string
}

/**
* Creates a greeting string using the specified name
*
* @param title The person's title, for example, Mr or Mrs
* @param firstName The person's first name
* @param lastName The person's last name (surname)
* @readonly
*/
export function greet(title: string, firstName: string, lastName: string): Greeting {
return {
polite: `Hello ${name.title} ${name.lastName}`,
casual: `G'day ${name.firstName}`
}
}
```

Descriptions are collected for:
* Functions
* Function parameters
* Types
* Type properties
Comment on lines +199 to +203
Copy link

Choose a reason for hiding this comment

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

👍


## Deploying with `hasura3 connector create`

You will need:
Expand Down
53 changes: 38 additions & 15 deletions ndc-lambda-sdk/src/inference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function deriveFunctionSchema(functionDeclaration: ts.FunctionDeclaration, expor
const functionSymbol = context.typeChecker.getSymbolAtLocation(functionIdentifier) ?? throwError(`Function '${exportedFunctionName}' didn't have a symbol`);
const functionType = context.typeChecker.getTypeOfSymbolAtLocation(functionSymbol, functionDeclaration);

const functionDescription = ts.displayPartsToString(functionSymbol.getDocumentationComment(context.typeChecker)).trim();
const functionDescription = getDescriptionFromJsDoc(functionSymbol, context.typeChecker);
const markedReadonlyInJsDoc = functionSymbol.getJsDocTags().find(e => e.name === "readonly") !== undefined;
const parallelDegreeResult = getParallelDegreeFromJsDoc(functionSymbol, markedReadonlyInJsDoc);

Expand All @@ -190,14 +190,19 @@ function deriveFunctionSchema(functionDeclaration: ts.FunctionDeclaration, expor

return Result.collectErrors3(functionSchemaArguments, returnTypeResult, parallelDegreeResult)
.map(([functionSchemaArgs, returnType, parallelDegree]) => ({
description: functionDescription ? functionDescription : null,
description: functionDescription,
ndcKind: markedReadonlyInJsDoc ? schema.FunctionNdcKind.Function : schema.FunctionNdcKind.Procedure,
arguments: functionSchemaArgs,
resultType: returnType,
parallelDegree,
}));
}

function getDescriptionFromJsDoc(symbol: ts.Symbol, typeChecker: ts.TypeChecker): string | null {
const description = ts.displayPartsToString(symbol.getDocumentationComment(typeChecker)).trim()
return description ? description : null;
}

function getParallelDegreeFromJsDoc(functionSymbol: ts.Symbol, functionIsReadonly: boolean): Result<number | null, string[]> {
const parallelDegreeTag = functionSymbol.getJsDocTags().find(e => e.name === "paralleldegree");
if (parallelDegreeTag === undefined) {
Expand Down Expand Up @@ -422,15 +427,15 @@ function deriveSchemaTypeIfObjectType(tsType: ts.Type, typePath: TypePathSegment
return new Ok({ type: 'named', name: info.generatedTypeName, kind: "object" });
}

context.objectTypeDefinitions[info.generatedTypeName] = { properties: [] }; // Break infinite recursion
context.objectTypeDefinitions[info.generatedTypeName] = { properties: [], description: null }; // Break infinite recursion

const propertyResults = Result.traverseAndCollectErrors(Array.from(info.members), ([propertyName, propertyType]) => {
return deriveSchemaTypeForTsType(propertyType, [...typePath, { segmentType: "ObjectProperty", typeName: info.generatedTypeName, propertyName }], context, recursionDepth + 1)
.map(propertyType => ({ propertyName: propertyName, type: propertyType }));
const propertyResults = Result.traverseAndCollectErrors(Array.from(info.properties), ([propertyName, propertyInfo]) => {
return deriveSchemaTypeForTsType(propertyInfo.tsType, [...typePath, { segmentType: "ObjectProperty", typeName: info.generatedTypeName, propertyName }], context, recursionDepth + 1)
.map(propertyType => ({ propertyName: propertyName, type: propertyType, description: propertyInfo.description }));
});

if (propertyResults instanceof Ok) {
context.objectTypeDefinitions[info.generatedTypeName] = { properties: propertyResults.data }
context.objectTypeDefinitions[info.generatedTypeName] = { properties: propertyResults.data, description: info.description }
return new Ok({ type: 'named', name: info.generatedTypeName, kind: "object" })
} else {
// Remove the recursion short-circuit to ensure errors are raised if this type is encountered again
Expand Down Expand Up @@ -475,12 +480,19 @@ function unwrapNullableType(ty: ts.Type): [ts.Type, schema.NullOrUndefinability]
: null;
}

type PropertyTypeInfo = {
tsType: ts.Type,
description: string | null,
}

type ObjectTypeInfo = {
// The name of the type; it may be a generated name if it is an anonymous type, or if it from an external module
generatedTypeName: string,
// The member properties of the object type. The types are
// The properties of the object type. The types are
// concrete types after type parameter resolution
members: Map<string, ts.Type>
properties: Map<string, PropertyTypeInfo>,
// The JSDoc comment on the type
description: string | null,
}

// TODO: This can be vastly simplified when I yeet the name qualification stuff
Expand All @@ -490,30 +502,36 @@ function getObjectTypeInfo(tsType: ts.Type, typePath: TypePathSegment[], typeChe
return null;
}

const symbolForDocs = tsType.aliasSymbol ?? tsType.getSymbol();
const description = symbolForDocs ? getDescriptionFromJsDoc(symbolForDocs, typeChecker) : null;

// Anonymous object type - this covers:
// - {a: number, b: string}
// - type Bar = { test: string }
// - type GenericBar<T> = { data: T }
if (tsutils.isObjectType(tsType) && tsutils.isObjectFlagSet(tsType, ts.ObjectFlags.Anonymous)) {
return {
generatedTypeName: qualifyTypeName(tsType, typePath, tsType.aliasSymbol ? typeChecker.typeToString(tsType) : null, functionsFilePath),
members: getMembers(tsType.getProperties(), typeChecker)
properties: getMembers(tsType.getProperties(), typeChecker),
description,
}
}
// Interface type - this covers:
// interface IThing { test: string }
else if (tsutils.isObjectType(tsType) && tsutils.isObjectFlagSet(tsType, ts.ObjectFlags.Interface)) {
return {
generatedTypeName: tsType.getSymbol()?.name ?? generateTypeNameFromTypePath(typePath),
members: getMembers(tsType.getProperties(), typeChecker)
properties: getMembers(tsType.getProperties(), typeChecker),
description,
}
}
// Generic interface type - this covers:
// interface IGenericThing<T> { data: T }
else if (tsutils.isTypeReference(tsType) && tsutils.isObjectFlagSet(tsType.target, ts.ObjectFlags.Interface) && typeChecker.isArrayType(tsType) === false && tsType.getSymbol()?.getName() !== "Promise") {
return {
generatedTypeName: tsType.getSymbol()?.name ?? generateTypeNameFromTypePath(typePath),
members: getMembers(tsType.getProperties(), typeChecker)
properties: getMembers(tsType.getProperties(), typeChecker),
description,
}
}
// Intersection type - this covers:
Expand All @@ -523,16 +541,21 @@ function getObjectTypeInfo(tsType: ts.Type, typePath: TypePathSegment[], typeChe
else if (tsutils.isIntersectionType(tsType)) {
return {
generatedTypeName: qualifyTypeName(tsType, typePath, tsType.aliasSymbol ? typeChecker.typeToString(tsType) : null, functionsFilePath),
members: getMembers(tsType.getProperties(), typeChecker)
properties: getMembers(tsType.getProperties(), typeChecker),
description,
}
}

return null;
}

function getMembers(propertySymbols: ts.Symbol[], typeChecker: ts.TypeChecker) {
function getMembers(propertySymbols: ts.Symbol[], typeChecker: ts.TypeChecker): Map<string, PropertyTypeInfo> {
return new Map(
propertySymbols.map(symbol => [symbol.name, typeChecker.getTypeOfSymbol(symbol)])
propertySymbols.map(symbol => {
const tsType = typeChecker.getTypeOfSymbol(symbol);
const description = getDescriptionFromJsDoc(symbol, typeChecker);
return [symbol.name, {tsType, description}]
})
)
}

Expand Down
11 changes: 10 additions & 1 deletion ndc-lambda-sdk/src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ export type ObjectTypeDefinitions = {
}

export type ObjectTypeDefinition = {
description: string | null,
properties: ObjectPropertyDefinition[]
}

export type ObjectPropertyDefinition = {
propertyName: string,
description: string | null,
type: TypeDefinition,
}

Expand Down Expand Up @@ -193,7 +195,14 @@ export function getNdcSchema(functionsSchema: FunctionsSchema): sdk.SchemaRespon

const objectTypes = mapObjectValues(functionsSchema.objectTypes, objDef => {
return {
fields: Object.fromEntries(objDef.properties.map(propDef => [propDef.propertyName, { type: convertTypeDefinitionToSdkType(propDef.type)}]))
fields: Object.fromEntries(objDef.properties.map(propDef => {
const objField: sdk.ObjectField = {
type: convertTypeDefinitionToSdkType(propDef.type),
...(propDef.description ? { description: propDef.description } : {})
}
return [propDef.propertyName, objField];
})),
...(objDef.description ? { description: objDef.description } : {})
}
});

Expand Down
4 changes: 4 additions & 0 deletions ndc-lambda-sdk/test/execution/prepare-arguments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ describe("prepare arguments", function() {
}
const objectTypes: ObjectTypeDefinitions = {
"MyObject": {
description: null,
properties: [
{
propertyName: "nullOnlyProp",
description: null,
type: {
type: "nullable",
nullOrUndefinability: NullOrUndefinability.AcceptsNullOnly,
Expand All @@ -183,6 +185,7 @@ describe("prepare arguments", function() {
},
{
propertyName: "undefinedOnlyProp",
description: null,
type: {
type: "nullable",
nullOrUndefinability: NullOrUndefinability.AcceptsUndefinedOnly,
Expand All @@ -195,6 +198,7 @@ describe("prepare arguments", function() {
},
{
propertyName: "nullOrUndefinedProp",
description: null,
type: {
type: "nullable",
nullOrUndefinability: NullOrUndefinability.AcceptsEither,
Expand Down
7 changes: 7 additions & 0 deletions ndc-lambda-sdk/test/execution/reshape-result.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,21 @@ describe("reshape result", function() {
describe("projects object types using fields", function() {
const objectTypes: ObjectTypeDefinitions = {
"TestObjectType": {
description: null,
properties: [
{
propertyName: "propA",
description: null,
type: { type: "named", kind: "scalar", name: BuiltInScalarTypeName.String }
},
{
propertyName: "propB",
description: null,
type: { type: "named", kind: "scalar", name: BuiltInScalarTypeName.String }
},
{
propertyName: "nested",
description: null,
type: { type: "nullable", nullOrUndefinability: NullOrUndefinability.AcceptsEither, underlyingType: { type: "named", kind: "object", name: "TestObjectType" } }
}
]
Expand Down Expand Up @@ -162,13 +166,16 @@ describe("reshape result", function() {
describe("projects object types using fields through an array type", function() {
const objectTypes: ObjectTypeDefinitions = {
"TestObjectType": {
description: null,
properties: [
{
propertyName: "propA",
description: null,
type: { type: "named", kind: "scalar", name: BuiltInScalarTypeName.String }
},
{
propertyName: "propB",
description: null,
type: { type: "nullable", nullOrUndefinability: NullOrUndefinability.AcceptsEither, underlyingType: { type: "named", kind: "scalar", name: BuiltInScalarTypeName.String } }
}
]
Expand Down
Loading