type safety

Author: asoorm

protographic/src/operations/message-builder.ts

@@ -18,6 +18,7 @@ import {
   isInterfaceType,
   isUnionType,
   GraphQLInterfaceType,
+  GraphQLUnionType,
 } from 'graphql';
 import { mapGraphQLTypeToProto, ProtoTypeInfo } from './type-mapper.js';
 import { FieldNumberManager } from './field-numbering.js';
@@ -58,7 +59,7 @@ export interface MessageBuilderOptions {
 export function buildMessageFromSelectionSet(
   messageName: string,
   selectionSet: SelectionSetNode,
-  parentType: GraphQLObjectType,
+  parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
 ): protobuf.Type {
@@ -71,7 +72,7 @@ export function buildMessageFromSelectionSet(
 
   const collectFields = (
     selections: readonly SelectionNode[],
-    currentType: GraphQLObjectType | GraphQLInterfaceType,
+    currentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   ) => {
     for (const selection of selections) {
       if (selection.kind === 'Field') {
@@ -151,7 +152,7 @@ export function buildMessageFromSelectionSet(
 function processFieldSelection(
   field: FieldNode,
   message: protobuf.Type,
-  parentType: GraphQLObjectType | GraphQLInterfaceType,
+  parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,
@@ -171,6 +172,13 @@ function processFieldSelection(
   }
 
   // Get the field definition from the parent type
+  // Union types don't have fields directly, so skip field validation for them
+  if (isUnionType(parentType)) {
+    // Union types should only be processed through inline fragments
+    // This shouldn't happen in normal GraphQL, but we'll handle it gracefully
+    return;
+  }
+  
   const fieldDef = parentType.getFields()[fieldName];
   if (!fieldDef) {
     throw new Error(
@@ -188,9 +196,9 @@ function processFieldSelection(
       // Use simple name since message will be nested inside parent
       const nestedMessageName = upperFirst(camelCase(fieldName));
 
-      // For interfaces and unions, we use the base type to collect fields from inline fragments
-      // For object types, we process normally
-      const typeForSelection = isObjectType(namedType) ? namedType : (namedType as any);
+      // For interfaces and unions, we use the type directly for processing
+      // Union types will only work with inline fragments that specify concrete types
+      const typeForSelection = namedType;
 
       const nestedMessage = buildMessageFromSelectionSet(
         nestedMessageName,
@@ -300,13 +308,13 @@ function processFieldSelection(
 function processInlineFragment(
   fragment: InlineFragmentNode,
   message: protobuf.Type,
-  parentType: GraphQLObjectType,
+  parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,
 ): void {
   // Determine the type for this inline fragment
-  let fragmentType: GraphQLObjectType;
+  let fragmentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType;
 
   if (fragment.typeCondition) {
     // Type condition specified: ... on User
@@ -319,8 +327,8 @@ function processInlineFragment(
     }
 
     const type = schema.getType(typeName);
-    if (!type || !isObjectType(type)) {
-      // Type not found or not an object type - skip
+    if (!type || !(isObjectType(type) || isInterfaceType(type) || isUnionType(type))) {
+      // Type not found or not a supported type - skip
       return;
     }
 
@@ -352,7 +360,7 @@ function processInlineFragment(
 function processFragmentSpread(
   spread: FragmentSpreadNode,
   message: protobuf.Type,
-  parentType: GraphQLObjectType,
+  parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,

protographic/tests/operations/request-builder.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from 'vitest';
-import { buildSchema, parse } from 'graphql';
+import { buildSchema, parse, GraphQLInputObjectType, GraphQLEnumType } from 'graphql';
 import { buildRequestMessage, buildInputObjectMessage, buildEnumType } from '../../src/operations/request-builder';
 import { createFieldNumberManager } from '../../src/operations/field-numbering';
 
@@ -244,7 +244,7 @@ describe('Request Builder', () => {
         throw new Error('Invalid input type');
       }
 
-      const message = buildInputObjectMessage(inputType as any);
+      const message = buildInputObjectMessage(inputType as GraphQLInputObjectType);
 
       expect(message.name).toBe('UserInput');
       expect(message.fieldsArray).toHaveLength(3);
@@ -270,7 +270,7 @@ describe('Request Builder', () => {
         throw new Error('Invalid input type');
       }
 
-      const message = buildInputObjectMessage(inputType as any);
+      const message = buildInputObjectMessage(inputType as GraphQLInputObjectType);
 
       expect(message.name).toBe('UserInput');
       expect(message.fields.profile).toBeDefined();
@@ -292,7 +292,7 @@ describe('Request Builder', () => {
 
       const manager = createFieldNumberManager();
 
-      const message = buildInputObjectMessage(inputType as any, {
+      const message = buildInputObjectMessage(inputType as GraphQLInputObjectType, {
         fieldNumberManager: manager,
       });
 
@@ -316,7 +316,7 @@ describe('Request Builder', () => {
         throw new Error('Invalid enum type');
       }
 
-      const protoEnum = buildEnumType(enumType as any);
+      const protoEnum = buildEnumType(enumType as GraphQLEnumType);
 
       expect(protoEnum.name).toBe('Status');
       expect(protoEnum.values.UNSPECIFIED).toBe(0);
@@ -338,7 +338,7 @@ describe('Request Builder', () => {
         throw new Error('Invalid enum type');
       }
 
-      const protoEnum = buildEnumType(enumType as any);
+      const protoEnum = buildEnumType(enumType as GraphQLEnumType);
 
       expect(protoEnum.values.UNSPECIFIED).toBe(0);
       expect(protoEnum.values.ADMIN).toBeGreaterThan(0);
@@ -359,7 +359,7 @@ describe('Request Builder', () => {
         throw new Error('Invalid enum type');
       }
 
-      const protoEnum = buildEnumType(enumType as any);
+      const protoEnum = buildEnumType(enumType as GraphQLEnumType);
 
       expect(protoEnum.values.UNSPECIFIED).toBe(0);
       expect(protoEnum.values.LOW).toBe(1);

protographic/tests/util.ts

@@ -136,19 +136,31 @@ export function getReservedNumbers(root: protobufjs.Root, typeName: string, isEn
     return [];
   }
 
-  // Type assertion needed because protobufjs doesn't expose reserved property in type definitions
-  const typeWithReserved = type as any;
-  return (
-    typeWithReserved.reserved?.map((range: number | { start: number; end: number }) => {
-      if (typeof range === 'number') {
-        return range;
-      } else if (range.start === range.end) {
-        return range.start;
+  // Use the existing reserved property from protobufjs types
+  if (!type.reserved) {
+    return [];
+  }
+
+  const numbers: number[] = [];
+  for (const range of type.reserved) {
+    if (typeof range === 'string') {
+      // Skip string reserved fields (field names)
+      continue;
+    }
+    if (Array.isArray(range)) {
+      // Handle number arrays [start, end]
+      if (range.length === 2) {
+        const [start, end] = range;
+        for (let i = start; i <= end; i++) {
+          numbers.push(i);
+        }
+      } else {
+        numbers.push(...range);
       }
-      // For ranges, just return the start for simplicity
-      return range.start;
-    }) || []
-  );
+    }
+  }
+  
+  return numbers;
 }
 
 /**