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

useOnResolve incorrectly calls onResolve multiple times #2222

Open
magJ opened this issue Apr 30, 2024 · 1 comment
Open

useOnResolve incorrectly calls onResolve multiple times #2222

magJ opened this issue Apr 30, 2024 · 1 comment
Labels
kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it

Comments

@magJ
Copy link

magJ commented Apr 30, 2024

Summary

If you have:

  • more than one plugin that uses useOnResolve
  • & onSchemaChange is triggered more than once

then field.resolve will be wrapped multiple times and onResolve will be triggered multiple times.

Detail

The number of redundant calls will be proportional to the number of useOnResolve plugins, and the number of times onSchemaChange is called.

This happens because the hasWrappedResolve symbol is private to a single plugin instance.

Reproduction

Consider the following scenario:

  • An envelop instance is created with two plugins that use useOnResolve, Plugin A and Plugin B
  • onSchemaChange fires the first time
  • Plugin A wraps the original field.resolve
  • Plugin B wraps Plugin A's wrapper.

So far, so good, each plugin will still behave as expected(onResolve is still only called once for each plugin when a resolver is invoked), however:

  • The schema is changed, and onSchemaChange is fired a second time.
  • Plugin A wraps Plugin B's wrapper of Plugin A's first wrapper
  • Plugin B wraps Plugin A's second wrapper of Plugin B's wrapper of Plugin A's first wrapper.

Now when a resolver is called onResolve ends up getting called twice per plugin.

Suggested solution

  • A new symbol should be addedSymbol('parentResolver'), this symbol must be a singleton, not private to a single plugin instance.
  • When wrapping field.resolve, field[parentResolver] should be set to the original resolver.
  • When checking if the plugin has hasWrappedResolveSymbol set, the parentResolver should be recursively checked to ensure that the resolver has not already been wrapped.

Side-note, the parentResolver symbol should probably be publicly exported, so that other code that may also be wrapping resolvers, can use it.

@EmrysMyrddin
Copy link
Collaborator

Thank you for your very detailed report!

I'm not sure to understand how we can end up with wrapped resolvers inside the schema on the second onSchemaChange call ? If the schema change, its resolvers should not be wrapped isn't it ?

@EmrysMyrddin EmrysMyrddin added kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it labels May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it
Projects
None yet
Development

No branches or pull requests

2 participants