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

monaco-graphql: Unexpected EOF with comments-only content #2589

Open
jonathanawesome opened this issue Jul 26, 2022 · 1 comment
Open

monaco-graphql: Unexpected EOF with comments-only content #2589

jonathanawesome opened this issue Jul 26, 2022 · 1 comment

Comments

@jonathanawesome
Copy link
Collaborator

jonathanawesome commented Jul 26, 2022

A user reported a UI bug in the GraphiQL design prototype I've been working on that's related to monaco-graphql.

When the model value consists only of comment lines, we get a syntax error:

Screen Shot 2022-07-26 at 10 26 05 AM

This is reproducible in the monaco-graphql-react-vite example.

While it's simply a little annoying to see the error in the editor, the impact is heavy when attempting to parse the comments-only value because it errors out.

Ideally, a comments-only value wouldn't produce an error during parsing.

I can work around this in the prototype, so this is low priority.

@acao
Copy link
Member

acao commented Jul 28, 2022

we get something similar with codemirror-graphql as well it seems. when graphql-js tries to parse an empty file with only comments, it doesn't like this! it used to just skip it and return null, which is why this worked better in earlier versions of graphiql/playground/etc, but now it throws a validation error (now as in graphl-js@15 at least, it's been like this a while)

the simple answer to this without comments (aka empty file) in monaco-graphql at least, was to make sure we only call validation in cases where the query is a string of more than a minimum length of say, two or three characters, which is what we do now.

however, this doesn't work when there is only empty whitespace lines or comments. so when graphql-js tries to parse this, it sees only whitespace. we can use a regex perhaps to see if any lines don't start with comment blocks, but what if the comment blocks are indented? there is a lot of complexity to this, because obviously we can't just try to parse the file. we could try to trap just this specific exception type and skip it, using error.message for example, and still bubble up the other exections?

@acao acao added the bug label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants