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

Cache yoga conditional schema #5170

Merged
merged 2 commits into from
Apr 25, 2024
Merged

Cache yoga conditional schema #5170

merged 2 commits into from
Apr 25, 2024

Conversation

lucasbordeau
Copy link
Contributor

In this PR I'm introducing a new patch on @graphql-yoga/nestjs package.

This patch overrides a previous patch that was made to compute the conditionnal schema on each request,

Here we use a cache map to compute only once per schema workspace cache version.

This allows us to have sub 100ms query time.

@lucasbordeau lucasbordeau changed the title Feat/yoga-schema-cache Cache yoga conditional schema Apr 25, 2024
@charlesBochet
Copy link
Member

LGTM! Bravo!

@charlesBochet charlesBochet merged commit 52f4c34 into main Apr 25, 2024
5 checks passed
@charlesBochet charlesBochet deleted the feat/yoga-schema-cache branch April 25, 2024 12:01
charlesBochet added a commit that referenced this pull request Apr 25, 2024
In this PR:
- Follow up on #5170 as we did not take into account not logged in users
- only apply throttler on root fields to avoid performance overhead
@magrinj
Copy link
Member

magrinj commented Apr 26, 2024

Great job, in my opinion we should make this more generic, as you're extracting stuff like workspace id from request and it's only based on our codebase. We should avoid that.
Also graphql-yoga already has a system of caching by request, maybe this one was not properly used with conditional schema

cc @charlesBochet, @lucasbordeau

@charlesBochet
Copy link
Member

Agree, let's discuss with Yoga guys to see what would be the ideal API.
We could either introduce callbacks or introduce a cacheKey that we would compute on our own

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