Skip to content

Fix derived type generation to use directly_queryable? instead of root_document_type? - #1096

Merged
myronmarston merged 2 commits into
block:mainfrom
marcdaniels-toast:mdaniels/fix-derived-type-generation-semantic
Mar 30, 2026
Merged

Fix derived type generation to use directly_queryable? instead of root_document_type?#1096
myronmarston merged 2 commits into
block:mainfrom
marcdaniels-toast:mdaniels/fix-derived-type-generation-semantic

Conversation

@marcdaniels-toast

@marcdaniels-toast marcdaniels-toast commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

Consider the following example schema:

graph TD
    DC[["DistributionChannel<br/>interface<br/>💾 own index<br/>directly queryable"]]
    R[["Retail<br/>interface<br/>⬆️ inherited index<br/>directly queryable"]]
    OS["OnlineStore<br/>concrete<br/>⬆️ inherited index<br/>NOT directly queryable"]
    PS[["PhysicalStore<br/>concrete<br/>💾 own index<br/>directly queryable"]]
    MS["MobileStore<br/>concrete<br/>⬆️ inherited index<br/>NOT directly queryable"]
    
    DC -->|implements| R
    R -->|implements| OS
    R -->|implements| PS
    R -->|implements| MS
Loading

Legend:

  • 💾 = Defines own index via t.index "..."
  • ⬆️ = Inherits index from an ancestor
  • Double border = should have all derived types (Connection, Edge, Aggregation, etc.)
  • Single border = should not have derived types of their own

And our semantic definition of root_document_type? and directly_queryable? are:

  • root_document_type?: Returns true if a type is stored in an index (own or inherited)
  • directly_queryable?: Returns true if a type has query fields on the root Query type

Before indexes could be inherited, these were equivalent. With inheritance, concrete types like OnlineStore can
inherit an index from DistributionChannel without being directly queryable.

When I introduceddirectly_queryable? in #1067, I didn't think much about the derived types (Connection, Edge, Aggregation, Highlights, GroupedBy, AggregatedValues, SortOrderInput). But working on the full end-to-end of inherited indexes I had to update hidden_types_spec.rb and this drew my attention to these types.

OnlineStore and MobileStore inherit an index from DistributionChannel, making root_document_type? == true for them. But they have no query fields—they're only accessible through retailers or distribution_channel.

Before this fix, derived types (Connection, Edge, Aggregation, Highlights, GroupedBy, AggregatedValues, SortOrderInput) were generated based on root_document_type?, causing:

  • Types like OnlineStore to generate unused derived types (e.g., OnlineStoreConnection)
  • These types would be orphaned (not reachable from Query root) and pruned during SDL generation, wasting schema artifact space and causing potential confusion. I ran into this when adjusting the hidden_types_spec.rb in the full spike branch.

Solution

Generate derived types based on directly_queryable? instead of root_document_type?:

Root derived types (only for directly queryable types):

  • Connection, Edge, Aggregation, AggregationConnection, AggregationEdge, SortOrderInput
  • Only relevant for types with root Query fields
  • Generated based on directly_queryable?

Remaining derived types (for all types):

  • GroupedBy, AggregatedValues, Highlights
  • Needed when types are embedded in aggregations/filters/search
  • Generated for all types (GraphQL gem prunes unreferenced ones)

…t_document_type?

Prevents derived types (Connection, Edge, Aggregation, Highlights, GroupedBy,
AggregatedValues, SortOrderInput) from being generated for types that only
inherit an index. Also renames EnumsForRootDocumentTypes to
EnumsForDirectlyQueryableTypes and adds test coverage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcdaniels-toast marcdaniels-toast changed the title Fix derived type generation to use directly_queryable? instead of roo… Fix derived type generation to use directly_queryable? instead of root_document_type? Mar 27, 2026

@myronmarston myronmarston left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great explanation of the issue! I'm in favor of this for the derived types that only apply to directly queryable types. But I'm less onboard for the derived types that usually apply in all cases.

These types are now generated for all types (not just directly queryable ones) since they may be used when the type is embedded in another type. The GraphQL gem automatically prunes unreferenced types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@myronmarston
myronmarston merged commit 0bcf242 into block:main Mar 30, 2026
30 of 32 checks passed
myronmarston added a commit that referenced this pull request Mar 30, 2026
This allows us to define the list of root derived types once,
and then expect different things of all elements in the list.
@marcdaniels-toast
marcdaniels-toast deleted the mdaniels/fix-derived-type-generation-semantic branch March 30, 2026 21:20
myronmarston added a commit that referenced this pull request Mar 30, 2026
This allows us to define the list of root derived types once,
and then expect different things of all elements in the list.
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.

2 participants