From 118a380fa6450b5c6ee9de4b674079e2387c9cea Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Wed, 8 Jun 2022 12:18:05 +0200 Subject: [PATCH 1/3] Disable`fillLeafsOnComplete` by default Users found this generally annoying by default, especially when there are required arguments Without automatically prompting autocompletion of required arguments as well as lead expansion, it makes the extension harder to use You can now supply this in your graphql config: `config.extensions.languageService.fillLeafsOnComplete` Setting it to to `true` will enable this feature. Will soon add the ability to manually enable this in `monaco-graphql` as well. For both, this kind of behavior would be better as a keyboard command, context menu item &/or codelens prompt --- .changeset/khaki-otters-brush.md | 20 +++++++++++++++++++ .../src/GraphQLLanguageService.ts | 7 ++++++- .../interface/getAutocompleteSuggestions.ts | 15 ++++++++------ packages/monaco-graphql/src/typings/index.ts | 6 ++++++ packages/monaco-graphql/src/workerManager.ts | 4 ++++ 5 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 .changeset/khaki-otters-brush.md diff --git a/.changeset/khaki-otters-brush.md b/.changeset/khaki-otters-brush.md new file mode 100644 index 00000000000..7881c43959d --- /dev/null +++ b/.changeset/khaki-otters-brush.md @@ -0,0 +1,20 @@ +--- +'graphql-language-service-server': patch +'monaco-graphql': patch +'vscode-graphql': patch +--- + +Disable`fillLeafsOnComplete` by default + +Users found this generally annoying by default, especially when there are required arguments + +Without automatically prompting autocompletion of required arguments as well as lead expansion, it makes the extension harder to use + +You can now supply this in your graphql config: + +`config.extensions.languageService.fillLeafsOnComplete` + +Setting it to to `true` will enable this feature. +Will soon add the ability to manually enable this in `monaco-graphql` as well. + +For both, this kind of behavior would be better as a keyboard command, context menu item &/or codelens prompt diff --git a/packages/graphql-language-service-server/src/GraphQLLanguageService.ts b/packages/graphql-language-service-server/src/GraphQLLanguageService.ts index 596b45ae3a6..80d93947e1c 100644 --- a/packages/graphql-language-service-server/src/GraphQLLanguageService.ts +++ b/packages/graphql-language-service-server/src/GraphQLLanguageService.ts @@ -262,7 +262,12 @@ export class GraphQLLanguageService { position, undefined, fragmentInfo, - { uri: filePath }, + { + uri: filePath, + fillLeafsOnComplete: + projectConfig?.extensions?.languageService?.fillLeafsOnComplete ?? + false, + }, ); } return []; diff --git a/packages/graphql-language-service/src/interface/getAutocompleteSuggestions.ts b/packages/graphql-language-service/src/interface/getAutocompleteSuggestions.ts index dc62d0d355d..02bf7aa00b9 100644 --- a/packages/graphql-language-service/src/interface/getAutocompleteSuggestions.ts +++ b/packages/graphql-language-service/src/interface/getAutocompleteSuggestions.ts @@ -552,14 +552,17 @@ function getSuggestionsForFieldNames( kind: CompletionItemKind.Field, type: field.type, }; - // TODO: fillLeafs capability - const insertText = getInsertText(field); - if (insertText) { - suggestion.insertText = field.name + insertText; - suggestion.insertTextFormat = InsertTextFormat.Snippet; - suggestion.command = SuggestionCommand; + if (options?.fillLeafsOnComplete) { + // TODO: fillLeafs capability + const insertText = getInsertText(field); + if (insertText) { + suggestion.insertText = field.name + insertText; + suggestion.insertTextFormat = InsertTextFormat.Snippet; + suggestion.command = SuggestionCommand; + } } + return suggestion; }), ); diff --git a/packages/monaco-graphql/src/typings/index.ts b/packages/monaco-graphql/src/typings/index.ts index 9e92ed116fd..6a22e062325 100644 --- a/packages/monaco-graphql/src/typings/index.ts +++ b/packages/monaco-graphql/src/typings/index.ts @@ -115,6 +115,12 @@ export type GraphQLLanguageConfig = { * Custom validation rules following `graphql` `ValidationRule` signature */ customValidationRules?: ValidationRule[]; + /** + * Should field leafs be automatically expanded & filled on autocomplete? + * + * NOTE: this can be annoying with required arguments + */ + fillLeafsOnComplete?: boolean; }; export interface IDisposable { diff --git a/packages/monaco-graphql/src/workerManager.ts b/packages/monaco-graphql/src/workerManager.ts index 0322ce70860..703e44ca1eb 100644 --- a/packages/monaco-graphql/src/workerManager.ts +++ b/packages/monaco-graphql/src/workerManager.ts @@ -80,6 +80,10 @@ export class WorkerManager { schemas: this._defaults.schemas?.map(getStringSchema), externalFragmentDefinitions: this._defaults.externalFragmentDefinitions, + // TODO: make this overrideable + // MonacoAPI possibly another configuration object for this I think? + // all of this could be organized better + fillLeafsOnComplete: false, }, } as ICreateData, }); From 51e7cee0c18fe4e41d3a5dc7238472db30e37f8a Mon Sep 17 00:00:00 2001 From: Rikki Date: Sun, 23 Oct 2022 00:08:00 +0200 Subject: [PATCH 2/3] fix tests, allow passing the configuration --- custom-words.txt | 3 + .../getAutocompleteSuggestions-test.ts | 58 ++++++------------- .../monaco-graphql/src/LanguageService.ts | 5 +- packages/monaco-graphql/src/api.ts | 38 ++++++++++-- packages/monaco-graphql/src/typings/index.ts | 33 +++++++++-- packages/monaco-graphql/src/workerManager.ts | 6 +- 6 files changed, 89 insertions(+), 54 deletions(-) diff --git a/custom-words.txt b/custom-words.txt index a748897d81d..d1fcb087a58 100644 --- a/custom-words.txt +++ b/custom-words.txt @@ -218,6 +218,9 @@ qlapi qlid qlide +// cspell en-us/en-gb edgecases? +behaviour + // other architecting codebases diff --git a/packages/graphql-language-service/src/interface/__tests__/getAutocompleteSuggestions-test.ts b/packages/graphql-language-service/src/interface/__tests__/getAutocompleteSuggestions-test.ts index 187adb1dad1..0237a95527b 100644 --- a/packages/graphql-language-service/src/interface/__tests__/getAutocompleteSuggestions-test.ts +++ b/packages/graphql-language-service/src/interface/__tests__/getAutocompleteSuggestions-test.ts @@ -7,7 +7,10 @@ * */ -import { CompletionItem } from 'graphql-language-service'; +import { + AutocompleteSuggestionOptions, + CompletionItem, +} from 'graphql-language-service'; import fs from 'fs'; import { @@ -20,50 +23,32 @@ import { import { Position } from '../../utils'; import path from 'path'; -import { - getAutocompleteSuggestions, - SuggestionCommand, -} from '../getAutocompleteSuggestions'; - -const commonInsert = { - insertTextFormat: 2, - command: SuggestionCommand, -}; +import { getAutocompleteSuggestions } from '../getAutocompleteSuggestions'; const expectedResults = { droid: { - ...commonInsert, label: 'droid', detail: 'Droid', - insertText: `droid {\n $1\n}`, }, hero: { - ...commonInsert, label: 'hero', detail: 'Character', - insertText: `hero {\n $1\n}`, }, human: { - ...commonInsert, label: 'human', detail: 'Human', - insertText: `human {\n $1\n}`, }, inputTypeTest: { - ...commonInsert, label: 'inputTypeTest', detail: 'TestType', - insertText: `inputTypeTest {\n $1\n}`, }, appearsIn: { label: 'appearsIn', detail: '[Episode]', }, friends: { - ...commonInsert, label: 'friends', detail: '[Character]', - insertText: `friends {\n $1\n}`, }, }; @@ -89,7 +74,7 @@ describe('getAutocompleteSuggestions', () => { query: string, point: Position, externalFragments?: FragmentDefinitionNode[], - uri?: string, + options?: AutocompleteSuggestionOptions, ): Array { return getAutocompleteSuggestions( schema, @@ -97,7 +82,7 @@ describe('getAutocompleteSuggestions', () => { point, undefined, externalFragments, - { uri }, + options, ) .filter( field => !['__schema', '__type'].some(name => name === field.label), @@ -520,7 +505,7 @@ describe('getAutocompleteSuggestions', () => { describe('with SDL types', () => { it('provides correct initial keywords', () => { expect( - testSuggestions('', new Position(0, 0), [], 'schema.graphqls'), + testSuggestions('', new Position(0, 0), [], { uri: 'schema.graphqls' }), ).toEqual([ { label: 'extend' }, { label: 'input' }, @@ -614,12 +599,9 @@ describe('getAutocompleteSuggestions', () => { ])); it('provides correct suggestions on object fields', () => expect( - testSuggestions( - `type Type {\n aField: s`, - new Position(0, 23), - [], - 'schema.graphqls', - ), + testSuggestions(`type Type {\n aField: s`, new Position(0, 23), [], { + uri: 'schema.graphqls', + }), ).toEqual([ { label: 'Episode' }, { label: 'String' }, @@ -629,12 +611,9 @@ describe('getAutocompleteSuggestions', () => { ])); it('provides correct suggestions on object fields that are arrays', () => expect( - testSuggestions( - `type Type {\n aField: []`, - new Position(0, 25), - [], - 'schema.graphqls', - ), + testSuggestions(`type Type {\n aField: []`, new Position(0, 25), [], { + uri: 'schema.graphqls', + }), ).toEqual([ { label: 'AnotherInterface' }, { label: 'Boolean' }, @@ -651,12 +630,9 @@ describe('getAutocompleteSuggestions', () => { ])); it('provides correct suggestions on input object fields', () => expect( - testSuggestions( - `input Type {\n aField: s`, - new Position(0, 23), - [], - 'schema.graphqls', - ), + testSuggestions(`input Type {\n aField: s`, new Position(0, 23), [], { + uri: 'schema.graphqls', + }), ).toEqual([{ label: 'Episode' }, { label: 'String' }])); it('provides correct directive suggestions on args definitions', () => expect( diff --git a/packages/monaco-graphql/src/LanguageService.ts b/packages/monaco-graphql/src/LanguageService.ts index ac2c983bfcb..f585e508270 100644 --- a/packages/monaco-graphql/src/LanguageService.ts +++ b/packages/monaco-graphql/src/LanguageService.ts @@ -50,12 +50,14 @@ export class LanguageService { private _externalFragmentDefinitionNodes: FragmentDefinitionNode[] | null = null; private _externalFragmentDefinitionsString: string | null = null; + private _fillLeafsOnComplete?: boolean = false; constructor({ parser, schemas, parseOptions, externalFragmentDefinitions, customValidationRules, + fillLeafsOnComplete, }: GraphQLLanguageConfig) { this._schemaLoader = defaultSchemaLoader; if (schemas) { @@ -65,6 +67,7 @@ export class LanguageService { if (parser) { this._parser = parser; } + this._fillLeafsOnComplete = fillLeafsOnComplete; if (parseOptions) { this._parseOptions = parseOptions; @@ -213,7 +216,7 @@ export class LanguageService { position, undefined, this.getExternalFragmentDefinitions(), - { uri }, + { uri, fillLeafsOnComplete: this._fillLeafsOnComplete }, ); }; /** diff --git a/packages/monaco-graphql/src/api.ts b/packages/monaco-graphql/src/api.ts index 92fe3616519..dc3b574f9f3 100644 --- a/packages/monaco-graphql/src/api.ts +++ b/packages/monaco-graphql/src/api.ts @@ -10,6 +10,7 @@ import { Emitter } from 'monaco-editor'; import type { IEvent } from 'monaco-editor'; import type { FragmentDefinitionNode, GraphQLSchema } from 'graphql'; import type { + CompletionSettings, DiagnosticSettings, FormattingOptions, ModeConfiguration, @@ -23,6 +24,7 @@ export type MonacoGraphQLAPIOptions = { modeConfiguration: ModeConfiguration; formattingOptions: FormattingOptions; diagnosticSettings: DiagnosticSettings; + completionSettings: CompletionSettings; }; export type SchemaEntry = { @@ -33,9 +35,10 @@ export type SchemaEntry = { export class MonacoGraphQLAPI { private _onDidChange = new Emitter(); - private _formattingOptions!: FormattingOptions; - private _modeConfiguration!: ModeConfiguration; - private _diagnosticSettings!: DiagnosticSettings; + private _formattingOptions: FormattingOptions; + private _modeConfiguration: ModeConfiguration; + private _diagnosticSettings: DiagnosticSettings; + private _completionSettings: CompletionSettings; private _schemas: SchemaConfig[] | null = null; private _schemasById: Record = Object.create(null); private _languageId: string; @@ -50,15 +53,17 @@ export class MonacoGraphQLAPI { modeConfiguration, formattingOptions, diagnosticSettings, + completionSettings, }: MonacoGraphQLAPIOptions) { this._languageId = languageId; if (schemas) { this.setSchemaConfig(schemas); } - this.setModeConfiguration(modeConfiguration); - this.setFormattingOptions(formattingOptions); - this.setDiagnosticSettings(diagnosticSettings); + this._modeConfiguration = modeConfiguration ?? modeConfigurationDefault; + this._completionSettings = completionSettings ?? completionSettingDefault; + this._diagnosticSettings = diagnosticSettings ?? diagnosticSettingDefault; + this._formattingOptions = formattingOptions ?? formattingDefaults; } public get onDidChange(): IEvent { @@ -85,6 +90,9 @@ export class MonacoGraphQLAPI { public get diagnosticSettings(): DiagnosticSettings { return this._diagnosticSettings; } + public get completionSettings(): CompletionSettings { + return this._completionSettings; + } public get externalFragmentDefinitions() { return this._externalFragmentDefinitions; } @@ -123,6 +131,11 @@ export class MonacoGraphQLAPI { this._diagnosticSettings = diagnosticSettings || Object.create(null); this._onDidChange.fire(this); } + + public setCompletionSettings(completionSettings: CompletionSettings): void { + this._completionSettings = completionSettings || Object.create(null); + this._onDidChange.fire(this); + } } export function create( @@ -132,9 +145,11 @@ export function create( if (!config) { return new MonacoGraphQLAPI({ languageId, + schemas: [], formattingOptions: formattingDefaults, modeConfiguration: modeConfigurationDefault, diagnosticSettings: diagnosticSettingDefault, + completionSettings: completionSettingDefault, }); } else { const { @@ -142,6 +157,7 @@ export function create( formattingOptions, modeConfiguration, diagnosticSettings, + completionSettings, } = config; return new MonacoGraphQLAPI({ languageId, @@ -162,6 +178,10 @@ export function create( ...diagnosticSettingDefault, ...diagnosticSettings, }, + completionSettings: { + ...completionSettingDefault, + ...completionSettings, + }, }); } } @@ -181,6 +201,8 @@ export const modeConfigurationDefault: Required = { export const formattingDefaults: FormattingOptions = { prettierConfig: { + // rationale? a11y. + // https://adamtuttle.codes/blog/2021/tabs-vs-spaces-its-an-accessibility-issue/ tabWidth: 2, }, }; @@ -190,3 +212,7 @@ export const diagnosticSettingDefault: DiagnosticSettings = { schemaValidation: 'error', }, }; + +export const completionSettingDefault: CompletionSettings = { + _experimental_fillLeafsOnComplete: false, +}; diff --git a/packages/monaco-graphql/src/typings/index.ts b/packages/monaco-graphql/src/typings/index.ts index 6a22e062325..6e2ac884c69 100644 --- a/packages/monaco-graphql/src/typings/index.ts +++ b/packages/monaco-graphql/src/typings/index.ts @@ -212,17 +212,34 @@ export type DiagnosticSettings = { jsonDiagnosticSettings?: languages.json.DiagnosticsOptions; }; +export type CompletionSettings = { + /** + * EXPERIMENTAL: Automatically fill required leaf nodes recursively + * upon triggering code completion events. + * + * + * - [x] fills required nodes + * - [x] automatically expands relay-style node/edge fields + * - [ ] automatically jumps to first required argument field + * - then, continues to prompt for required argument fields + * - (fixing this will make it non-experimental) + * - when it runs out of arguments, or you choose `{` as a completion option + * that appears when all required arguments are supplied, the argument + * selection closes `)` and the leaf field expands again `{ \n| }` + */ + _experimental_fillLeafsOnComplete?: boolean; +}; + /** * Configuration to initialize the editor with */ export type MonacoGraphQLInitializeConfig = { /** - * Specify array of `SchemaConfig` items used to initialize the `GraphQLWorker` if available. - * You can also `api.setSchemaConfig()` after instantiating the mode. + * custom (experimental) settings for autocompletion behaviour */ - schemas?: SchemaConfig[]; + completionSettings?: CompletionSettings; /** - * + * custom settings for diagnostics (validation) */ diagnosticSettings?: DiagnosticSettings; /** @@ -235,7 +252,15 @@ export type MonacoGraphQLInitializeConfig = { * ``` */ formattingOptions?: FormattingOptions; + /** + * Generic monaco language mode options, same as for the official monaco json mode + */ modeConfiguration?: ModeConfiguration; + /** + * Specify array of `SchemaConfig` items used to initialize the `GraphQLWorker` if available. + * You can also `api.setSchemaConfig()` after instantiating the mode. + */ + schemas?: SchemaConfig[]; }; export interface ICreateData { diff --git a/packages/monaco-graphql/src/workerManager.ts b/packages/monaco-graphql/src/workerManager.ts index 703e44ca1eb..4278dbc0e75 100644 --- a/packages/monaco-graphql/src/workerManager.ts +++ b/packages/monaco-graphql/src/workerManager.ts @@ -80,10 +80,12 @@ export class WorkerManager { schemas: this._defaults.schemas?.map(getStringSchema), externalFragmentDefinitions: this._defaults.externalFragmentDefinitions, - // TODO: make this overrideable + // TODO: make this overridable // MonacoAPI possibly another configuration object for this I think? // all of this could be organized better - fillLeafsOnComplete: false, + fillLeafsOnComplete: + this._defaults.completionSettings + ._experimental_fillLeafsOnComplete, }, } as ICreateData, }); From e385cc0612fdae5d3e0db426e9f2cdf84049b86b Mon Sep 17 00:00:00 2001 From: Rikki Date: Sun, 23 Oct 2022 02:46:23 +0200 Subject: [PATCH 3/3] react-like experiment flagging --- packages/monaco-graphql/README.md | 14 +++++++++++++- packages/monaco-graphql/src/api.ts | 2 +- packages/monaco-graphql/src/typings/index.ts | 2 +- packages/monaco-graphql/src/workerManager.ts | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/monaco-graphql/README.md b/packages/monaco-graphql/README.md index 2d521623da8..652cd23b140 100644 --- a/packages/monaco-graphql/README.md +++ b/packages/monaco-graphql/README.md @@ -208,7 +208,14 @@ MonacoGraphQLAPI.setDiagnosticSettings({ allowComments: true, // allow json, parse with a jsonc parser to make requests }, }); -// TODO: document manual alternative approach + +MonacoGraphQL.setCompletionSettings({ + // this auto-fills NonNull leaf fields + // it used to be on by default, but is annoying when + // fields contain required argument. + // hoping to fix that soon! + __experimental__fillLeafsOnComplete: true, +}); ``` You can also experiment with the built-in I think `jsonc`? (MSFT json5-ish syntax, for `tsconfig.json` etc.) and the 3rd party `monaco-yaml` language modes for completion of other types of variable input. you can also experiment with editor methods to parse detected input into different formats, etc (`yaml` pastes as `json`, etc.) @@ -471,6 +478,11 @@ If you are familiar with Codemirror/Atom-era terminology and features, here's so - [Monaco Editor API Docs](https://microsoft.github.io/monaco-editor/api/index.html) - [Monaco Editor Samples](https://github.com/Microsoft/monaco-editor-samples) repository is great for tips on implementing with different bundlers, runtimes, etc. +## Inspiration + +`microsoft/monaco-json` was our inspiration from the outset, when it was still a standalone repository. @acao actually wholesale copied many files, you could +almost say it was a fork! + ## TODO - [x] variables JSON validation diff --git a/packages/monaco-graphql/src/api.ts b/packages/monaco-graphql/src/api.ts index dc3b574f9f3..98a33d73921 100644 --- a/packages/monaco-graphql/src/api.ts +++ b/packages/monaco-graphql/src/api.ts @@ -214,5 +214,5 @@ export const diagnosticSettingDefault: DiagnosticSettings = { }; export const completionSettingDefault: CompletionSettings = { - _experimental_fillLeafsOnComplete: false, + __experimental__fillLeafsOnComplete: false, }; diff --git a/packages/monaco-graphql/src/typings/index.ts b/packages/monaco-graphql/src/typings/index.ts index 6e2ac884c69..8c9cd9c835a 100644 --- a/packages/monaco-graphql/src/typings/index.ts +++ b/packages/monaco-graphql/src/typings/index.ts @@ -227,7 +227,7 @@ export type CompletionSettings = { * that appears when all required arguments are supplied, the argument * selection closes `)` and the leaf field expands again `{ \n| }` */ - _experimental_fillLeafsOnComplete?: boolean; + __experimental__fillLeafsOnComplete?: boolean; }; /** diff --git a/packages/monaco-graphql/src/workerManager.ts b/packages/monaco-graphql/src/workerManager.ts index 4278dbc0e75..9b77f71be39 100644 --- a/packages/monaco-graphql/src/workerManager.ts +++ b/packages/monaco-graphql/src/workerManager.ts @@ -85,7 +85,7 @@ export class WorkerManager { // all of this could be organized better fillLeafsOnComplete: this._defaults.completionSettings - ._experimental_fillLeafsOnComplete, + .__experimental__fillLeafsOnComplete, }, } as ICreateData, });