Skip to content

Allow context to be scoped #1385

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

Merged
merged 2 commits into from
May 30, 2023
Merged

Allow context to be scoped #1385

merged 2 commits into from
May 30, 2023

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented May 26, 2023

By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:

query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}

In the above situation, the following will happen:

  • a receives the cloned context from the executor
  • b receives the cloned context from a
  • c receives the cloned context from b
  • d receives the cloned context from a
  • e receives the cloned context from d
  • f receives the cloned context from e
  • g receives the cloned context from the executor
  • h receives the cloned context from g
  • i receives the cloned context from h

References:

@ruudk ruudk force-pushed the scoped-context branch 2 times, most recently from 6ee78e9 to cef780a Compare May 26, 2023 09:46
Copy link

@SavageTiger SavageTiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was fast 🚀

@ruudk ruudk force-pushed the scoped-context branch 2 times, most recently from 892fd86 to 9f4fec6 Compare May 26, 2023 09:54
@ruudk
Copy link
Contributor Author

ruudk commented May 26, 2023

@spawnia The PR is ready for review. I'd love to hear what you think. If you have suggestions on how to improve this, please let me know

Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's figure out where this can be documented. I think at least the PHPDoc that describes $contextValue should be extended, perhaps an entire new section of docs can be added?

@ruudk ruudk force-pushed the scoped-context branch 4 times, most recently from 545bc8b to 698d617 Compare May 26, 2023 14:26
@ruudk
Copy link
Contributor Author

ruudk commented May 26, 2023

@spawnia Ready. Added a few lines to the documentation. I think it's sufficient.

@spawnia
Copy link
Collaborator

spawnia commented May 28, 2023

Concerning docs, I would like to provide an example. The pull request description does not seem right:

d receives the cloned context from d
f receives the cloned context from d

Again, I think the following is still true:

I think at least the PHPDoc that describes $contextValue should be extended,

See

* contextValue:
* The context value is provided as an argument to resolver functions after
* field arguments. It is used to pass shared information useful at any point
* during executing this query, for example the currently logged in user and
* connections to databases or other services.
. A more detailed description can be available in the interface. Also, the interface should be added to the class reference generation.

By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `a`
* `f` receives the cloned context from `e`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
@ruudk ruudk force-pushed the scoped-context branch from 698d617 to 026729c Compare May 30, 2023 12:53
@ruudk
Copy link
Contributor Author

ruudk commented May 30, 2023

@spawnia Updated the PR. If you have better wording, please let me know.

@spawnia spawnia merged commit 94ac1db into webonyx:master May 30, 2023
@ruudk
Copy link
Contributor Author

ruudk commented May 30, 2023

@spawnia Thanks for merging it, would love to have it tagged if you have time 🙏 💙

nvm: I see the PR already.

ruudk added a commit to ruudk/graphql-php that referenced this pull request Mar 24, 2024
…lemented on the Context,

it would clone the context down to it's children.

This works great for queries. But mutations are executed serially. I forgot to
call `maybeScopeContext` there.

This fixes the problem, and adds a test to guard it.
ruudk added a commit to ruudk/graphql-php that referenced this pull request Mar 24, 2024
In webonyx#1385 we introduced the `ScopedContext` interface. When implemented on the Context, it would clone
the context down to it's children.

This works great for queries. But mutations are executed serially. I forgot to call `maybeScopeContext` there.

This fixes the problem, and adds a test to guard it.it would clone the context down to it's children.
spawnia pushed a commit that referenced this pull request Jun 10, 2024
In #1385 we introduced the `ScopedContext` interface. When implemented on the Context, it would clone
the context down to it's children.

This works great for queries. But mutations are executed serially. I forgot to call `maybeScopeContext` there.

This fixes the problem, and adds a test to guard it.it would clone the context down to it's children.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants