feat(cli): record the GraphQL kind of every named type - #17542
feat(cli): record the GraphQL kind of every named type#17542devin-ai-integration[bot] wants to merge 5 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Adds a typeCategories map to GraphQLConverterResult derived from the declared GraphQL kind, with a fixture and snapshot updates. Logic is straightforward; two concerns: instanceof + assertNever can throw at runtime under duplicate graphql installs, and the category is recorded before the type is guaranteed to be emitted into types, which can break the documented "keyed exactly like types" invariant.
- 🟡 1 warning(s)
- 🔵 1 suggestion(s)
To request another review, comment /ai-review on this pull request.
| 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); | ||
| } |
There was a problem hiding this comment.
🟡 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.
| } | ||
|
|
||
| const typeId = this.getNamespacedTypeId(typeName); | ||
| this.typeCategories[typeId] = this.typeCategoryOf(type); |
There was a problem hiding this comment.
🔵 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().
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…hql-type-categories
Threads the typeCategories map from GraphQLConverter through DocsDefinitionResolver into ApiReferenceNodeConverter, which now emits one graphqlType nav node per named type under a per-kind section (Objects, Inputs, Enums, Scalars, Interfaces, Unions). Kinds the schema does not declare produce no section. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
`#convertApiDefinitionPackage` recurses into subpackages before reaching the GraphQL types block, so the "first call wins" flag handed the type sections to whichever subpackage was converted first. An API section with both an OpenAPI spec and a GraphQL schema rendered them nested under an unrelated REST tag, at `<api>/<tag>/types/<kind>/<type>` instead of the documented `<api>/types/<kind>/<type>`, while Queries and Mutations correctly stayed at the root. Emission is now keyed off the root package itself rather than call order, and the per-kind groups are collected under a single "GraphQL Types" section: types belong to the schema rather than to any one package, and every GraphQL spec in the API section contributes to that one section. Child slugs are unchanged, so the type-page URLs stay as documented. Adds a `graphql-type-navigation-subpackages` fixture — the same schema plus an OpenAPI spec whose tags become subpackages — which is the shape the existing single-spec fixture could not express. Verified the new test fails against the previous implementation. Co-Authored-By: Claude <noreply@anthropic.com>
…nts clauses Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Description
Linear ticket: Refs
Phase 1 of GraphQL Types rendering: the converter now reports which GraphQL kind each emitted type was declared with, so docs can group a Types section and build
api/types/<category>/<type>routes (the FDR contract from fern-platform#14190,GraphQlTypeCategory = object | input | enum | scalar | interface | union).The kind cannot live inside the FDR
TypeDefinition— that shape is shared with OpenAPI and gRPC, is owned by fern-platform, and is lossy for GraphQL (an object and an interface both convert toobject, a custom scalar toalias). So it travels beside the types:Nav emission (one
graphqlTypenode per type, grouped by category) is deliberately not here: the CLI writes the V1 navigation model, which has noGraphQlTypeNodeuntil fern-platform#14190 ships a new@fern-api/fdr-sdk. This PR is the half that is unblocked today.Changes Made
GraphQLConverterResultgainstypeCategories, populated incollectTypeDefinitionsunder the same namespacedTypeIdused fortypes, so a consumer can look up a category for any type it renders without a fallback.GraphQLObjectType,GraphQLInputObjectType, …) rather than from the converted FDR shape, with anassertNevertail so a new kind is a compile error instead of a mislabeled page.@fern-api/core-utilsadded as a dependency (forassertNever).Testing
type-categoriesfixture exercising all six kinds (the existing GraphQL fixtures cover neitherinputnorscalar), including an orphan interface with no implementors: the case that proves the category comes from the declared kind and not the shape. The full map is asserted, plus that its keys matchtypesexactly and that namespacing applies to both.pnpm turbo run test --filter @fern-api/graphql-to-fdr— 44 passed; existing snapshots updated for the new field.pnpm turbo run compile --filter @fern-api/graphql-to-fdr,pnpm lint:biome,pnpm format.Link to Devin session: https://app.devin.ai/sessions/e73e4cb2bda942aa862ef0d8e2a1db48