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

feat: monaco variables json, improved schema & worker loading & monaco api #1997

Merged
merged 16 commits into from Dec 5, 2021

Conversation

acao
Copy link
Member

@acao acao commented Oct 31, 2021

a ton of monaco-graphql changes here, many many breaking changes. API is entirely rewritten.

Changes

  • adds variables JSON completion, validation, hover, etc using the JSON Schema support in monaco-graphql
    • you can now set variable diagnostics like this:
    monacoAPI.setDiagnosticSettings({ 
      // currently Record<string, string[]>, 
      // may opt for Record<string, { uris: string[], ...otherConfig }>
      variablesEditorValidation: {
        [myOperationModel.uri.toString()]: [variablesModel.uri.toString]
      },
     // same as json.jsonDefaults.setDiagnosticConfig() options
      variablesDiagnosticSettings: { 
        allowComments: true, // enables jsonc support
        validateSchema: 'error'
      }
    })
    • currently you can only overwrite this config all at once, but I may add a method to update it more atomically
    • this is an abstraction that inevitably leads to some encapsulation issues, so the readme will demonstrate how to add support without this method.
  • uses graphql as the language ID finally. this defaults to microsoft's provided syntax support and language config (which you can find in the monaco-editor repository under basicLangauges).
    • important! this presumes that any improvements we need to make to the monarch grammar and language config will be contributed to monaco-editor, and we will serve as a full language worker implementation that extends their basic configuration.
  • completely rewrote monaco graphql schema loading logic. schema is loaded synchronously in main process now, and passed off to the web worker. monaco does not do any business of fetching remote schema, ala codemirror-graphql. This is a feature as graphql userland has many ways to request remote schemas. See graphql-tools for example!
  • accepts multiple schemas! using schemas ala monaco-json 🎉
    • for each SchemaConfig, you can provide either schema, introspectionJSON, introspectionJSONString, documentString and documentAST as schema input.
    • schema requires a uri that is not used for remote loading, but as a cache key and for the model uri. for definitions support, we will set an SDL string to a model at this URI that will be used for schema lookups. this can be a URL or a file URI, anything that works with monaco.URI.parse(). this is a common pattern with monaco, and the file URIs can either be an internal implementation detail, or can be presented to users ala vscode/codesandbox/etc, that's up to you.
    • ala monaco-json, each entry in schemas has a fileMatch array of normal file paths or globs to match up graphql files to their schemas <3
    • normalized graphql-config output could likely be mapped to this!
    • the api exposes a function to updateSchemas() in the GraphQLWorker.
    • whenever the schemas change, the editor language features are reset
    • GraphQLWorker will keep an in-memory cache of schema information despite changes to config

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2021

🦋 Changeset detected

Latest commit: 7fb25ac

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

This PR includes changesets to release 7 packages
Name Type
graphql-language-service-utils Minor
graphql-language-service Major
monaco-graphql Major
codemirror-graphql Patch
graphiql Patch
graphql-language-service-cli Patch
graphql-language-service-server 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

@acao acao changed the title feat: variables json working feat: monaco variables json working Oct 31, 2021
@acao acao force-pushed the feat/monaco-variables-json branch from 4f2dcf3 to 9a59671 Compare October 31, 2021 18:56
@github-actions github-actions bot temporarily deployed to graphiql-webpack October 31, 2021 19:00 Inactive
@acao acao force-pushed the feat/monaco-variables-json branch from 9a59671 to 2275fca Compare October 31, 2021 19:01
@codecov
Copy link

codecov bot commented Oct 31, 2021

Codecov Report

Merging #1997 (7fb25ac) into main (2d91916) will decrease coverage by 2.40%.
The diff coverage is 66.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1997      +/-   ##
==========================================
- Coverage   65.70%   63.30%   -2.41%     
==========================================
  Files          85       79       -6     
  Lines        5106     5276     +170     
  Branches     1631     1663      +32     
==========================================
- Hits         3355     3340      -15     
- Misses       1747     1932     +185     
  Partials        4        4              
