Skip to content

Conversation

acoBOYZ
Copy link
Contributor

@acoBOYZ acoBOYZ commented Apr 10, 2025

Fix internal defineLoader batching issue + dependency & GraphiQL updates

This pull request fixes a problem in Mercurius' internal defineLoader function, where GraphQL field loaders were being called once per item, instead of once per field with batching.


✅ What was fixed

The problem was caused by using complex objects ({ obj, params }) as loader keys, which prevented Mercurius from recognizing repeated calls as "the same".

We now use a serialize() function that:

  • Uses obj.id and params.lang to create a fast and consistent batching key
  • Falls back to JSON.stringify({ obj, params }) if id is not available

We use id because it's a commonly used identifier in GraphQL schemas, and it's much faster than serializing the entire object.


✅ Also included

  • Upgraded all dependencies to their latest stable versions
  • Updated GraphiQL static files (main.js, config.js) to the latest version

🧪 Real-world test results (k6)

Before (no batching):

  • ~122ms average response time
  • Loader was called many times per query

After (with batching):

  • ~37ms average response time
  • Loader is called only once, with all keys batched
  • ~70% performance improvement confirmed under load

🔒 Notes

  • This fix is applied internally, users do not need to change how they define their own loaders.
  • It's safe and backward-compatible.

Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you add some tests as well?

return query

return stringify({ obj, params })
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is generic enough to be included in the library. How about you add a a way to include a custom serializer for the loader instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I found a better way to handle it. Also i will add some tests for it. After that if you like it can pr docs for that too. I will publish it soon

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