From 8f738a774c202c111b576bed168b3b7ba7764fd8 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Sat, 29 Nov 2025 10:09:03 +0000 Subject: [PATCH 01/12] fix - need to ensure that operations are pascal case --- .../grpc-service/commands/generate.ts | 18 -- protographic/src/operation-to-proto.ts | 19 +- .../operations/operation-validation.test.ts | 162 ++++++++++++++++++ 3 files changed, 178 insertions(+), 21 deletions(-) diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index 6080bcdda1..66b38cc7ef 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -27,7 +27,6 @@ type CLIOptions = { customScalarMapping?: string; customScalarMappingFile?: string; maxDepth?: string; - prefixOperationType?: boolean; } & ProtoOptions; export default (opts: BaseCommandOptions) => { @@ -72,11 +71,6 @@ export default (opts: BaseCommandOptions) => { 'Maximum recursion depth for processing nested selections and fragments (default: 50). ' + 'Increase this if you have deeply nested queries or decrease to catch potential circular references earlier.', ); - command.option( - '--prefix-operation-type', - 'Prefix RPC method names with the operation type (Query/Mutation). Only applies with --with-operations. ' + - 'Subscriptions are not prefixed.', - ); command.action(generateCommandAction); return command; @@ -163,11 +157,6 @@ async function generateCommandAction(name: string, options: CLIOptions) { maxDepth = parsed; } - // Validate prefix-operation-type usage - if (options.prefixOperationType && !options.withOperations) { - spinner.warn('--prefix-operation-type flag is ignored when not using --with-operations'); - } - const languageOptions: ProtoOptions = { goPackage: options.goPackage, javaPackage: options.javaPackage, @@ -192,7 +181,6 @@ async function generateCommandAction(name: string, options: CLIOptions) { operationsDir: options.withOperations, customScalarMappings, maxDepth, - prefixOperationType: options.prefixOperationType, }); // Write the generated files @@ -244,7 +232,6 @@ type GenerationOptions = { operationsDir?: string; customScalarMappings?: Record; maxDepth?: number; - prefixOperationType?: boolean; }; /** @@ -335,7 +322,6 @@ function mergeProtoRoots(roots: protobuf.Root[], serviceName: string): protobuf. * @param lockFile - Path to the proto lock file * @param customScalarMappings - Custom scalar type mappings * @param maxDepth - Maximum recursion depth - * @param prefixOperationType - Whether to prefix operation types * @returns Generation result with proto content and lock data * @note All Query operations are automatically marked with NO_SIDE_EFFECTS idempotency level */ @@ -349,7 +335,6 @@ async function generateFromOperations( lockFile: string, customScalarMappings?: Record, maxDepth?: number, - prefixOperationType?: boolean, ): Promise { spinner.text = 'Reading operation files...'; const operationFiles = await readOperationFiles(operationsPath); @@ -376,7 +361,6 @@ async function generateFromOperations( lockData: currentLockData, customScalarMappings, maxDepth, - prefixOperationType, }); // Keep the AST root instead of the string @@ -495,7 +479,6 @@ async function generateProtoAndMapping({ operationsDir, customScalarMappings, maxDepth, - prefixOperationType, }: GenerationOptions): Promise { const schema = await readFile(schemaFile, 'utf8'); const serviceName = upperFirst(camelCase(name)); @@ -518,7 +501,6 @@ async function generateProtoAndMapping({ lockFile, customScalarMappings, maxDepth, - prefixOperationType, ); } else { return generateFromSDL(schema, serviceName, spinner, packageName, languageOptions, lockFile); diff --git a/protographic/src/operation-to-proto.ts b/protographic/src/operation-to-proto.ts index 28b19f2095..3bb70b1b16 100644 --- a/protographic/src/operation-to-proto.ts +++ b/protographic/src/operation-to-proto.ts @@ -264,7 +264,19 @@ class OperationsToProtoVisitor { return; } - // 2. Validate no root-level field aliases (breaks reversibility) + // 2. Validate operation name is PascalCase + // This ensures exact matching between GraphQL operation names and RPC method names + // PascalCase: starts with uppercase, contains at least one lowercase letter + if (!/^[A-Z](?=.*[a-z])[a-zA-Z0-9]*$/.test(operationName)) { + throw new Error( + `Operation name "${operationName}" must be in PascalCase ` + + `(start with uppercase letter, followed by mixed-case letters/numbers). ` + + `Examples: GetUser, CreatePost, OnMessageAdded. ` + + `This ensures the RPC method name exactly matches the GraphQL operation name.`, + ); + } + + // 3. Validate no root-level field aliases (breaks reversibility) if (node.selectionSet) { for (const selection of node.selectionSet.selections) { if (selection.kind === 'Field' && selection.alias) { @@ -277,8 +289,9 @@ class OperationsToProtoVisitor { } } - // 3. Create method name from operation name, optionally prefixed with operation type - let methodName = upperFirst(camelCase(operationName)); + // 4. Create method name from operation name + // Use operation name as-is to ensure exact matching (no transformation) + let methodName = operationName; // Add operation type prefix if requested if (this.prefixOperationType) { diff --git a/protographic/tests/operations/operation-validation.test.ts b/protographic/tests/operations/operation-validation.test.ts index a93d436505..1edb2ce377 100644 --- a/protographic/tests/operations/operation-validation.test.ts +++ b/protographic/tests/operations/operation-validation.test.ts @@ -198,6 +198,168 @@ describe('Operation Validation', () => { }); }); + describe('Operation Name PascalCase Validation', () => { + test('should accept PascalCase operation names', () => { + const operation = ` + query GetUser { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should accept PascalCase with numbers', () => { + const operation = ` + query GetUser123 { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).not.toThrow(); + }); + + test('should reject camelCase operation names', () => { + const operation = ` + query getUser { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "getUser" must be in PascalCase/); + }); + + test('should reject snake_case operation names', () => { + const operation = ` + query get_user { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "get_user" must be in PascalCase/); + }); + + test('should reject all-UPPERCASE operation names', () => { + const operation = ` + query GETUSER { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "GETUSER" must be in PascalCase/); + }); + + test('should reject operation names with only uppercase and numbers', () => { + const operation = ` + query GET123USER { + user { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/Operation name "GET123USER" must be in PascalCase/); + }); + + test('should provide helpful error message for camelCase', () => { + const operation = ` + query getUserById { + user { + id + } + } + `; + + expect(() => { + compileOperationsToProto(operation, schema); + }).toThrow(/must be in PascalCase.*Examples: GetUser, CreatePost, OnMessageAdded/); + }); + + test('should validate mutation operation names', () => { + const mutationSchema = ` + type Mutation { + createUser(name: String!): User + } + + type User { + id: ID! + name: String + } + `; + + const operation = ` + mutation createUser($name: String!) { + createUser(name: $name) { + id + name + } + } + `; + + expect(() => { + compileOperationsToProto(operation, mutationSchema); + }).toThrow(/Operation name "createUser" must be in PascalCase/); + }); + + test('should validate subscription operation names', () => { + const subscriptionSchema = ` + type Query { + ping: String + } + + type Subscription { + messageAdded: Message + } + + type Message { + id: ID! + content: String + } + `; + + const operation = ` + subscription onMessageAdded { + messageAdded { + id + content + } + } + `; + + expect(() => { + compileOperationsToProto(operation, subscriptionSchema); + }).toThrow(/Operation name "onMessageAdded" must be in PascalCase/); + }); + }); + describe('Root-Level Field Aliases', () => { test('should reject root-level field aliases', () => { const operation = ` From 4e0e7697957ebba9d84eb80bdd14232e088fe3e6 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Sat, 29 Nov 2025 11:02:49 +0000 Subject: [PATCH 02/12] refactor and addressing PR review comments --- protographic/src/operation-to-proto.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/protographic/src/operation-to-proto.ts b/protographic/src/operation-to-proto.ts index 3bb70b1b16..30913e4604 100644 --- a/protographic/src/operation-to-proto.ts +++ b/protographic/src/operation-to-proto.ts @@ -268,10 +268,12 @@ class OperationsToProtoVisitor { // This ensures exact matching between GraphQL operation names and RPC method names // PascalCase: starts with uppercase, contains at least one lowercase letter if (!/^[A-Z](?=.*[a-z])[a-zA-Z0-9]*$/.test(operationName)) { + const suggestedName = upperFirst(camelCase(operationName)); throw new Error( `Operation name "${operationName}" must be in PascalCase ` + `(start with uppercase letter, followed by mixed-case letters/numbers). ` + `Examples: GetUser, CreatePost, OnMessageAdded. ` + + `Suggested name: "${suggestedName}". ` + `This ensures the RPC method name exactly matches the GraphQL operation name.`, ); } From fc601d617fa8264d91db94b501444e487d989444 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Tue, 16 Dec 2025 18:38:51 +0000 Subject: [PATCH 03/12] docs: clarify proto generation constraints and improve ConnectRPC logging --- cli/src/commands/grpc-service/commands/generate.ts | 2 +- protographic/OPERATIONS_TO_PROTO.md | 4 ++-- protographic/src/operation-to-proto.ts | 7 ++++--- protographic/tests/operations/operation-validation.test.ts | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index 66b38cc7ef..c7d73ffe70 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -344,7 +344,7 @@ async function generateFromOperations( // Load lock data for field number stability let currentLockData = await fetchLockData(lockFile); - // Process each operation file separately to maintain reversibility + // Process each operation file separately to maintain proto schema consistency // Collect the AST roots instead of proto strings const roots: protobuf.Root[] = []; diff --git a/protographic/OPERATIONS_TO_PROTO.md b/protographic/OPERATIONS_TO_PROTO.md index ae850b016b..9951b6f943 100644 --- a/protographic/OPERATIONS_TO_PROTO.md +++ b/protographic/OPERATIONS_TO_PROTO.md @@ -716,6 +716,6 @@ Field number conflict in message X ## Limitations - **Named operations only**: All operations must have a name. Anonymous operations are not supported -- **Single operation per document**: Multiple operations in one document are not supported for proto reversibility -- **No root-level field aliases**: Field aliases at the root level break proto-to-GraphQL reversibility +- **Single operation per document**: Multiple operations in one document are not supported for deterministic proto schema generation +- **No root-level field aliases**: Field aliases at the root level break proto schema generation consistency (each GraphQL field must map to exactly one proto field name) - **Alpha status**: The API may change in future releases diff --git a/protographic/src/operation-to-proto.ts b/protographic/src/operation-to-proto.ts index 30913e4604..4a0f57ff22 100644 --- a/protographic/src/operation-to-proto.ts +++ b/protographic/src/operation-to-proto.ts @@ -104,7 +104,7 @@ export function compileOperationsToProto( const operationNames = namedOperations.map((op) => (op as OperationDefinitionNode).name!.value).join(', '); throw new Error( `Multiple operations found in document: ${operationNames}. ` + - 'Only a single named operation per document is supported for proto reversibility. ' + + 'Only a single named operation per document is supported for deterministic proto schema generation. ' + 'Please compile each operation separately.', ); } @@ -278,13 +278,14 @@ class OperationsToProtoVisitor { ); } - // 3. Validate no root-level field aliases (breaks reversibility) + // 3. Validate no root-level field aliases (breaks proto schema consistency) if (node.selectionSet) { for (const selection of node.selectionSet.selections) { if (selection.kind === 'Field' && selection.alias) { throw new Error( `Root-level field alias "${selection.alias.value}: ${selection.name.value}" is not supported. ` + - 'Field aliases at the root level break proto-to-GraphQL reversibility. ' + + 'Field aliases at the root level break proto schema generation consistency. ' + + 'Each GraphQL field must map to exactly one proto field name. ' + 'Please remove the alias or use it only on nested fields.', ); } diff --git a/protographic/tests/operations/operation-validation.test.ts b/protographic/tests/operations/operation-validation.test.ts index 1edb2ce377..d29612ff7c 100644 --- a/protographic/tests/operations/operation-validation.test.ts +++ b/protographic/tests/operations/operation-validation.test.ts @@ -178,8 +178,8 @@ describe('Operation Validation', () => { }); }); - describe('Reversibility Considerations', () => { - test('should allow single operation for proto-to-graphql reversibility', () => { + describe('Proto Schema Consistency', () => { + test('should allow single operation for deterministic proto schema generation', () => { const operation = ` query GetUser { user { From f931efd1248b34de979c829c7cfc57a01da37186 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Tue, 30 Dec 2025 10:00:03 +0100 Subject: [PATCH 04/12] feat(protographic): support GraphQL variable name field options Add wg.connectrpc.graphql_variable_name field option to map proto fields to non-standard GraphQL variable names. Auto-imports annotations.proto and generates proper field option syntax in proto output. --- protographic/src/naming-conventions.ts | 8 +++ .../src/operations/proto-field-options.ts | 19 ++++++ .../src/operations/proto-text-generator.ts | 64 ++++++++++++++++++- .../src/operations/request-builder.ts | 14 ++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 protographic/src/operations/proto-field-options.ts diff --git a/protographic/src/naming-conventions.ts b/protographic/src/naming-conventions.ts index 1faf41942f..5715449a5d 100644 --- a/protographic/src/naming-conventions.ts +++ b/protographic/src/naming-conventions.ts @@ -26,6 +26,14 @@ export function graphqlArgumentToProtoField(argName: string): string { return snakeCase(argName); } +/** + * Converts a proto field name (snake_case) to the expected protobuf JSON format (camelCase) + * This matches how protobuf JSON marshaling automatically converts field names + */ +export function protoFieldToProtoJSON(protoFieldName: string): string { + return camelCase(protoFieldName); +} + /** * Creates an operation method name from an operation type and field name */ diff --git a/protographic/src/operations/proto-field-options.ts b/protographic/src/operations/proto-field-options.ts new file mode 100644 index 0000000000..ea95bd1aa8 --- /dev/null +++ b/protographic/src/operations/proto-field-options.ts @@ -0,0 +1,19 @@ +/** + * Protocol Buffer field option constants for ConnectRPC integration. + * + * See proto/com/wundergraph/connectrpc/options/v1/annotations.proto for full documentation. + */ + +export interface ProtoFieldOption { + readonly fieldNumber: number; + readonly optionName: string; +} + +/** + * Maps protobuf fields to GraphQL variable names when they don't match the expected format. + * Field number 50001 is in the user-defined extension range. + */ +export const GRAPHQL_VARIABLE_NAME: ProtoFieldOption = { + fieldNumber: 50001, + optionName: '(wg.connectrpc.graphql_variable_name)', +} as const; diff --git a/protographic/src/operations/proto-text-generator.ts b/protographic/src/operations/proto-text-generator.ts index c8f245a706..ef8f578757 100644 --- a/protographic/src/operations/proto-text-generator.ts +++ b/protographic/src/operations/proto-text-generator.ts @@ -94,6 +94,11 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string // Imports const imports = new Set(); + // Check if any field uses graphql_variable_name option + if (detectGraphQLVariableNameUsage(root)) { + imports.add('com/wundergraph/connectrpc/options/v1/annotations.proto'); + } + // Only add wrapper types import if actually used if (detectWrapperTypeUsage(root)) { imports.add('google/protobuf/wrappers.proto'); @@ -112,6 +117,9 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string lines.push(''); } + // Extension is now imported from com/wundergraph/connectrpc/options/v1/annotations.proto + // No need to define it inline + // Options - use shared utility for standard options const protoOptions: string[] = buildProtoOptions( { @@ -285,7 +293,24 @@ export function formatField(field: protobuf.Field, options?: ProtoTextOptions, i // Build field line const repeated = field.repeated ? 'repeated ' : ''; - lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id};`)); + + // Check if field has options + if (field.options && Object.keys(field.options).length > 0) { + // Field with options - format with brackets + const optionsStr = Object.entries(field.options) + .map(([key, value]) => { + // The key already includes parentheses if it's an extension option + // e.g., "(cosmo.connectrpc.graphql_variable_name)" + // Handle string values with quotes + const formattedValue = typeof value === 'string' ? `"${value}"` : value; + return `${key} = ${formattedValue}`; + }) + .join(', '); + lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id} [${optionsStr}];`)); + } else { + // Field without options + lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id};`)); + } return lines; } @@ -419,6 +444,43 @@ function messageUsesWrapperTypes(message: protobuf.Type): boolean { return false; } +/** + * Detects if any field in the root uses the graphql_variable_name option + */ +function detectGraphQLVariableNameUsage(root: protobuf.Root): boolean { + for (const nested of root.nestedArray) { + if (nested instanceof protobuf.Type) { + if (messageUsesGraphQLVariableName(nested)) { + return true; + } + } + } + return false; +} + +/** + * Recursively checks if a message or its nested messages use graphql_variable_name option + */ +function messageUsesGraphQLVariableName(message: protobuf.Type): boolean { + // Check fields in this message + for (const field of message.fieldsArray) { + if (field.options && field.options['(graphql_variable_name)']) { + return true; + } + } + + // Check nested messages recursively + for (const nested of message.nestedArray) { + if (nested instanceof protobuf.Type) { + if (messageUsesGraphQLVariableName(nested)) { + return true; + } + } + } + + return false; +} + /** * Helper to format method definitions for services */ diff --git a/protographic/src/operations/request-builder.ts b/protographic/src/operations/request-builder.ts index 7e5983230a..e5186755bf 100644 --- a/protographic/src/operations/request-builder.ts +++ b/protographic/src/operations/request-builder.ts @@ -18,7 +18,9 @@ import { graphqlArgumentToProtoField, createEnumUnspecifiedValue, graphqlEnumValueToProtoEnumValue, + protoFieldToProtoJSON, } from '../naming-conventions.js'; +import { GRAPHQL_VARIABLE_NAME } from './proto-field-options.js'; /** * Options for building request messages @@ -155,6 +157,18 @@ export function buildVariableField( field.repeated = true; } + // Add wundergraph.connectrpc.graphql_variable_name option if the GraphQL variable name doesn't match + // the expected protobuf JSON format (camelCase of snake_case field name) + const expectedProtoJSON = protoFieldToProtoJSON(protoFieldName); + if (variableName !== expectedProtoJSON) { + // Store the GraphQL variable name as a custom option + // This will be used by the handler to map proto JSON to GraphQL variables + if (!field.options) { + field.options = {}; + } + field.options[GRAPHQL_VARIABLE_NAME.optionName] = variableName; + } + return field; } From 272c25d85cc53cdf59060e72b70b77259b107269 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Tue, 30 Dec 2025 11:48:20 +0100 Subject: [PATCH 05/12] fix(protographic): track nested messages independently in lock file Use full dot-notation paths to prevent field number conflicts between nested messages with the same name in different parent contexts. --- .../src/operations/message-builder.ts | 42 +- .../src/operations/proto-field-options.ts | 2 +- .../src/operations/proto-text-generator.ts | 43 +- .../src/operations/request-builder.ts | 12 +- .../tests/operations/fragments.test.ts | 2 +- .../nested-message-field-numbering.test.ts | 502 ++++++++++++++++++ 6 files changed, 569 insertions(+), 34 deletions(-) create mode 100644 protographic/tests/operations/nested-message-field-numbering.test.ts diff --git a/protographic/src/operations/message-builder.ts b/protographic/src/operations/message-builder.ts index a0facb7742..a2660b7471 100644 --- a/protographic/src/operations/message-builder.ts +++ b/protographic/src/operations/message-builder.ts @@ -78,10 +78,15 @@ export function buildMessageFromSelectionSet( parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType, typeInfo: TypeInfo, options?: MessageBuilderOptions, + parentPath?: string, ): protobuf.Type { const message = new protobuf.Type(messageName); const fieldNumberManager = options?.fieldNumberManager; + // Build the full path for lock file tracking (e.g., "ParentMessage.ChildMessage") + // This is used for field number management to prevent collisions between nested messages + const fullPath = parentPath ? `${parentPath}.${messageName}` : messageName; + // First pass: collect all field names that will be in this message const fieldNames: string[] = []; const fieldSelections = new Map(); @@ -168,14 +173,15 @@ export function buildMessageFromSelectionSet( collectFields(selectionSet.selections, parentType, currentDepth); // Reconcile field order using lock manager if available + // Use fullPath for lock file operations to track nested message hierarchy let orderedFieldNames = fieldNames; if (fieldNumberManager && 'reconcileFieldOrder' in fieldNumberManager) { - orderedFieldNames = fieldNumberManager.reconcileFieldOrder(messageName, fieldNames); + orderedFieldNames = fieldNumberManager.reconcileFieldOrder(fullPath, fieldNames); } // Second pass: process fields in reconciled order // Pre-assign field numbers from lock data if available - assignFieldNumbersFromLockData(messageName, orderedFieldNames, fieldNumberManager); + assignFieldNumbersFromLockData(fullPath, orderedFieldNames, fieldNumberManager); for (const protoFieldName of orderedFieldNames) { const fieldData = fieldSelections.get(protoFieldName); @@ -184,7 +190,15 @@ export function buildMessageFromSelectionSet( ...options, _depth: currentDepth, }; - processFieldSelection(fieldData.selection, message, fieldData.type, typeInfo, fieldOptions, fieldNumberManager); + processFieldSelection( + fieldData.selection, + message, + fieldData.type, + typeInfo, + fieldOptions, + fieldNumberManager, + fullPath, + ); } } @@ -195,23 +209,24 @@ export function buildMessageFromSelectionSet( * Gets or assigns a field number for a proto field */ function getOrAssignFieldNumber( - message: protobuf.Type, + messagePath: string, protoFieldName: string, fieldNumberManager?: FieldNumberManager, + message?: protobuf.Type, ): number { - const existingFieldNumber = fieldNumberManager?.getFieldNumber(message.name, protoFieldName); + const existingFieldNumber = fieldNumberManager?.getFieldNumber(messagePath, protoFieldName); if (existingFieldNumber !== undefined) { return existingFieldNumber; } if (fieldNumberManager) { - const fieldNumber = fieldNumberManager.getNextFieldNumber(message.name); - fieldNumberManager.assignFieldNumber(message.name, protoFieldName, fieldNumber); + const fieldNumber = fieldNumberManager.getNextFieldNumber(messagePath); + fieldNumberManager.assignFieldNumber(messagePath, protoFieldName, fieldNumber); return fieldNumber; } - return message.fieldsArray.length + 1; + return message ? message.fieldsArray.length + 1 : 1; } /** @@ -292,6 +307,7 @@ function processFieldSelection( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { const fieldName = field.name.value; @@ -332,6 +348,8 @@ function processFieldSelection( // Build nested message for object types const namedType = getNamedType(fieldType); if (isObjectType(namedType) || isInterfaceType(namedType) || isUnionType(namedType)) { + // Use clean nested message name (just the field name in PascalCase) + // The full path will be tracked separately for lock file operations const nestedMessageName = upperFirst(camelCase(fieldName)); const nestedOptions = { @@ -339,12 +357,16 @@ function processFieldSelection( _depth: (options?._depth ?? 0) + 1, }; + // Pass the current message path to build the full hierarchy for lock file tracking + const currentPath = messagePath || message.name; + const nestedMessage = buildMessageFromSelectionSet( nestedMessageName, field.selectionSet, namedType, typeInfo, nestedOptions, + currentPath, // Pass parent path for nested message tracking ); message.add(nestedMessage); @@ -373,7 +395,9 @@ function processFieldSelection( const { typeName, isRepeated } = resolveTypeNameAndRepetition(baseTypeName, protoTypeInfo, fieldType, options); - const fieldNumber = getOrAssignFieldNumber(message, protoFieldName, fieldNumberManager); + // Use the full message path for field number assignment to prevent collisions + const currentPath = messagePath || message.name; + const fieldNumber = getOrAssignFieldNumber(currentPath, protoFieldName, fieldNumberManager, message); const protoField = createProtoField(protoFieldName, fieldNumber, typeName, isRepeated, fieldDef, options); diff --git a/protographic/src/operations/proto-field-options.ts b/protographic/src/operations/proto-field-options.ts index ea95bd1aa8..eaa2a436f2 100644 --- a/protographic/src/operations/proto-field-options.ts +++ b/protographic/src/operations/proto-field-options.ts @@ -1,6 +1,6 @@ /** * Protocol Buffer field option constants for ConnectRPC integration. - * + * * See proto/com/wundergraph/connectrpc/options/v1/annotations.proto for full documentation. */ diff --git a/protographic/src/operations/proto-text-generator.ts b/protographic/src/operations/proto-text-generator.ts index ef8f578757..1c7b143bfc 100644 --- a/protographic/src/operations/proto-text-generator.ts +++ b/protographic/src/operations/proto-text-generator.ts @@ -1,6 +1,7 @@ import protobuf from 'protobufjs'; import { buildProtoOptions } from '../proto-options.js'; import { MethodWithIdempotency } from '../types.js'; +import { GRAPHQL_VARIABLE_NAME } from './proto-field-options.js'; /** * Helper to format indentation @@ -95,8 +96,10 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string const imports = new Set(); // Check if any field uses graphql_variable_name option - if (detectGraphQLVariableNameUsage(root)) { - imports.add('com/wundergraph/connectrpc/options/v1/annotations.proto'); + const usesGraphQLVariableName = detectGraphQLVariableNameUsage(root); + if (usesGraphQLVariableName) { + // Import descriptor.proto for field option extension + imports.add('google/protobuf/descriptor.proto'); } // Only add wrapper types import if actually used @@ -117,8 +120,14 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string lines.push(''); } - // Extension is now imported from com/wundergraph/connectrpc/options/v1/annotations.proto - // No need to define it inline + // Add field option extension if used + if (usesGraphQLVariableName) { + lines.push('// Field option extension for GraphQL variable name mapping'); + lines.push('extend google.protobuf.FieldOptions {'); + lines.push(' string wg.connectrpc.graphql_variable_name = 50001;'); + lines.push('}'); + lines.push(''); + } // Options - use shared utility for standard options const protoOptions: string[] = buildProtoOptions( @@ -293,20 +302,20 @@ export function formatField(field: protobuf.Field, options?: ProtoTextOptions, i // Build field line const repeated = field.repeated ? 'repeated ' : ''; - + // Check if field has options if (field.options && Object.keys(field.options).length > 0) { - // Field with options - format with brackets - const optionsStr = Object.entries(field.options) - .map(([key, value]) => { - // The key already includes parentheses if it's an extension option - // e.g., "(cosmo.connectrpc.graphql_variable_name)" - // Handle string values with quotes - const formattedValue = typeof value === 'string' ? `"${value}"` : value; - return `${key} = ${formattedValue}`; - }) - .join(', '); - lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id} [${optionsStr}];`)); + // Field with options - format with brackets + const optionsStr = Object.entries(field.options) + .map(([key, value]) => { + // The key already includes parentheses if it's an extension option + // e.g., "(cosmo.connectrpc.graphql_variable_name)" + // Handle string values with quotes + const formattedValue = typeof value === 'string' ? `"${value}"` : value; + return `${key} = ${formattedValue}`; + }) + .join(', '); + lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id} [${optionsStr}];`)); } else { // Field without options lines.push(formatIndent(indent, `${repeated}${field.type} ${field.name} = ${field.id};`)); @@ -464,7 +473,7 @@ function detectGraphQLVariableNameUsage(root: protobuf.Root): boolean { function messageUsesGraphQLVariableName(message: protobuf.Type): boolean { // Check fields in this message for (const field of message.fieldsArray) { - if (field.options && field.options['(graphql_variable_name)']) { + if (field.options && field.options[GRAPHQL_VARIABLE_NAME.optionName]) { return true; } } diff --git a/protographic/src/operations/request-builder.ts b/protographic/src/operations/request-builder.ts index e5186755bf..22d5b84337 100644 --- a/protographic/src/operations/request-builder.ts +++ b/protographic/src/operations/request-builder.ts @@ -161,12 +161,12 @@ export function buildVariableField( // the expected protobuf JSON format (camelCase of snake_case field name) const expectedProtoJSON = protoFieldToProtoJSON(protoFieldName); if (variableName !== expectedProtoJSON) { - // Store the GraphQL variable name as a custom option - // This will be used by the handler to map proto JSON to GraphQL variables - if (!field.options) { - field.options = {}; - } - field.options[GRAPHQL_VARIABLE_NAME.optionName] = variableName; + // Store the GraphQL variable name as a custom option + // This will be used by the handler to map proto JSON to GraphQL variables + if (!field.options) { + field.options = {}; + } + field.options[GRAPHQL_VARIABLE_NAME.optionName] = variableName; } return field; diff --git a/protographic/tests/operations/fragments.test.ts b/protographic/tests/operations/fragments.test.ts index 26b388b461..d742496222 100644 --- a/protographic/tests/operations/fragments.test.ts +++ b/protographic/tests/operations/fragments.test.ts @@ -1263,7 +1263,7 @@ describe('Fragment Support', () => { message Child { message Child { string id = 1; - google.protobuf.StringValue value = 4; + google.protobuf.StringValue value = 2; } string id = 1; google.protobuf.StringValue name = 2; diff --git a/protographic/tests/operations/nested-message-field-numbering.test.ts b/protographic/tests/operations/nested-message-field-numbering.test.ts new file mode 100644 index 0000000000..6b7302d8d6 --- /dev/null +++ b/protographic/tests/operations/nested-message-field-numbering.test.ts @@ -0,0 +1,502 @@ +import { describe, expect, test } from 'vitest'; +import { compileOperationsToProto } from '../../src'; +import { expectValidProto } from '../util'; + +describe('Nested Message Field Numbering', () => { + test('should assign field numbers starting from 1 in a simple nested message', () => { + const schema = ` + type Query { + getEmployee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + forename: String + surname: String + } + `; + + const operation = ` + query GetEmployee { + getEmployee { + details { + forename + surname + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Details { + google.protobuf.StringValue forename = 1; + google.protobuf.StringValue surname = 2; + } + Details details = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify lock file uses dot notation for nested message path + expect(lockData).toBeDefined(); + expect(lockData!.messages['GetEmployeeResponse.GetEmployee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "forename": 1, + "surname": 2, + }, + } + `); + }); + + test('should not share field number counters between sibling nested messages with same name', () => { + const schema = ` + type Query { + getData: GetData + } + + type GetData { + userInfo: UserInfo + productInfo: ProductInfo + } + + type UserInfo { + details: UserDetails + } + + type UserDetails { + name: String + email: String + } + + type ProductInfo { + details: ProductDetails + } + + type ProductDetails { + title: String + price: Float + } + `; + + const operation = ` + query GetData { + getData { + userInfo { + details { + name + email + } + } + productInfo { + details { + title + price + } + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetData(GetDataRequest) returns (GetDataResponse) {} + } + + message GetDataRequest { + } + + message GetDataResponse { + message GetData { + message UserInfo { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue email = 2; + } + Details details = 1; + } + message ProductInfo { + message Details { + google.protobuf.StringValue title = 1; + google.protobuf.DoubleValue price = 2; + } + Details details = 1; + } + UserInfo user_info = 1; + ProductInfo product_info = 2; + } + GetData get_data = 1; + } + " + `); + + // Verify both Details messages have independent field numbering in lock file + // Each starts from 1 because they're tracked separately using full dot-notation paths + expect(lockData!.messages['GetDataResponse.GetData.UserInfo.Details']).toMatchInlineSnapshot(` + { + "fields": { + "email": 2, + "name": 1, + }, + } + `); + expect(lockData!.messages['GetDataResponse.GetData.ProductInfo.Details']).toMatchInlineSnapshot(` + { + "fields": { + "price": 2, + "title": 1, + }, + } + `); + }); + + test('should preserve existing field numbers when adding new fields to nested messages', () => { + const schema1 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + } + `; + + const operation1 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + } + } + } + } + `; + + const result1 = compileOperationsToProto(operation1, schema1); + + // Now add a field to the nested Details message + const schema2 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + email: String + } + `; + + const operation2 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + email + } + } + } + } + `; + + const result2 = compileOperationsToProto(operation2, schema2, { lockData: result1.lockData }); + + expectValidProto(result2.proto); + + expect(result2.proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Employee { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue email = 2; + } + Details details = 1; + } + Employee employee = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify field numbers are stable: name keeps 1, email gets 2 + expect(result2.lockData!.messages['GetEmployeeResponse.GetEmployee.Employee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "email": 2, + "name": 1, + }, + } + `); + }); + + test('should mark removed field numbers as reserved in nested messages', () => { + const schema1 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + email: String + phone: String + } + `; + + const operation1 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + email + phone + } + } + } + } + `; + + const result1 = compileOperationsToProto(operation1, schema1); + + // Remove the email field + const schema2 = ` + type Query { + getEmployee: GetEmployee + } + + type GetEmployee { + employee: Employee + } + + type Employee { + details: EmployeeDetails + } + + type EmployeeDetails { + name: String + phone: String + } + `; + + const operation2 = ` + query GetEmployee { + getEmployee { + employee { + details { + name + phone + } + } + } + } + `; + + const result2 = compileOperationsToProto(operation2, schema2, { lockData: result1.lockData }); + + expectValidProto(result2.proto); + + expect(result2.proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetEmployee(GetEmployeeRequest) returns (GetEmployeeResponse) {} + } + + message GetEmployeeRequest { + } + + message GetEmployeeResponse { + message GetEmployee { + message Employee { + message Details { + google.protobuf.StringValue name = 1; + google.protobuf.StringValue phone = 3; + } + Details details = 1; + } + Employee employee = 1; + } + GetEmployee get_employee = 1; + } + " + `); + + // Verify field number 2 (email) is reserved for backward compatibility + expect(result2.lockData!.messages['GetEmployeeResponse.GetEmployee.Employee.Details']).toMatchInlineSnapshot(` + { + "fields": { + "name": 1, + "phone": 3, + }, + "reservedNumbers": [ + 2, + ], + } + `); + }); + + test('should correctly track field numbers in deeply nested message hierarchies (5+ levels)', () => { + const schema = ` + type Query { + getDeep: Level1 + } + + type Level1 { + level2: Level2 + } + + type Level2 { + level3: Level3 + } + + type Level3 { + level4: Level4 + } + + type Level4 { + level5: Level5 + } + + type Level5 { + value: String + } + `; + + const operation = ` + query GetDeep { + getDeep { + level2 { + level3 { + level4 { + level5 { + value + } + } + } + } + } + } + `; + + const { proto, lockData } = compileOperationsToProto(operation, schema); + + expectValidProto(proto); + + expect(proto).toMatchInlineSnapshot(` + "syntax = "proto3"; + package service.v1; + + import "google/protobuf/wrappers.proto"; + + service DefaultService { + rpc GetDeep(GetDeepRequest) returns (GetDeepResponse) {} + } + + message GetDeepRequest { + } + + message GetDeepResponse { + message GetDeep { + message Level2 { + message Level3 { + message Level4 { + message Level5 { + google.protobuf.StringValue value = 1; + } + Level5 level_5 = 1; + } + Level4 level_4 = 1; + } + Level3 level_3 = 1; + } + Level2 level_2 = 1; + } + GetDeep get_deep = 1; + } + " + `); + + // Verify the deepest nested message uses full dot-notation path in lock file + expect(lockData!.messages['GetDeepResponse.GetDeep.Level2.Level3.Level4.Level5']).toMatchInlineSnapshot(` + { + "fields": { + "value": 1, + }, + } + `); + }); +}); From ffbed71afb82057bd56543c60b567b176aae49cf Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Tue, 30 Dec 2025 12:44:57 +0100 Subject: [PATCH 06/12] fix: as field option extension is embedded, we cannot provide fully qualified name --- protographic/src/operations/proto-field-options.ts | 2 +- protographic/src/operations/proto-text-generator.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/protographic/src/operations/proto-field-options.ts b/protographic/src/operations/proto-field-options.ts index eaa2a436f2..5c66a5a70f 100644 --- a/protographic/src/operations/proto-field-options.ts +++ b/protographic/src/operations/proto-field-options.ts @@ -15,5 +15,5 @@ export interface ProtoFieldOption { */ export const GRAPHQL_VARIABLE_NAME: ProtoFieldOption = { fieldNumber: 50001, - optionName: '(wg.connectrpc.graphql_variable_name)', + optionName: '(graphql_variable_name)', } as const; diff --git a/protographic/src/operations/proto-text-generator.ts b/protographic/src/operations/proto-text-generator.ts index 1c7b143bfc..efbda271e3 100644 --- a/protographic/src/operations/proto-text-generator.ts +++ b/protographic/src/operations/proto-text-generator.ts @@ -124,7 +124,7 @@ function generateHeader(root: protobuf.Root, options?: ProtoTextOptions): string if (usesGraphQLVariableName) { lines.push('// Field option extension for GraphQL variable name mapping'); lines.push('extend google.protobuf.FieldOptions {'); - lines.push(' string wg.connectrpc.graphql_variable_name = 50001;'); + lines.push(' string graphql_variable_name = 50001;'); lines.push('}'); lines.push(''); } From 38c946d6a7e59707e1607afcf2c10f006a31d9fe Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Tue, 30 Dec 2025 12:59:31 +0100 Subject: [PATCH 07/12] fix: correctly looking up graphql_name from extension options --- protographic/src/operations/proto-text-generator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protographic/src/operations/proto-text-generator.ts b/protographic/src/operations/proto-text-generator.ts index efbda271e3..02c58c698a 100644 --- a/protographic/src/operations/proto-text-generator.ts +++ b/protographic/src/operations/proto-text-generator.ts @@ -309,7 +309,7 @@ export function formatField(field: protobuf.Field, options?: ProtoTextOptions, i const optionsStr = Object.entries(field.options) .map(([key, value]) => { // The key already includes parentheses if it's an extension option - // e.g., "(cosmo.connectrpc.graphql_variable_name)" + // e.g., "(graphql_variable_name)" // Handle string values with quotes const formattedValue = typeof value === 'string' ? `"${value}"` : value; return `${key} = ${formattedValue}`; From c5dc3fba24db3ad202c5041a8ae85ab203105490 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Wed, 31 Dec 2025 16:53:32 +0100 Subject: [PATCH 08/12] refactor: allow all-uppercase operation names Relaxes validation to accept names like GETUSER, HRService, APIHandler. Operation names must start with uppercase letter and contain only letters/numbers. --- protographic/src/operation-to-proto.ts | 16 ++++++++-------- .../operations/operation-validation.test.ts | 18 +++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/protographic/src/operation-to-proto.ts b/protographic/src/operation-to-proto.ts index 4a0f57ff22..cec2f5efeb 100644 --- a/protographic/src/operation-to-proto.ts +++ b/protographic/src/operation-to-proto.ts @@ -264,17 +264,17 @@ class OperationsToProtoVisitor { return; } - // 2. Validate operation name is PascalCase - // This ensures exact matching between GraphQL operation names and RPC method names - // PascalCase: starts with uppercase, contains at least one lowercase letter - if (!/^[A-Z](?=.*[a-z])[a-zA-Z0-9]*$/.test(operationName)) { + // 2. Validate operation name starts with uppercase letter + // This follows protobuf RPC naming conventions while allowing flexibility + // Must start with uppercase letter, followed by letters/numbers + if (!/^[A-Z][a-zA-Z0-9]*$/.test(operationName)) { const suggestedName = upperFirst(camelCase(operationName)); throw new Error( - `Operation name "${operationName}" must be in PascalCase ` + - `(start with uppercase letter, followed by mixed-case letters/numbers). ` + - `Examples: GetUser, CreatePost, OnMessageAdded. ` + + `Operation name "${operationName}" must start with an uppercase letter ` + + `and contain only letters and numbers. ` + + `Examples: GetUser, CreatePost, HRService, GETUSER. ` + `Suggested name: "${suggestedName}". ` + - `This ensures the RPC method name exactly matches the GraphQL operation name.`, + `This follows protobuf RPC naming conventions.`, ); } diff --git a/protographic/tests/operations/operation-validation.test.ts b/protographic/tests/operations/operation-validation.test.ts index d29612ff7c..004882ab85 100644 --- a/protographic/tests/operations/operation-validation.test.ts +++ b/protographic/tests/operations/operation-validation.test.ts @@ -241,7 +241,7 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, schema); - }).toThrow(/Operation name "getUser" must be in PascalCase/); + }).toThrow(/Operation name "getUser" must start with an uppercase letter/); }); test('should reject snake_case operation names', () => { @@ -256,10 +256,10 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, schema); - }).toThrow(/Operation name "get_user" must be in PascalCase/); + }).toThrow(/Operation name "get_user" must start with an uppercase letter/); }); - test('should reject all-UPPERCASE operation names', () => { + test('should accept all-UPPERCASE operation names', () => { const operation = ` query GETUSER { user { @@ -271,10 +271,10 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, schema); - }).toThrow(/Operation name "GETUSER" must be in PascalCase/); + }).not.toThrow(); }); - test('should reject operation names with only uppercase and numbers', () => { + test('should accept operation names with only uppercase and numbers', () => { const operation = ` query GET123USER { user { @@ -286,7 +286,7 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, schema); - }).toThrow(/Operation name "GET123USER" must be in PascalCase/); + }).not.toThrow(); }); test('should provide helpful error message for camelCase', () => { @@ -300,7 +300,7 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, schema); - }).toThrow(/must be in PascalCase.*Examples: GetUser, CreatePost, OnMessageAdded/); + }).toThrow(/must start with an uppercase letter.*Examples: GetUser, CreatePost, HRService, GETUSER/); }); test('should validate mutation operation names', () => { @@ -326,7 +326,7 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, mutationSchema); - }).toThrow(/Operation name "createUser" must be in PascalCase/); + }).toThrow(/Operation name "createUser" must start with an uppercase letter/); }); test('should validate subscription operation names', () => { @@ -356,7 +356,7 @@ describe('Operation Validation', () => { expect(() => { compileOperationsToProto(operation, subscriptionSchema); - }).toThrow(/Operation name "onMessageAdded" must be in PascalCase/); + }).toThrow(/Operation name "onMessageAdded" must start with an uppercase letter/); }); }); From 7ebd229fb2e96d188231930e0f9796d43101bdb0 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Wed, 31 Dec 2025 17:07:49 +0100 Subject: [PATCH 09/12] fix(protographic): propagate messagePath through fragment processing Ensures that messagePath parameter is properly propagated through processInlineFragment and processFragmentSpread functions to maintain correct field number tracking for nested messages created within fragments. This fixes an issue where nested messages in fragments would lose their path context, potentially causing field number collisions in the proto lock file mechanism. --- protographic/src/operations/message-builder.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/protographic/src/operations/message-builder.ts b/protographic/src/operations/message-builder.ts index a2660b7471..262f6ca73b 100644 --- a/protographic/src/operations/message-builder.ts +++ b/protographic/src/operations/message-builder.ts @@ -415,6 +415,7 @@ function processInlineFragment( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { // Determine the type for this inline fragment let fragmentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType; @@ -445,12 +446,12 @@ function processInlineFragment( if (fragment.selectionSet) { for (const selection of fragment.selectionSet.selections) { if (selection.kind === 'Field') { - processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'InlineFragment') { // Nested inline fragment - processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'FragmentSpread') { - processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager); + processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath); } } } @@ -467,6 +468,7 @@ function processFragmentSpread( typeInfo: TypeInfo, options?: MessageBuilderOptions, fieldNumberManager?: FieldNumberManager, + messagePath?: string, ): void { const fragmentName = spread.name.value; const fragments = options?.fragments; @@ -500,12 +502,12 @@ function processFragmentSpread( // Process the fragment's selection set with the resolved type for (const selection of fragmentDef.selectionSet.selections) { if (selection.kind === 'Field') { - processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager); + processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'InlineFragment') { - processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager); + processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } else if (selection.kind === 'FragmentSpread') { // Nested fragment spread (fragment inside fragment) - processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager); + processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager, messagePath); } } } From afe5d9486ffec4d1fc6d2ee836700a738834bca4 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Wed, 31 Dec 2025 17:20:06 +0100 Subject: [PATCH 10/12] fix(cli): preserve acronyms in service names for operations-based generation Use service name as-is when generating proto from operations to preserve acronyms like HRService, APIService, etc. --- cli/src/commands/grpc-service/commands/generate.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index c7d73ffe70..92ae6fff99 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -203,7 +203,7 @@ async function generateCommandAction(name: string, options: CLIOptions) { const resultInfo: Record = { 'input file': inputFile, 'output dir': options.output, - 'service name': upperFirst(camelCase(name)), + 'service name': result.isOperationsMode ? name : upperFirst(camelCase(name)), 'generation mode': result.isOperationsMode ? 'operations-based' : 'SDL-based', generated: generatedFiles.join(', '), }; @@ -481,7 +481,6 @@ async function generateProtoAndMapping({ maxDepth, }: GenerationOptions): Promise { const schema = await readFile(schemaFile, 'utf8'); - const serviceName = upperFirst(camelCase(name)); // Validate the GraphQL schema spinner.text = 'Validating GraphQL schema...'; @@ -490,6 +489,9 @@ async function generateProtoAndMapping({ // Determine generation mode if (operationsDir) { + // For operations-based generation, use the service name as-is to preserve + // user-provided formatting including acronyms (e.g., HRService, APIService) + const serviceName = name; const operationsPath = resolve(operationsDir); return generateFromOperations( schema, @@ -503,6 +505,8 @@ async function generateProtoAndMapping({ maxDepth, ); } else { + // For SDL-based generation, apply transformation to ensure proper casing + const serviceName = upperFirst(camelCase(name)); return generateFromSDL(schema, serviceName, spinner, packageName, languageOptions, lockFile); } } From 90ffd0db198195d1d281a73261f19464303276dd Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Wed, 31 Dec 2025 18:49:01 +0100 Subject: [PATCH 11/12] Revert "fix(cli): preserve acronyms in service names for operations-based generation" This reverts commit afe5d9486ffec4d1fc6d2ee836700a738834bca4. --- cli/src/commands/grpc-service/commands/generate.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index 92ae6fff99..c7d73ffe70 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -203,7 +203,7 @@ async function generateCommandAction(name: string, options: CLIOptions) { const resultInfo: Record = { 'input file': inputFile, 'output dir': options.output, - 'service name': result.isOperationsMode ? name : upperFirst(camelCase(name)), + 'service name': upperFirst(camelCase(name)), 'generation mode': result.isOperationsMode ? 'operations-based' : 'SDL-based', generated: generatedFiles.join(', '), }; @@ -481,6 +481,7 @@ async function generateProtoAndMapping({ maxDepth, }: GenerationOptions): Promise { const schema = await readFile(schemaFile, 'utf8'); + const serviceName = upperFirst(camelCase(name)); // Validate the GraphQL schema spinner.text = 'Validating GraphQL schema...'; @@ -489,9 +490,6 @@ async function generateProtoAndMapping({ // Determine generation mode if (operationsDir) { - // For operations-based generation, use the service name as-is to preserve - // user-provided formatting including acronyms (e.g., HRService, APIService) - const serviceName = name; const operationsPath = resolve(operationsDir); return generateFromOperations( schema, @@ -505,8 +503,6 @@ async function generateProtoAndMapping({ maxDepth, ); } else { - // For SDL-based generation, apply transformation to ensure proper casing - const serviceName = upperFirst(camelCase(name)); return generateFromSDL(schema, serviceName, spinner, packageName, languageOptions, lockFile); } } From 03fa914595932f5fb7605a59b653949ce33633c2 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Thu, 1 Jan 2026 13:22:57 +0100 Subject: [PATCH 12/12] fix: correct proto file reference in proto-field-options.ts comment The comment referenced a non-existent proto file path. Updated to accurately describe that field options are defined using 'extend google.protobuf.FieldOptions' in generated proto files, as seen in the actual implementation. --- cli/src/commands/grpc-service/commands/generate.ts | 4 ++-- protographic/src/operations/proto-field-options.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/src/commands/grpc-service/commands/generate.ts b/cli/src/commands/grpc-service/commands/generate.ts index c7d73ffe70..493e3d219d 100644 --- a/cli/src/commands/grpc-service/commands/generate.ts +++ b/cli/src/commands/grpc-service/commands/generate.ts @@ -344,8 +344,8 @@ async function generateFromOperations( // Load lock data for field number stability let currentLockData = await fetchLockData(lockFile); - // Process each operation file separately to maintain proto schema consistency - // Collect the AST roots instead of proto strings + // Process each operation file separately, updating lock data sequentially for field number stability. + // Collect AST roots for merging rather than proto strings. const roots: protobuf.Root[] = []; for (const { filename, content } of operationFiles) { diff --git a/protographic/src/operations/proto-field-options.ts b/protographic/src/operations/proto-field-options.ts index 5c66a5a70f..89c99a6d4e 100644 --- a/protographic/src/operations/proto-field-options.ts +++ b/protographic/src/operations/proto-field-options.ts @@ -1,7 +1,7 @@ /** * Protocol Buffer field option constants for ConnectRPC integration. * - * See proto/com/wundergraph/connectrpc/options/v1/annotations.proto for full documentation. + * These options are defined using `extend google.protobuf.FieldOptions` in generated proto files. */ export interface ProtoFieldOption {