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

Allow for custom queryTransformers #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

toddtarsi
Copy link

First off, let me start by saying nice plugin!

Next, I would like to request this feature.
This would allow me to chain my custom persistgraphql queryTransformers via your tool, and still use the very simple assignment method of generateId: query => persistedQueries[query]. I am a fan of Apollo 2.0, but I don't feel ready to go all in on engine yet, so I don't use apollo-link-persisted-queries in the ad hoc method prescribed in the demos. As a result of using link directives, my queries have to be adjusted for the server. For example, I chain the addTypename queryTransformer with another transform to strip client fields. Rather than process the query in generateId and try to connect the dots, I'd rather transform the persistedQueries here as well and keep the same generateId call as before.

Please let me know your thoughts, and thanks!

First off, let me start by saying nice plugin!

Next, I would like to request this feature.
This would allow me to chain my custom persistgraphql queryTransformers via your tool, and still use the very simple assignment method of `generateId: query => persistedQueries[query]`. I am a fan of Apollo 2.0, but I don't feel ready to go all in on engine yet, so I don't use apollo-link-persisted-queries in the ad hoc method prescribed in the demos. As a result of using link directives, my queries have to be adjusted for the server.  For example, I chain the addTypename queryTransformer with another transform to strip client fields. Rather than process the query in generateId and try to connect the dots, I'd rather transform the persistedQueries here as well and keep the same generateId call as before.

Please let me know your thoughts, and thanks!
@toddtarsi
Copy link
Author

Hello, I just wanted to see if there were any issues here that I needed to address. I only need to use an optional override here, and can add to the documentation or whatever. Thanks for the repo!

@toddtarsi
Copy link
Author

Bump. This is a little silly. If you don't respond by the end of the month, I'll assume this is a dead repo, and make a competitor on npm and maintain that myself. I've been patient for over half a month, and I'm trying to help your tools. Would you like more documentation or something?

@leoasis
Copy link
Owner

leoasis commented Jun 18, 2018

@toddtarsi sorry for taking so much to answer this. I've been a bit away from Github because I recently switched jobs and had been a bit busy afterwards. I'll take a look at this today. Sorry for the frustration, will try to avoid it in the future!

@leoasis
Copy link
Owner

leoasis commented Jun 18, 2018

The change looks good, could you please update the docs for this new option and how it works, and also add an example for it? If you want you can put a simple use case for this here as a comment and I can work the example out from that.

@toddtarsi
Copy link
Author

Haha, okay thank you. Sorry I got crazy; I know thats the worst on Github. I'll update that this week. Thanks for the great repo!

@leoasis
Copy link
Owner

leoasis commented Jun 18, 2018

Sure thing, looking forward to the changes! Glad you found this useful and again sorry to keep you unanswered for so long

@toddtarsi
Copy link
Author

@leoasis - Sorry this took ten years! Updated with documentation. Please review and let me know your thoughts.

@toddtarsi
Copy link
Author

@leoasis - Hey, just checking in. Wanted to see if this was still on the table.

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