feat(router): add router support for field resolvers without arguments#2645
Merged
feat(router): add router support for field resolvers without arguments#2645
Conversation
Contributor
WalkthroughThis PR adds support for field resolvers without arguments to the projects subgraph. It introduces four computed fields (taskCount, activeMilestoneCount, totalProjectCount, featureMatrix) in the GraphQL schema with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
📝 Coding Plan
Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
demo/pkg/subgraphs/projects/src/service/service_resolve.go (1)
637-646: Consider performance for large datasets.The nested iteration (all projects × team members per project × employee contexts) has O(n×m×c) complexity. For each employee context, you iterate through all projects and their team members to count memberships.
For this demo service, the current approach is acceptable. However, if this pattern were to be used in production with large datasets, consider building an inverted index (employee → projects) to achieve O(1) lookups per employee.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo/pkg/subgraphs/projects/src/service/service_resolve.go` around lines 637 - 646, The current triple-nested loops over req.Context, data.ServiceProjects and members (using data.GetTeamMembersByProjectId and ctx.GetId()) are O(n×m×c); instead, build a single inverted map (e.g., empToCount) by iterating once over data.ServiceProjects and their members to increment counts per member ID, then replace the inner scans with O(1) lookups from empToCount for each ctx to set count—update the block around the loop that references req.Context, data.ServiceProjects, data.GetTeamMembersByProjectId and ctx.GetId() to use this map-based approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@router/go.mod`:
- Line 34: The go.mod currently pins a pseudo-version for
github.com/wundergraph/graphql-go-tools/v2 (v2.0.0-rc.262...2cd631249f01);
replace that pseudo-version with the appropriate stable tagged release of
github.com/wundergraph/graphql-go-tools/v2 before merging. Update the dependency
by running a go get for the chosen tag (e.g. go get
github.com/wundergraph/graphql-go-tools/v2@<stable-tag>), then run go mod tidy
to update go.mod and go.sum, and run the relevant tests/build to verify nothing
breaks; ensure the final commit contains the stable semver (not the rc
pseudo-version).
---
Nitpick comments:
In `@demo/pkg/subgraphs/projects/src/service/service_resolve.go`:
- Around line 637-646: The current triple-nested loops over req.Context,
data.ServiceProjects and members (using data.GetTeamMembersByProjectId and
ctx.GetId()) are O(n×m×c); instead, build a single inverted map (e.g.,
empToCount) by iterating once over data.ServiceProjects and their members to
increment counts per member ID, then replace the inner scans with O(1) lookups
from empToCount for each ctx to set count—update the block around the loop that
references req.Context, data.ServiceProjects, data.GetTeamMembersByProjectId and
ctx.GetId() to use this map-based approach.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0fdf1185-5921-4b92-a9be-5c69212d7665
⛔ Files ignored due to path filters (6)
demo/pkg/subgraphs/projects/generated/mapping.jsonis excluded by!**/generated/**demo/pkg/subgraphs/projects/generated/service.pb.gois excluded by!**/*.pb.go,!**/generated/**demo/pkg/subgraphs/projects/generated/service.protois excluded by!**/generated/**demo/pkg/subgraphs/projects/generated/service_grpc.pb.gois excluded by!**/*.pb.go,!**/generated/**router-tests/go.sumis excluded by!**/*.sumrouter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
demo/pkg/subgraphs/projects/src/schema.graphqldemo/pkg/subgraphs/projects/src/service/service_resolve.gorouter-tests/go.modrouter-tests/grpc_subgraph_test.gorouter-tests/router_plugin_test.gorouter-tests/testenv/testdata/configWithGRPC.jsonrouter-tests/testenv/testdata/configWithPlugins.jsonrouter/go.mod
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2645 +/- ##
==========================================
+ Coverage 62.65% 62.85% +0.20%
==========================================
Files 244 244
Lines 25831 25831
==========================================
+ Hits 16184 16237 +53
+ Misses 8298 8259 -39
+ Partials 1349 1335 -14 🚀 New features to boost your workflow:
|
Noroth
requested review from
a team,
SkArchon,
StarpTech,
devsergiy,
endigma and
jensneuse
as code owners
devsergiy
approved these changes
Mar 13, 2026
…163-integration-into-cosmo-router
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
router-tests/protocol/router_plugin_test.go (1)
478-497: Consider centralizing duplicated no-arg resolver cases shared with gRPC protocol tests.This block is nearly identical to the new cases in
router-tests/protocol/grpc_subgraph_test.go; pulling shared cases/expected payloads into a helper would reduce drift and future update cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/protocol/router_plugin_test.go` around lines 478 - 497, The test cases for no-arg field resolvers (e.g., cases named "query project taskCount no-arg field resolver", "query projects with no-arg field resolvers", "query employees with totalProjectCount no-arg field resolver", and "query project mixing no-arg and with-arg field resolvers") are duplicated between router_plugin_test.go and grpc_subgraph_test.go; extract these shared entries and their expected JSON payloads into a single helper (e.g., a function like sharedNoArgResolverCases() or a package-level variable sharedNoArgCases) and have both test files consume that helper instead of duplicating the literal cases, updating the test harness that iterates over the cases to use the shared slice. Ensure the helper returns the same test struct type used by the existing tests so no signature changes are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@router-tests/protocol/router_plugin_test.go`:
- Around line 478-497: The test cases for no-arg field resolvers (e.g., cases
named "query project taskCount no-arg field resolver", "query projects with
no-arg field resolvers", "query employees with totalProjectCount no-arg field
resolver", and "query project mixing no-arg and with-arg field resolvers") are
duplicated between router_plugin_test.go and grpc_subgraph_test.go; extract
these shared entries and their expected JSON payloads into a single helper
(e.g., a function like sharedNoArgResolverCases() or a package-level variable
sharedNoArgCases) and have both test files consume that helper instead of
duplicating the literal cases, updating the test harness that iterates over the
cases to use the shared slice. Ensure the helper returns the same test struct
type used by the existing tests so no signature changes are needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33124b13-c9af-421a-a5d3-eae64eb552ff
📒 Files selected for processing (2)
router-tests/protocol/grpc_subgraph_test.gorouter-tests/protocol/router_plugin_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #2527
This PR adds support for field resolvers without arguments in the cosmo router. Previously we only allowed field resolvers to be defined on fields that have arguments. To achieve the same result we could provide a hacky solution to add a dummy argument to a field
We can now omit expensive fields from the type lookup explicitly without providing a dummy argument.
Summary by CodeRabbit
New Features
Checklist