Skip to content

Comments

fix: merge interface fields into derived types in SDL generation#111

Open
kleberbaum wants to merge 1 commit intogetcronit:v3from
kleberbaum:fix/inherited-fields-in-derived-types
Open

fix: merge interface fields into derived types in SDL generation#111
kleberbaum wants to merge 1 commit intogetcronit:v3from
kleberbaum:fix/inherited-fields-in-derived-types

Conversation

@kleberbaum
Copy link
Member

Summary

Fixes #110 — inherited/implemented fields are not included in the SDL for derived types, causing GraphQL schema validation to fail at runtime.

Problem

When class Traffic extends RegionNode, Pylon correctly:

  1. Detects the inheritance relationship
  2. Creates the IRegionNode interface with all fields
  3. Adds implements IRegionNode to Traffic

But it does not copy the interface fields into Traffic's field list in the SDL. GraphQL-js validateSchema then fails:

Interface field IRegionNode.id expected but Traffic does not provide it.

Fix

In addInterfaceToDerived (schema-parser.ts), after adding the implements clause, merge all interface fields into the derived type's field list (skipping fields already declared on the derived type).

if (targetInterface) {
  for (const ifaceField of targetInterface.fields) {
    const alreadyHasField = derivedSchemaType.fields.some(
      f => f.name === ifaceField.name
    )
    if (!alreadyHasField) {
      derivedSchemaType.fields.push({...ifaceField})
    }
  }
}

Test plan

  • All 22 existing schema tests pass (inheritance, generics, basic-features, inputs-and-args)
  • Verified at runtime on Cloudflare Workers with a real-world GTFS GraphQL API using Traffic extends RegionNode and Tourism extends RegionNode

When a derived type declares `implements IFoo`, the SDL must include
all interface fields in the derived type's field list. Without this,
GraphQL schema validation fails at runtime:

  "Interface field IFoo.x expected but Bar does not provide it."

The fix merges interface fields into derived type field lists during
the `addInterfaceToDerived` pass, skipping fields already declared
on the derived type.

Fixes getcronit#110
@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2026

⚠️ No Changeset found

Latest commit: c5722c7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant