Skip to content
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

Batch delegation memoization is insensitive to non-key arguments #5188

Open
4 tasks
Tracked by #5201 ...
maxbol opened this issue Apr 18, 2023 · 1 comment
Open
4 tasks
Tracked by #5201 ...

Batch delegation memoization is insensitive to non-key arguments #5188

maxbol opened this issue Apr 18, 2023 · 1 comment

Comments

@maxbol
Copy link
Contributor

maxbol commented Apr 18, 2023

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

    Make sure to fork this template and run yarn generate in the terminal.

    Please make sure the GraphQL Tools package versions under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

When using batchDelegateToSchema, the cacheKey for fetching a memoized loader is created using either the requested field name (for fields returning scalars) or the field name + selection set (for fields returning composites). When using this in conjunction with non-key arguments (for instance via argsFromKeys), those arguments do not effect the cacheKey. Thus, delegating the same field multiple times with different arguments only ever result in one delegation.

Possible solution is to use the entire field node (with alias redacted) instead of only the selection set when creating the cache key. This seems to solve the problem, afaik (see related PR).

#5189

To Reproduce
Steps to reproduce the behavior:

See unit tests in related pull request.

Expected behavior

Delegate resolvers should run once for every set of non-key arguments passed to batchDelegateToSchema.

Environment:

  • OS:
  • @graphql-tools/batch-delegate: 8.4.25
  • NodeJS: v16.13.2
@maxbol
Copy link
Contributor Author

maxbol commented Dec 19, 2023

Has anyone been able to have a look at this? Would love a fix - or a suggestion on how to move forward with implementation since my PR received a negative review. Right now we have to run a fork of batch-delegate which is not ideal

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

No branches or pull requests

1 participant