Skip to content
Open
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
1 change: 1 addition & 0 deletions packages/cli/api-importers/graphql/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"test:update": "vitest --run -u --passWithNoTests"
},
"dependencies": {
"@fern-api/core-utils": "workspace:*",
"@fern-api/fdr-sdk": "catalog:",
"@fern-api/fs-utils": "workspace:*",
"@fern-api/task-context": "workspace:*",
Expand Down
100 changes: 57 additions & 43 deletions packages/cli/api-importers/graphql/src/GraphQLConverter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { FdrAPI } from "@fern-api/fdr-sdk";
import { assertNever } from "@fern-api/core-utils";
import { FdrAPI, FernNavigation } from "@fern-api/fdr-sdk";
import { AbsoluteFilePath } from "@fern-api/fs-utils";
import { TaskContext } from "@fern-api/task-context";
import { readFile } from "fs/promises";
Expand All @@ -13,6 +14,7 @@ import {
GraphQLInputType,
GraphQLInterfaceType,
GraphQLList,
GraphQLNamedType,
GraphQLNonNull,
GraphQLObjectType,
GraphQLOutputType,
Expand All @@ -26,6 +28,17 @@ import { mergeGraphQlDocuments } from "./mergeGraphQlDocuments.js";
export interface GraphQLConverterResult {
graphqlOperations: Record<FdrAPI.GraphQlOperationId, FdrAPI.api.v1.register.GraphQlOperation>;
types: Record<FdrAPI.TypeId, FdrAPI.api.v1.register.TypeDefinition>;
/**
* The GraphQL kind each documented type was declared with, keyed like `types`.
*
* This is the set of types that get a page, so it is a subset of `types`: an operation
* namespace is left out because its fields are documented as operations, while its definition
* stays in `types` for the namespace query's page to render. Look up with a null check.
*
* The FDR type shape is shared with OpenAPI and gRPC and cannot express a GraphQL kind, so
* the kind travels alongside the types rather than inside them.
*/
typeCategories: Record<FdrAPI.TypeId, FernNavigation.GraphQlTypeCategory>;
}

export interface GraphQlExampleInput {
Expand Down Expand Up @@ -55,6 +68,8 @@ export class GraphQLConverter {
private namespace: string | undefined;
private processingTypes: Set<string> = new Set();
private types: Record<FdrAPI.TypeId, FdrAPI.api.v1.register.TypeDefinition> = {};
private typeCategories: Record<FdrAPI.TypeId, FernNavigation.GraphQlTypeCategory> = {};
private namespaceTypeNames: Set<string> = new Set();
private examplesByOperation: Map<string, FdrAPI.api.v1.register.GraphQlExample[]> = new Map();

constructor({
Expand Down Expand Up @@ -235,7 +250,14 @@ export class GraphQLConverter {

const graphqlOperations = this.resolveOperationIds(pendingOperations);

return { graphqlOperations, types: this.types };
// A namespace type's fields are documented as operations, so it groups operations rather
// than being a documented type: no kind means no type page. Its definition stays in
// `types` because a namespace query's page renders the nested fields from it.
for (const typeName of this.namespaceTypeNames) {
delete this.typeCategories[this.getNamespacedTypeId(typeName)];
}

return { graphqlOperations, types: this.types, typeCategories: this.typeCategories };
}

private resolveOperationIds(
Expand Down Expand Up @@ -289,6 +311,7 @@ export class GraphQLConverter {
}

const typeId = this.getNamespacedTypeId(typeName);
this.typeCategories[typeId] = this.typeCategoryOf(type);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 suggestion

The category is written before any of the kind-specific branches run, so if a branch bails out (recursion guard, unsupported/skipped conversion) you get a typeCategories key with no corresponding types entry — breaking the "keyed exactly like types" contract the docstring and tests promise. Safer to set the category at the same point each branch writes this.types[typeId], or to assert/derive it from this.types keys at the end of convert().


if (type instanceof GraphQLEnumType) {
this.processingTypes.add(typeName);
Expand Down Expand Up @@ -374,6 +397,31 @@ export class GraphQLConverter {
}
}

// Derived from the declared kind, never from the converted FDR shape: an interface and an
// object both convert to `object` and a custom scalar to `alias`, so shape-sniffing would
// mislabel types.
private typeCategoryOf(type: GraphQLNamedType): FernNavigation.GraphQlTypeCategory {
if (type instanceof GraphQLObjectType) {
return "object";
}
if (type instanceof GraphQLInputObjectType) {
return "input";
}
if (type instanceof GraphQLEnumType) {
return "enum";
}
if (type instanceof GraphQLInterfaceType) {
return "interface";
}
if (type instanceof GraphQLUnionType) {
return "union";
}
if (type instanceof GraphQLScalarType) {
return "scalar";
}
assertNever(type);
}
Comment on lines +403 to +423

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 warning

instanceof checks against graphql-js classes are unreliable when more than one copy of graphql ends up in the dependency tree (a classic pnpm/monorepo hazard) — that's precisely why graphql-js exports isObjectType, isInputObjectType, isEnumType, isInterfaceType, isUnionType, isScalarType, which check the internal symbol tag instead. Here the failure mode is worse than elsewhere in the file: falling through hits assertNever, which throws and kills the whole conversion instead of just mislabeling a type.

Consider switching to the predicates (they narrow just as well, so assertNever still gives you the exhaustiveness guarantee), or at minimum degrade gracefully rather than throwing on an unrecognized kind.


// Builds an operation id of the form `<operationType>_<segments joined by ".">`.
// Flat (top-level) ids use a single segment; namespaced ids include the full field
// path so that fields sharing a leaf name across namespaces resolve to distinct ids.
Expand Down Expand Up @@ -409,6 +457,7 @@ export class GraphQLConverter {
operation: this.convertField(field, fieldName, operationType)
});
}
this.namespaceTypeNames.add(returnRawType.name);
this.convertNamespaceOperations(returnRawType, operationType, pending, [fieldName]);
} else {
const flatId = this.buildOperationId(operationType, [fieldName]);
Expand Down Expand Up @@ -688,21 +737,10 @@ export class GraphQLConverter {
})
);

// Only extend interfaces that are converted to plain objects (no implementations).
// Interfaces with implementations are converted to undiscriminatedUnion, and the
// frontend's unwrapObjectType only supports extending object types.
// GraphQL implementing types already include all interface fields, so extends is
// only needed for documentation purposes when the interface is a plain object.
const interfaces = type.getInterfaces();
const extendsIds = interfaces
.filter((iface) => {
if (!this.schema) {
return true;
}
const implementations = this.schema.getPossibleTypes(iface);
return implementations.length === 0;
})
.map((iface) => this.getNamespacedTypeId(iface.name));
// `extends` carries the `implements` clause, which is the only edge the docs have to
// resolve an interface's implementors. Implementing types already inline every interface
// field, so the extended properties are deduplicated away when rendering.
const extendsIds = type.getInterfaces().map((iface) => this.getNamespacedTypeId(iface.name));

return {
type: "object",
Expand All @@ -712,33 +750,9 @@ export class GraphQLConverter {
};
}

// An interface is its own set of fields, not the union of the types that implement it: the
// implementors are reachable from each implementing type's `extends`.
private convertInterfaceTypeDefinition(type: GraphQLInterfaceType): FdrAPI.api.v1.register.TypeShape {
if (!this.schema) {
return this.convertInterfaceAsObject(type);
}

const implementations = this.schema.getPossibleTypes(type);
if (implementations.length === 0) {
return this.convertInterfaceAsObject(type);
}

return {
type: "undiscriminatedUnion",
variants: implementations.map((impl) => ({
typeName: impl.name,
displayName: impl.name,
type: {
type: "id",
value: this.getNamespacedTypeId(impl.name),
default: undefined
},
description: impl.description ?? undefined,
availability: undefined
}))
};
}

private convertInterfaceAsObject(type: GraphQLInterfaceType): FdrAPI.api.v1.register.TypeShape {
const fields = type.getFields();
const properties: FdrAPI.api.v1.register.ObjectProperty[] = Object.entries(fields).map(
([fieldName, field]) => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,73 @@ describe("GraphQLConverter custom scalars", () => {
expect(graphqlOperations[FdrAPI.GraphQlOperationId("query_users")]?.examples?.[0]?.name).toBe("from spec A");
});
});

describe("GraphQLConverter type categories", () => {
const TYPE_CATEGORIES_SCHEMA = join(
FIXTURES_DIR,
RelativeFilePath.of("type-categories"),
RelativeFilePath.of("schema.graphql")
);

it("categorizes every emitted type by its GraphQL kind", async () => {
const converter = new GraphQLConverter({
context: createMockTaskContext(),
filePath: TYPE_CATEGORIES_SCHEMA
});

const { types, typeCategories } = await converter.convert();

// Asserted as a whole map: a type gaining or losing a category is as much a
// regression as a miscategorized one. `Auditable` is implemented by nothing and
// `Node` by two types, and both stay interfaces — the category comes from the
// declared kind, not from the converted shape (which is `object` for either).
expect(typeCategories).toEqual({
[FdrAPI.TypeId("Product")]: "object",
[FdrAPI.TypeId("Collection")]: "object",
[FdrAPI.TypeId("ProductCreateInput")]: "input",
[FdrAPI.TypeId("ProductSortKeys")]: "enum",
[FdrAPI.TypeId("DateTime")]: "scalar",
[FdrAPI.TypeId("Node")]: "interface",
[FdrAPI.TypeId("Auditable")]: "interface",
[FdrAPI.TypeId("SearchResult")]: "union"
});

// Nothing here is an operation namespace, so every type is documented and the two maps
// line up. See the namespace test below for the case where they deliberately do not.
expect(Object.keys(typeCategories).sort()).toEqual(Object.keys(types).sort());
});

it("gives an operation namespace no category, so it gets no type page", async () => {
const converter = new GraphQLConverter({
context: createMockTaskContext(),
filePath: join(FIXTURES_DIR, RelativeFilePath.of("namespace-types"), RelativeFilePath.of("schema.graphql"))
});

const { types, typeCategories } = await converter.convert();

// A namespace type's fields are documented as operations, so listing it as a type too
// would duplicate every one of its fields and arguments on the referenced types' pages.
expect(typeCategories).toEqual({
[FdrAPI.TypeId("Account")]: "object",
[FdrAPI.TypeId("InventoryLocation")]: "object"
});

// The definitions stay: a namespace query's page renders its nested fields from them.
expect(Object.keys(types)).toContain("AccountMutations");
expect(Object.keys(types)).toContain("InventoryQueries");
});

it("namespaces category keys alongside type ids", async () => {
const converter = new GraphQLConverter({
context: createMockTaskContext(),
filePath: TYPE_CATEGORIES_SCHEMA,
namespace: "myapi"
});

const { types, typeCategories } = await converter.convert();

expect(typeCategories[FdrAPI.TypeId("myapi_ProductCreateInput")]).toBe("input");
expect(typeCategories[FdrAPI.TypeId("ProductCreateInput")]).toBeUndefined();
expect(Object.keys(typeCategories).sort()).toEqual(Object.keys(types).sort());
});
});
Loading
Loading