Skip to content

feat: allow empty arguments for field resolvers#1350

Merged
Noroth merged 2 commits intomasterfrom
ludwig/eng-8592-allow-providing-no-arguments-to-field-resolvers
Dec 8, 2025
Merged

feat: allow empty arguments for field resolvers#1350
Noroth merged 2 commits intomasterfrom
ludwig/eng-8592-allow-providing-no-arguments-to-field-resolvers

Conversation

@Noroth
Copy link
Copy Markdown
Contributor

@Noroth Noroth commented Dec 5, 2025

Introduction

This PR introduces support to call field resolvers without any arguments. Currently we only check if field resolvers provide arguments with parentheses.

Field resolvers were determined whether fields have arguments. This is technically correct but instead of checking field arguments on the operation side, they have to be determined from the schema definition.

Prerequisites

The following problem statement refer to this example schema

type Query {
  categories: [Category!]!
}

type Category {
  id: ID!
  # The argument is nullable and can be omitted
  relevantTopic(orderBy: String): Topic! 
}

type Topic {
  id: ID!
  description: String!
} 

Problem Statement

This will correctly call the field resolver

query {
  categories {
    id
    relevantTopic("") {
      id
      description
    }
  }
}

This did not call the field resolver

query {
  categories {
    id
    relevantTopic {
      id
      description
    }
  }
}

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error handling and validation for field resolution
    • Improved support for optional arguments in field resolvers
    • Better error messages when field definitions cannot be resolved
  • Tests

    • Expanded test coverage for field resolver edge cases

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 5, 2025

Walkthrough

Refactors field resolution in gRPC datasource processing from fieldRef-based to fieldDefRef-based (field definition references) throughout execution plan construction and visitor logic. Adds strict validation for field definitions with error handling, conditional argument allocation in resolver RPC calls, and corresponding test coverage for optional resolver arguments.

Changes

Cohort / File(s) Summary
Execution Plan Field Definition Resolution
v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
Replaces fieldRef with fieldDefRef for field resolution across buildRequiredField, buildFieldMessage, buildCompositeFields, and related routines. Adds field definition lookup with strict validation. Introduces fieldDefinitionRefForType helper. Implements conditional argument allocation in resolver RPC calls. Updates error messages to reference field names via fieldDefRef context.
Visitor Field Resolver Detection
v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go, execution_plan_visitor_federation.go
Updates EnterField to use fieldDefRef for resolver detection instead of raw field ref. Adds field definition resolution in LeaveField with defensive error reporting if definition not found. Changes resolver-check logic to depend on validated fieldDefRef state.
Field Resolver Tests
v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
Adds test case for resolver with optional argument (fooResolverOptionalArgument). Extends GraphQL schema and test mappings to include new optional-argument resolver wiring and RPC routing configuration.
Integration Tests
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go
Adds test case "Query with field resolver without parentheses (nullable parameter)" validating that popularityScore defaults to 50 when threshold parameter is omitted.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Field definition reference propagation across execution_plan.go functions; verify fieldDefRef consistency through buildRequiredField → buildFieldMessage → nested resolution chains
  • Error handling logic in LeaveField of both visitor files; confirm error reporting path when field definitions cannot be resolved
  • Conditional argument allocation in resolver RPC calls; ensure error guards within loops properly prevent partial state corruption
  • fieldDefinitionRefForType helper usage in composite field resolution; validate correctness of field-name-to-definition mapping
  • Test coverage for new optional-argument resolver pathway; verify RPC call sequencing and dependency handling match expected patterns

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: allow empty arguments for field resolvers' directly aligns with the PR's main objective to add support for calling field resolvers without any arguments, as confirmed across all modified files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ludwig/eng-8592-allow-providing-no-arguments-to-field-resolvers

Comment @coderabbitai help to get the list of available commands and usage tips.

@ysmolski
Copy link
Copy Markdown
Contributor

ysmolski commented Dec 8, 2025

started review

Comment thread v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
@Noroth Noroth merged commit 5db8c5a into master Dec 8, 2025
12 checks passed
@Noroth Noroth deleted the ludwig/eng-8592-allow-providing-no-arguments-to-field-resolvers branch December 8, 2025 14:26
Noroth pushed a commit that referenced this pull request Dec 8, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.0.0-rc.240](v2.0.0-rc.239...v2.0.0-rc.240)
(2025-12-08)


### Features

* add composite type support for field resolvers
([#1341](#1341))
([701ea8d](701ea8d))
* allow empty arguments for field resolvers
([#1350](#1350))
([5db8c5a](5db8c5a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added support for composite types in field resolvers
  * Added support for empty arguments in field resolvers

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.

3 participants