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

Export Parser class? #2741

Closed
dotansimha opened this issue Aug 11, 2020 · 4 comments · Fixed by #2744
Closed

Export Parser class? #2741

dotansimha opened this issue Aug 11, 2020 · 4 comments · Fixed by #2744

Comments

@dotansimha
Copy link
Member

dotansimha commented Aug 11, 2020

At the moment, the Parser class isn't exported at all (https://github.com/graphql/graphql-js/blob/master/src/language/parser.js#L165) which limits the ability to extend or use it as-is.

Is there a reason for that?

I would like to extend and add functionality to it, but it seems like it's not possible :(

@IvanGoncharov

IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this issue Aug 12, 2020
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this issue Aug 12, 2020
@IvanGoncharov
Copy link
Member

IvanGoncharov commented Aug 12, 2020

@dotansimha Exported in #2744 note that we plan so major changes to parser implementation as part of merging GraphiQL into graphql-js plus this class can be changed due to reflect spec changes.
So you should treat this API as exported but it still treated as internal API in all other aspects including versioning.
It also accessible only with explicit import:

import { Parser } from 'graphql/language/parser`

Also, can you please share what features is missing in our current parser so we can consider adding it in the future?

@dotansimha
Copy link
Member Author

dotansimha commented Aug 16, 2020

@IvanGoncharov thanks!

I wanted to fix a parsing issue related to comments/descriptions.
There is commentsDescriptions in print method, in order to print it correctly, but in terms of parsing, v15 of graphql-js will hide internally all comments that were defined with #.
This was needed for graphql-codegen - we wanted to allow developers to load their schema from SDL that contains #, and convert the # into """ comments while parsing (and not while printing), in order to have access to those later, while traveling the AST tree with visit.
Since Parser wasn't available, we needed to wrap it with a custom parse method here: ardatan/graphql-tools#1900 , that convert # to """ prior to calling the original parse.
Ideally, we would implement an extended version of Parser that does that automatically (instead of parsing it twice).

@IvanGoncharov
Copy link
Member

@dotansimha Thanks for the detailed explanation 👍
Note: we will remove support for # as descriptions in v16.

@dotansimha
Copy link
Member Author

Yeah I know, that's the reason we wanted to support it in graphql-tools. To allow developers who still have # comments to upgrade to v16 of graphql and still use the tools without breaking changes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants