Skip to content

feat(resolveField): allow field middlewares to return a marshaler directly#3928

Merged
StevenACoffman merged 1 commit into
99designs:masterfrom
lbarthon:lbarthonet/resolve-field-marshaler
Dec 2, 2025
Merged

feat(resolveField): allow field middlewares to return a marshaler directly#3928
StevenACoffman merged 1 commit into
99designs:masterfrom
lbarthon:lbarthonet/resolve-field-marshaler

Conversation

@lbarthon

@lbarthon lbarthon commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

In some specific cases, we'd like the ability from a field resolver to return a marshaler directly. This is needed for specific performance-sensitive codepaths we have in our application, where part of the request might already be resolved and we don't want to resolve it again, as the performance hit from doing so would be too high.

This shouldn't impact any already existing usecase, and only make those field middlewares more capable.

Describe your PR and link to any relevant issues.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@lbarthon

Copy link
Copy Markdown
Contributor Author

@StevenACoffman is that something you'd consider landing? will fork otherwise, as this is much needed. happy to add test in followup diffs

@lbarthon lbarthon force-pushed the lbarthonet/resolve-field-marshaler branch from d483a0d to c124303 Compare November 19, 2025 15:14
@lbarthon lbarthon changed the title feat(resolveField): allow field middlewares to return a marshaler dir… feat(resolveField): allow field middlewares to return a marshaler directly Nov 19, 2025
@lbarthon lbarthon force-pushed the lbarthonet/resolve-field-marshaler branch from c124303 to e0cd850 Compare November 19, 2025 15:18
@coveralls

coveralls commented Nov 19, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 0.0%. remained the same
when pulling 2bede3b on lbarthon:lbarthonet/resolve-field-marshaler
into 03c5f87 on 99designs:master.

@StevenACoffman

Copy link
Copy Markdown
Collaborator

I would be ok with merging this, but would you be ok with adding some test coverage and/or documentation for this new behavior?

@StevenACoffman

StevenACoffman commented Dec 1, 2025

Copy link
Copy Markdown
Collaborator

@lbarthon I was checking in on this, as I would like to merge this before the next release, but I'm holding off merging this PR until you get a chance to add some test coverage and/or documentation for this new behavior. Do you anticipate being able to spend some time on that soon?

@lbarthon

lbarthon commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

Yes @StevenACoffman, on it now

@lbarthon lbarthon force-pushed the lbarthonet/resolve-field-marshaler branch from e0cd850 to 7fd9fbb Compare December 1, 2025 23:05
@lbarthon

lbarthon commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

I added tests - is that enough? Got another PR I want to make, working on this now

…ectly

In some specific cases, we'd like the ability from a field resolver to return a
marshaler directly. This is needed for specific performance-sensitive codepaths
we have in our application, where part of the request might already be resolved
and we don't want to resolve it again, as the performance hit from doing so
would be too high.

This shouldn't impact any already existing usecase, and only make those field
middlewares more capable.
@lbarthon lbarthon force-pushed the lbarthonet/resolve-field-marshaler branch from 7fd9fbb to 2bede3b Compare December 1, 2025 23:07
@StevenACoffman

Copy link
Copy Markdown
Collaborator

Thanks! BTW, you refer to "we", and I'm curious who "we" is? Are you working with a group of developers? Is there a public project I can peek at somewhere? If you can't speak about it publically, maybe reach out to me directly on linkedin at https://www.linkedin.com/in/steve-coffman-79322175/ or via my work email steve@khanacademy.org

@StevenACoffman StevenACoffman merged commit 9da91b3 into 99designs:master Dec 2, 2025
18 checks passed
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