-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add ast toString returning query string #214
base: master
Are you sure you want to change the base?
Conversation
@jnwng Would you mind reviewing this? This could be a helpful feature and make debugging easier as well. |
This is just what I want. @jnwng ping |
@stubailo Would you mind reviewing this? |
@darren128 still waiting on a fix! |
1ebc5b8
to
332cbdf
Compare
@stubailo Fixed and added a test for the use case. |
Ready for review @stubailo |
It would be nice to add a parsed.toString = function (condense) { return condense ? print(this, { condense: true }) : print(this); };
// ...
const query = `{ query { user { firstName lastName } } }`;
const ast = gql(query);
console.log(ast.toString(true)); // "{query{user{firstName lastName}}}" Related: graphql/graphql-js#1523 |
@rybon, i wrote this right now: https://github.com/gabsprates/minify-graphql-loader |
@stubailo Bumping this for review, thanks! |
would love to see this merged! |
Is there any update about it? |
@gfx Bumping this for review, thanks! |
@jnwng will this PR ever move anywhere? |
to confirm, this is a breaking change, correct? are there cases where folks may be relying on the current return value of |
@jnwng Yes, it would break the current usage of |
as mentioned in #206 (comment), using given the breaking-ness of this change, i'm not sure it merits a major bump and i'm wondering if documentation might be resolve this issue. is it important that we're passing the string through the printer? |
Would changing the naming in combination with graphql/graphql-js#1523 feature help resolve those concerns? |
Here's a proposal: var stripIgnoredCharacters = require('graphql').stripIgnoredCharacters;
// ...
parsed.toStrippedString = function() { return stripIgnoredCharacters(this); }; Requires graphql-js 14.3.0 |
My proposal as a PR: #260 |
If it's any help, I managed to add this feature to I believe it achieves the same thing as this PR, only difference is the GraphQL string literals are stripped of insignificant characters instead of pretty printed. |
As mentioned in this comment, you can do the following to get the graphql string: require('graphql-tag').default('query { test }').loc.source.body |
Will we get the ability to |
Thanks. Like
|
Chiming in here to say that since |
@good-idea How did you handle this finally? |
Pull Request Labels
For #206
Example