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

fix: do not allow reserved context keys in ServerContext or UserContext #3263

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ardatan
Copy link
Collaborator

@ardatan ardatan commented May 9, 2024

Do not allow reserved context keys in UserContext and ServerContext if they don't match

For example, you cannot have request as a key in UserContext or ServerContext unless it is Request like below;

// @ts-expect-error Not allowed
createYoga<{
    request: FastifyRequest
}>(/* ... */);

// But allowed
createYoga<{
    req: FastifyRequest
}>(/* ... */);

// Also allowed
createYoga<{
    request: Request // From Fetch API
}>(/* ... */);

Copy link

changeset-bot bot commented May 9, 2024

🦋 Changeset detected

Latest commit: e78e15c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
graphql-yoga Patch
apollo-federation-gateway-with-yoga Patch
apollo-subgraph-with-yoga Patch
graphql-lambda Patch
cloudflare-advanced Patch
cloudflare Patch
functions Patch
nextjs-app Patch
hello-world-benchmark Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ardatan ardatan marked this pull request as draft May 9, 2024 14:25
Copy link
Collaborator

@enisdenjo enisdenjo left a comment

Choose a reason for hiding this comment

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

I like it!

@EmrysMyrddin
Copy link
Collaborator

Great idea ! You need help to actually finalize it ?

@ardatan
Copy link
Collaborator Author

ardatan commented May 17, 2024

It'd be great @EmrysMyrddin
There are still issues with TS stuff.

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.

None yet

3 participants