DisablefillLeafsOnComplete by default (#2488)

* 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 * fix tests, allow passing the configuration * react-like experiment flagging
graphql · Nov 3, 2022 · 967006a · 967006a
1 parent 9b98c1b
commit 967006a
Show file tree

Hide file tree

Showing 10 changed files with 145 additions and 60 deletions.
diff --git a/.changeset/khaki-otters-brush.md 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/custom-words.txt b/custom-words.txt
@@ -222,6 +222,9 @@ qlapi
 qlid
 qlide
 
+// cspell en-us/en-gb edgecases?
+behaviour
+
 // other
 architecting
 codebases

diff --git 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/__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,15 +74,15 @@ describe('getAutocompleteSuggestions', () => {
     query: string,
     point: Position,
     externalFragments?: FragmentDefinitionNode[],
-    uri?: string,
+    options?: AutocompleteSuggestionOptions,
   ): Array<CompletionItem> {
     return getAutocompleteSuggestions(
       schema,
       query,
       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/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/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/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
@@ -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<MonacoGraphQLAPI>();
-  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<string, SchemaConfig> = 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<MonacoGraphQLAPI> {
@@ -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,16 +145,19 @@ export function create(
   if (!config) {
     return new MonacoGraphQLAPI({
       languageId,
+      schemas: [],
       formattingOptions: formattingDefaults,
       modeConfiguration: modeConfigurationDefault,
       diagnosticSettings: diagnosticSettingDefault,
+      completionSettings: completionSettingDefault,
     });
   } else {
     const {
       schemas,
       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<ModeConfiguration> = {
 
 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,
+};