Wrap cm6-graphql lint logic in try..catch

[imolorhe]

It turns out that getDiagnostics may throw errors which codemirror 6 doesn't handle in the lint. Wrapping the logic in try..catch to handle errors and just return an empty array instead.

[changeset-bot]

🦋 Changeset detected

Latest commit: 54ebaeb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
cm6-graphql	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

Merging #3461 (54ebaeb) into main (2348641) will increase coverage by 0.10%.
Report is 28 commits behind head on main.
The diff coverage is 79.31%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3461      +/-   ##
==========================================
+ Coverage   55.75%   55.85%   +0.10%     
==========================================
  Files         110      110              
  Lines        5243     5264      +21     
  Branches     1426     1435       +9     
==========================================
+ Hits         2923     2940      +17     
- Misses       1897     1898       +1     
- Partials      423      426       +3     

Files	Coverage Δ
packages/graphiql-react/src/editor/context.tsx	73.39% <100.00%> (ø)
...raphql-language-service-server/src/GraphQLCache.ts	50.88% <50.00%> (ø)
...ql-language-service-server/src/MessageProcessor.ts	46.11% <50.00%> (-0.11%)	⬇️
...nguage-service/src/utils/getVariablesJSONSchema.ts	94.59% <98.38%> (+0.71%)	⬆️
packages/graphiql/src/components/GraphiQL.tsx	68.96% <0.00%> (-0.97%)	⬇️
packages/graphiql-react/src/editor/hooks.ts	40.15% <23.52%> (+1.56%)	⬆️

[github-actions]

The latest changes of this PR are available as canary in npm (based on the declared changesets):

cm6-graphql@0.0.11-canary-7efdaeb3.0

[acao]

makes sense for me! now I think i need to add this to our other mode, haha!

[imolorhe]

@acao I was thinking about this a bit more. The error that gets thrown is usually because of an invalid schema:

• getDiagnostics() calls validate() in graphql
• validate() throws an error if the schema is not valid

With this current try..catch, we are ignoring this type of error, which you can argue is fine since it's not a validation error in the GraphQL query (which the linter is meant for), but should we also report the invalid GraphQL schema from the linter as well? (The position of the error can just be line 1, col 1). I feel like that might be more useful to devs. We could also consider making devs choose the behavior they want here with a config option.

What do you think?

[acao]

Hmm yeah, that makes a lot of sense to me!
Perhaps we can even write schema errors to an StateField? Or maybe that's not needed

[imolorhe]

Yeah writing to a state field may not be needed