graphql-hive / graphql-hive/envelop

`useOnResolve` incorrectly calls `onResolve` multiple times

Open
#2,222 1 comment 0 reactions 0 assignees View on GitHub
kind/bug stage/0-issue-prerequisites
Dominant language
No language data
Stars
827
Forks
132
PR merge metrics
No merged PRs in 30d

Description

## Summary
If you have:
- more than one plugin that uses `useOnResolve`
- & `onSchemaChange` is triggered more than once

then [`field.resolve`](https://github.com/n1ru4l/envelop/blob/448da121c41c3e7c760c8544db4713d358214375/packages/plugins/on-resolve/src/index.ts#L66) 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`](https://github.com/n1ru4l/envelop/blob/448da121c41c3e7c760c8544db4713d358214375/packages/plugins/on-resolve/src/index.ts#L53C43-L53C60) 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 added`Symbol('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.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.