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

Only one operation per file #5

Open
sebas5384 opened this issue Aug 6, 2018 · 4 comments
Open

Only one operation per file #5

sebas5384 opened this issue Aug 6, 2018 · 4 comments

Comments

@sebas5384
Copy link
Contributor

Hi! this lib is so cool, thank you 👍

This lib supports one operation by file, but the graphql-tag/loader supports multiple.

Why do we have this limitation here?

I guess it could be useful to warn users about this limitation, since I got caught by surprise.

thanks!

@sebas5384
Copy link
Contributor Author

sebas5384 commented Aug 6, 2018

Don't know if make sense but maybe It could modify the exported queries when there's multiple operations.

from:

module.exports = doc;
module.exports["MyQuery"] = oneQuery(doc, "MyQuery");
module.exports["MyMutation"] = oneQuery(doc, "MyMutation");

to:

module.exports = doc;
module.exports["MyQuery"] = oneQuery(doc, "MyQuery");
module.exports["MyMutation"] = oneQuery(doc, "MyMutation");
module.exports["MyQuery"].documentId = "HASH1";
module.exports["MyMutation"].documentId = "HASH2";

I guess the challenge here could be getting the operation name from the queryMap keys and then concatenate the module.exports["operationName"].documentId = "hash"; for each query at the end of the content.

@leoasis
Copy link
Owner

leoasis commented Oct 24, 2018

Hey! @sebas5384 I've been thinking about this, and based on the code I think we may need to do something different. In practice, you persist a document, not just a single query. In the case of a document with different operations, the documentId should still be the same, and you'd refer to the operation you want to execute with both the documentId and the operationName.

I think I'll be adding those changes, as right now PersistGraphQL splits the documents per operation and that's not what we really want to do. Also, PersistGraphQL will be deprecated, so we might just as well remove our dependency with it.

Any thoughts? Concerns? Anything you think I may be missing?

@leoasis
Copy link
Owner

leoasis commented Oct 24, 2018

Having said that, it seems that this is not really how the rest of the Apollo libraries work, as they all work with documents that contain a single operation, or split them into separate documents if that's the case. It feels now that maybe documentId is misleading in this case.

@sebas5384
Copy link
Contributor Author

Hey @leoasis , yeah when I commit the code I thought about the documentId since is not the "document id" anymore is the "operation id".
Maybe we could change that to operationId and get rid of the documentId.

About the PersistGraphQL being deprecated, are thinking on a new approach or something like that? or about these code:

const { ExtractGQL } = require('persistgraphql/lib/src/ExtractGQL');
const queryTransformers = require('persistgraphql/lib/src/queryTransformers');

if it is the code, then I guess we could open an issue as the apollographql/persistgraphql#67 states, what do you think?

Sorry about the delay to answer you! I had to think for a while about my answer.

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

No branches or pull requests

2 participants