Skip to content

Document use of DataFetcherResult for injecting values in local context #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bclozel opened this issue Apr 1, 2025 · 2 comments
Closed
Assignees
Labels
type: documentation A documentation task
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Apr 1, 2025

See #1142 for background

Right now Annotated controllers support many method arguments, including:

  • @ContextValue, for access to an attribute from the main GraphQLContext in DataFetchingEnvironment.
  • @LocalContextValue, for access to an attribute from the local GraphQLContext in DataFetchingEnvironment.
  • GraphQLContext, for access to the context from the DataFetchingEnvironment.

Controller methods can only contribute a new local context by returning a DataFetcherResult<ReturnType>. Spring MVC does support java.util.Map, org.springframework.ui.Model, org.springframework.ui.ModelMap as mutable maps to be used when rendering a view. Injecting a new, mutable GraphQLContext local context would be quite similar to existing MVC support but might be confusing given the extensive context support we already have.

As part of this issue, we should first document the DataFetcherResult route and then consider whether we want to improve support.

@bclozel bclozel added the type: documentation A documentation task label Apr 1, 2025
@bclozel bclozel added this to the 1.4.0-RC1 milestone Apr 1, 2025
@bclozel bclozel self-assigned this Apr 1, 2025
@bclozel bclozel added the for: team-attention An issue we need to discuss as a team to make progress label Apr 1, 2025
bclozel added a commit that referenced this issue Apr 1, 2025
@bclozel
Copy link
Member Author

bclozel commented Apr 1, 2025

@bclozel bclozel removed the for: team-attention An issue we need to discuss as a team to make progress label Apr 3, 2025
@bclozel bclozel closed this as completed in e2f954a Apr 3, 2025
@bclozel
Copy link
Member Author

bclozel commented Apr 3, 2025

We discussed the possibility of adding specific support for Local Contexts in the controller method signatures, but we decided against it in the end for the following reasons:

  • the DataFetcherResult API is quite sensible already
  • so far, we only inject input values as method parameters and injecting somehow a new local context instance would break this rule
  • usage of local context is not central to GraphQL applications, unlike Model for MVC apps. Even there, we also often use ModelAndView or Rendering return types
  • this support would require more than a typical argument resolved, as we would need to also wrap the response with a DataFetcherResult
  • we would have to specify the behavior when the method gets injected with a local context and also returns a DataFetcherResult
  • We can't simply rely on the GraphQLContext type for method argument resolution as it's already used for the main context. We would need maybe to contribute a new specific type

Overall, we don't think working on a new feature there is worth it right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

1 participant