Skip to content

fix(cli): emit custom scalars and all named types in GraphQL converter#16140

Open
lifanzou wants to merge 1 commit into
mainfrom
barryzou/fer-10878-cli-stop-dropping-custom-scalars-emit-all-named-graphql
Open

fix(cli): emit custom scalars and all named types in GraphQL converter#16140
lifanzou wants to merge 1 commit into
mainfrom
barryzou/fer-10878-cli-stop-dropping-custom-scalars-emit-all-named-graphql

Conversation

@lifanzou
Copy link
Copy Markdown
Contributor

Description

Linear ticket: Refs FER-10878

The GraphQL converter silently dropped custom scalars. In collectTypeDefinitions, the loop hit a continue for any non-built-in GraphQLScalarType, so custom scalars (e.g. DateTime, URL, EmailAddress) never made it into the types map. They only appeared inlined as scalar primitives at each reference site, which left them without a stable id the frontend can use as an href anchor.

This fix emits every custom scalar as a named alias type (mapping to the appropriate base primitive via the existing convertScalarTypeDefinition / getBaseTypeForCustomScalar logic) with a stable, namespace-aware id, and updates references to custom scalars to point at that id instead of inlining the scalar shape.

Note: this PR intentionally does not add navigation/sidebar nodes for types or touch field-level arguments — those are tracked separately.

Changes Made

  • GraphQLConverter.collectTypeDefinitions: replaced the continue for custom scalars with emission of a named alias TypeDefinition (stable id, description preserved). Built-in scalars are still skipped (they map to inline primitives).
  • GraphQLConverter.convertScalarType: custom-scalar references now return { type: "id", value: <typeId> } so they anchor to the emitted named type. Built-in scalars are unchanged (inline primitives).
  • Tests: added assertion-based unit tests proving (1) every custom scalar in the basic fixture lands in types as an alias, (2) built-in scalars are not emitted, (3) references use the stable id, and (4) ids are namespaced. Updated the basic and account-schema snapshots accordingly.
  • Changelog: added fix-graphql-custom-scalars.yml (type fix).
  • Updated README.md generator (if applicable) — N/A

Testing

  • Unit tests added/updated (GraphQLConverter.test.ts, 6 passing)
  • Snapshots updated (basic.json, account-schema.json)
  • Package compiles (tsc) and lint/format pass (biome)
  • Manual testing completed

🤖 Generated with Claude Code

@lifanzou lifanzou self-assigned this May 31, 2026
The GraphQL converter silently dropped custom scalars: collectTypeDefinitions
hit a `continue` for non-built-in GraphQLScalarType, so scalars like DateTime,
URL, and EmailAddress never landed in the `types` map. They were only inlined
as `scalar` primitives at each reference site, which left them without a stable
id the frontend can use as an href anchor.

Now every custom scalar is emitted as a named alias type (mapping to the
appropriate base primitive) with a stable, namespace-aware id, and references
to custom scalars point to that id instead of inlining the scalar shape.

Refs FER-10878

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lifanzou lifanzou force-pushed the barryzou/fer-10878-cli-stop-dropping-custom-scalars-emit-all-named-graphql branch from 83eead5 to 30ac7d2 Compare May 31, 2026 05:11
@lifanzou lifanzou marked this pull request as ready for review May 31, 2026 05:51
@lifanzou lifanzou requested a review from amckinney as a code owner May 31, 2026 05:51
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@github-actions
Copy link
Copy Markdown
Contributor

Docs Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-05-31T05:29:54Z).

Fixture main PR Delta
docs 230.4s (n=5) 221.2s (35 versions) -9.2s (-4.0%)

Docs generation runs fern generate --docs --preview end-to-end against the benchmark fixture with 35 API versions (each version: markdown processing + OpenAPI-to-IR + FDR upload).
Delta is computed against the nightly baseline on main.
Baseline from nightly run(s) on main (latest: 2026-05-31T05:29:54Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-05-31 06:10 UTC

@github-actions
Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-05-31T05:29:54Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
csharp-sdk square 72s (n=5) 110s (n=5) 64s -8s (-11.1%)
go-sdk square 136s (n=5) 286s (n=5) 131s -5s (-3.7%)
java-sdk square 225s (n=5) 273s (n=5) 205s -20s (-8.9%)
php-sdk square 60s (n=5) 81s (n=5) 52s -8s (-13.3%)
python-sdk square 137s (n=5) 235s (n=5) 123s -14s (-10.2%)
ruby-sdk-v2 square 90s (n=5) 125s (n=5) 86s -4s (-4.4%)
rust-sdk square 169s (n=5) 170s (n=5) 166s -3s (-1.8%)
swift-sdk square 57s (n=5) 736s (n=5) 51s -6s (-10.5%)
ts-sdk square 229s (n=5) 233s (n=5) 218s -11s (-4.8%)

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 fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-05-31T05:29:54Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-05-31 06:12 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant