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 caching for GraphQL queries #395

Merged
merged 1 commit into from
Feb 27, 2025
Merged

Fix caching for GraphQL queries #395

merged 1 commit into from
Feb 27, 2025

Conversation

alecgeatches
Copy link
Contributor

@alecgeatches alecgeatches commented Feb 26, 2025

We currently use this HTTP caching strategy on trunk:

// For most HTTP requests, we only want to cache GET requests. This is
// overridden for GraphQL queries when using GraphqlQuery
if ( 'GET' !== strtoupper( $this->get_request_method() ) ) {
// Disable caching.
return -1;
}

The comment is incorrect. We inherit the same strategy in GraphqlQuery, and this has some issues. GraphQL queries are typically implemented as POST requests, which means that we're skipping the cache each time we make a GraphQL query. For large pages of items or a lot of properties, this results in a huge number of cache misses and slow page loads for our GraphQL data sources like Shopify.

This PR adjusts GraphQL queries to use the default caching strategy, ignoring the fact that it's a POST request. GraphQL mutations are still uncached.

Testing

  1. On the trunk branch, set up a Shopify connection.

  2. Create a new post and add a product with the default template.

  3. Publish the page and view the page on the frontend as a logged-in user (e.g. as admin on a local wp-env instance).

  4. View the Query Monitor logs by clicking on the query monitor bar (top arrow) and clicking the "Logs" tab on the lower left:

    Screenshot 2025-02-26 at 12 38 38 PM

  5. Note the several logs related to cache misses. Refresh the page and confirm that caching is not working.

  6. Switch to this branch fix/graphql-caching.

  7. Refresh the page. You should see one cache miss follow by several cache hits:

    image

  8. Refresh again. All requests should hit the cache now, and the page should load very quickly.

  9. Wait 60 seconds and refresh. You should see a fresh uncached request followed by cached result, the same as step 7.

Copy link

Test this PR in WordPress Playground.

Copy link
Contributor

@shekharnwagh shekharnwagh left a comment

Choose a reason for hiding this comment

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

Works as described. Thank you for fixing it !

@alecgeatches
Copy link
Contributor Author

Thank you @shekharnwagh! Merging!

@alecgeatches alecgeatches merged commit 73a6bfc into trunk Feb 27, 2025
13 checks passed
@alecgeatches alecgeatches deleted the fix/graphql-caching branch February 27, 2025 17:42
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.

2 participants