Initial implementation of persisted queries plugin

[santino]

Description

This draft PR is for implementing a persisted queries plugin with the following goals:

• Handle multiple persisted queries list, to potentially support multiple clients integrating with a single GraphQL server
• Provide a basic implementation for a store that handle static JSON files (as generated by most common clients)
• Provide a mechanism to update the store at runtime, even targeting specific lists only (whitelist)

Since the plugin is meant to support multiple JSON files, it also offers an option to match from a single list only.
This allows developers to implement logic to target a single list, so you can avoid matching against all lists when you know your request comes from a specific application and so must be available in a single list.
The point is not necessarily to improve performance, in fact matching against multiple lists might still be implemented with o(1) cost (since it's just a lookup job), but more about offering proper support for multiple applications; in fact it might be that different applications have one or more equal queries, that would result in the same SHA256 id. By handling multiple and separate queries lists there is no worry of conflicts and file might just be provided to the server as they're generated by clients.

Type of change

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How Has This Been Tested?

Being just a draft, this is open for a conversation.
I am planning to add tests once we finalise the strategy.

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented on my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

Further comments

This plugin might be seen in conflict with the existing persisted-oeprations, but it differs as it is more intended (but not limited) to handle static lists.
The main difference is the concept of handling multiple lists to support multiple client applications.
Also this plugin provides an implementation for a store (again targeting a basic usage with static files), which is useful to be used as a guide for developers wanting to implement their own store from other sources (such as databases).

Ultimately I am open to merge the two plugins so that we offer a solution that is flexible enough and provides a basic store implementation.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0e80b10

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/envelop/En2spsP5Nq6WpT2kySkozFdPCw3L
✅ Preview: https://envelop-git-fork-santino-usepersistedqueries-theguild.vercel.app

[santino]

Adding this separate comment to discuss more the differences with the existing "persist-operations" plugin.
I think the goals of the two are different. The existing plugin is highly customisable but relies on full implementation from developers, they can write their own store in any shape and build their matching logic.
This new plugin wants to be more of a Plug'n'Play solution, whilst also providing extra functionalities. This is possible by being more assertive on the shape of the store, in this case, I'm expecting a Map with a key/value pair where the key is the name of a list and the value is an object containing hashes and their respective stringified documents.

In all honesty, I wouldn't even be sure how to add the extra functionalities to the existing plugin without changing the existing API.
You cannot support multiple persisted lists if you don't know which shape your store has, at least not if you want to provide a ready-to-use matching logic; rather than just relying on developers building the whole thing on their own; which is a core goal of this plugin.
If you do change the API, then you introduce breaking changes and in the end, you do end up with a different plugin anyway, even if you keep it in the existing package and with the same name.

I could add a simple store implementation that handles static JSON files to the existing plugin, but then developers will still have to build their matching logic to retrieve the stringified documents from the store.
The above logic devs need to build, becomes more complex if they want to handle multiple lists. You could provide snippets in the README for doing this, but still, it remains code that developers are responsible for, they need to write it and maintain it, hence keeping the knowledge overhead; as such is not a Plug'n'Play solution.

My plugin provides a good solution for static JSON files, but this can be used as an example and customised for more complex setups (such as grabbing persisted lists from a database f.i.).
Yet, the only thing developers need to provide is just a Map with key/value pairs; it will work with a single list or multiple lists and they won't need to write nor maintain any logic to use their lists.

Hope this helps.

[dotansimha]

I like this concept. I wonder if some of the things can be merged with persisted-operations plugin? 🤔

[santino]

Some, probably, you could define an easy way to get the id from context and implement default behaviour to get it from GraphQL source.
You could also provide a simple store implementation for static files.

Anything else, like a built-in matching logic that works out-of-the-box with one/multiple lists, would require breaking changes in the plugin.

The best way I could think about implementing those functionalities is the way I built this new plugin.

So the question still remains.
Do you think we should have both plugins as part of Envelop?
If not, which one should we prioritise?
We introduce concepts implemented here onto existing persisted-operations, but at the condition of introducing breaking changes?
Or do we release a new plugin and deprecate the existing one?

[dotansimha]

Some, probably, you could define an easy way to get the id from context and implement default behaviour to get it from GraphQL source.

@kamilkisiela did writeToContext flag in the existing plugin, in order to make it compatible. We can also add a function to extract the id from the context, and then you can use context.req if it's there, and provide custom function for that.

You could also provide a simple store implementation for static files.

The way I see it, the store should have a single API for get, like we have today, and then behind that, you can implement any store, based on any amount of files.

Anything else, like a built-in matching logic that works out-of-the-box with one/multiple lists, would require breaking changes in the plugin.

I'm not sure that we have to break it, can you please explain why we can do a single store, that supports multiple files? eventually it doesn't matter which file the data is fetched from, it's just needs the id and the document.

Do you think we should have both plugins as part of Envelop?

I still think we can find a way to keep the persisted operations API simple as possible, and allow the store to implement any kind of flow.

Maybe the API for get(id: string): string | DocumentNode | null; can be get(id: string): { document: string | DocumentNode | null, customData: any }; and then each store can return custom data to be added to context? (so if it's based on multiple stores or files, and you need to know about that you can add it there).

[santino]

We can also add a function to extract the id from the context

yes, this can easily be added, and also a default behavior to get id from source, as I have implemented here (which is effectively comparable to a default implementation for canHandle as currently expected by "persisted-operations" plugin.

can you please explain why we can do a single store, that supports multiple files

The main use case is to support multiple applications. Each application exports its list of persisted queries. Some applications might require one or more exact queries that will have conflicting hashes. The conflict is not necessarily a problem, cause you can ignore duplicate hashes, trusting that their value will always be equal.

But when I support multiple applications I might want to tell my server that the requests come from application A and it should match queries only from the list generated by application A rather than all applications supported by my server.
It might not necessarily be a performance concern, because after all, we're still talking about O(1) and not O(n); but it might be a security concern: I want clients to disclose who they're, and allow them to access only queries defined by their themselves.
Let's not forget that persisted queries are not just about improved performance, but also security improvement as you're not disclosing the schema openly.

I'm not sure that we have to break it

With the context above, if you do want to support multiple lists, you need a signature for the store, which is no longer a simple key/value pair.

I still think we can find a way to keep the persisted operations API simple as possible

This is my main goal, if you look at the README file you can see how you can simply pass one or more json files to the plugin and it works out of the box; the API is damn simple in my view.

If you do want to implement your own store, the only overhead I am proposing (to prescript the store shape) is to wrap each list in a Map which key is the name of the list and value is the full object containing hashes and their values.
It's literally a wrapping layer, that does not make the API more complex.
In spite of this wrapping layer, you do not need to pass canHandle nor get functions in the new API.
You can, for a more complex setup (that you are willing to add to the existing plugin) use setQueryId to retrieve query id in a custom way (e.g. from context).

and allow the store to implement any kind of flow

Do you think the plugin I am proposing would prevent anything here?

As for your proposal to extend get API, yes, probably that could work.
But you would still rely on developers building their own logic to support multiple lists in a single store, within the get function; where my goal is precisely to support one and multiple lists out-of-the-box.

I am only addressing your points, but really do not feel like I am forcing you to accept this new plugin.
In the same way, I am addressing your points I would invite you to reflect, and maybe share, how my proposed plugin might affect them negatively, or offer an API that is not as simple as the existing one.
At the end of the day, I appreciate my plugin is different from the existing one, but if it does tick all the boxes, with simple API design, extended functionalities, and the ability to fain-grain custom usages; it might be good regardless.

[dotansimha]

We are going with #498 , closing.