Impacted Files Coverage Δ
packages/codemirror-graphql/src/hint.ts 94.73% <ø> (ø)
packages/codemirror-graphql/src/lint.ts 100.00% <ø> (ø)
packages/codemirror-graphql/src/results/mode.ts 47.05% <ø> (ø)
...kages/codemirror-graphql/src/utils/forEachState.ts 100.00% <ø> (ø)
...ckages/codemirror-graphql/src/utils/mode-indent.ts 0.00% <0.00%> (ø)
packages/codemirror-graphql/src/variables/hint.ts 89.70% <ø> (ø)
packages/codemirror-graphql/src/variables/mode.ts 79.48% <ø> (ø)
packages/graphiql/src/components/QueryEditor.tsx 63.96% <ø> (ø)
packages/graphiql/src/utility/fillLeafs.ts 5.33% <ø> (ø)
packages/graphiql/src/utility/onHasCompletion.ts 2.17% <0.00%> (ø)
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b72af5...7fb25ac. Read the comment docs.

@github-actions github-actions bot temporarily deployed to graphiql-1 October 31, 2021 19:04 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack October 31, 2021 19:05 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql October 31, 2021 19:06 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-1 October 31, 2021 19:30 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql October 31, 2021 19:31 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack October 31, 2021 19:31 Inactive
@acao acao marked this pull request as draft November 1, 2021 11:30
@github-actions github-actions bot temporarily deployed to graphiql-1 November 1, 2021 22:46 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack November 1, 2021 22:47 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql November 1, 2021 22:48 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-1 November 1, 2021 23:02 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack November 1, 2021 23:03 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql November 1, 2021 23:03 Inactive
@graphql graphql deleted a comment from github-actions bot Nov 1, 2021
@graphql graphql deleted a comment from github-actions bot Nov 1, 2021
@graphql graphql deleted a comment from github-actions bot Nov 1, 2021
@acao acao marked this pull request as ready for review November 2, 2021 18:11
@acao acao requested a review from a team November 3, 2021 15:36
@dotansimha
Copy link
Member

@acao I think having alpha releases will make testing for users easier. At the moment it's hard to test active PRs before they are being merged.

I started a draft PR here: #2000 (based on changesets and release only affected packages)

@acao
Copy link
Member Author

acao commented Nov 4, 2021

@dotansimha sure! i'm just asking folks to review the deploy previews which seem to have vanished :(

@acao
Copy link
Member Author

acao commented Nov 4, 2021

@dotansimha we used alphas before and it was really messy :( I think your approach with using canaries for each PR will be much better

@orta
Copy link
Member

orta commented Nov 4, 2021

On TypeScript we generate playgrounds for people to test the PRs against by requesting them in the PR - feels like something similar could happen here where we bundle up an arbitrary build and throw It on netlify/next/whatever?

@acao
Copy link
Member Author

acao commented Nov 4, 2021

@orta we generate three deploy previews for each PR currently, and push them to netlify

@acao
Copy link
Member Author

acao commented Nov 4, 2021

if you look at "deployments" below, they stopped deploying 3 days ago for some reason :/

acao and others added 15 commits December 5, 2021 21:49
Co-authored-by: Saihajpreet Singh <saihajpreet.singh@gmail.com>
- introduce api for diagnosticsOptions for validating variables json
- fileMatch schema config option allows multiple schemas
- generate jsonSchema in GraphQLWorker
- variables diagnostics happen alongside graphql diagnostics
- monaco-graphql no longer fetches remote schema
- update example to fetch schema
- move schema cache to language service
- initialize -> initializeMode, no longer async
@acao acao force-pushed the feat/monaco-variables-json branch from 80418d2 to 73f9198 Compare December 5, 2021 20:52
@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview December 5, 2021 20:55 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview December 5, 2021 20:55 Inactive
@acao acao force-pushed the feat/monaco-variables-json branch from 73f9198 to ac68098 Compare December 5, 2021 21:05
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview December 5, 2021 21:09 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview December 5, 2021 21:09 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql-webpack-preview December 5, 2021 21:09 Inactive
@acao acao force-pushed the feat/monaco-variables-json branch from ac68098 to 7fb25ac Compare December 5, 2021 22:49
@github-actions github-actions bot temporarily deployed to graphiql-webpack-preview December 5, 2021 22:51 Inactive
@github-actions github-actions bot temporarily deployed to monaco-graphql-webpack-preview December 5, 2021 22:51 Inactive
@github-actions github-actions bot temporarily deployed to graphiql-cdn-preview December 5, 2021 22:51 Inactive
@acao acao merged commit 9df315b into main Dec 5, 2021
@acao acao deleted the feat/monaco-variables-json branch December 5, 2021 22:53
@github-actions github-actions bot mentioned this pull request Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants