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

DisablefillLeafsOnComplete by default #2488

Merged
merged 3 commits into from
Nov 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/khaki-otters-brush.md
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions custom-words.txt
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ qlapi
qlid
qlide

// cspell en-us/en-gb edgecases?
behaviour

// other
architecting
codebases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,12 @@ export class GraphQLLanguageService {
position,
undefined,
fragmentInfo,
{ uri: filePath },
{
uri: filePath,
fillLeafsOnComplete:
projectConfig?.extensions?.languageService?.fillLeafsOnComplete ??
false,
},
);
}
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
*
*/

import { CompletionItem } from 'graphql-language-service';
import {
AutocompleteSuggestionOptions,
CompletionItem,
} from 'graphql-language-service';

import fs from 'fs';
import {
Expand All @@ -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}`,
},
};

Expand All @@ -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),
Expand Down Expand Up @@ -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' },
Expand Down Expand Up @@ -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' },
Expand All @@ -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' },
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}),
);
Expand Down
14 changes: 13 additions & 1 deletion packages/monaco-graphql/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion packages/monaco-graphql/src/LanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -65,6 +67,7 @@ export class LanguageService {
if (parser) {
this._parser = parser;
}
this._fillLeafsOnComplete = fillLeafsOnComplete;

if (parseOptions) {
this._parseOptions = parseOptions;
Expand Down Expand Up @@ -213,7 +216,7 @@ export class LanguageService {
position,
undefined,
this.getExternalFragmentDefinitions(),
{ uri },
{ uri, fillLeafsOnComplete: this._fillLeafsOnComplete },
);
};
/**
Expand Down
38 changes: 32 additions & 6 deletions packages/monaco-graphql/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,6 +24,7 @@ export type MonacoGraphQLAPIOptions = {
modeConfiguration: ModeConfiguration;
formattingOptions: FormattingOptions;
diagnosticSettings: DiagnosticSettings;
completionSettings: CompletionSettings;
};

export type SchemaEntry = {
Expand All @@ -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;
Expand All @@ -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> {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -162,6 +178,10 @@ export function create(
...diagnosticSettingDefault,
...diagnosticSettings,
},
completionSettings: {
...completionSettingDefault,
...completionSettings,
},
});
}
}
Expand All @@ -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,
},
};
Expand All @@ -190,3 +212,7 @@ export const diagnosticSettingDefault: DiagnosticSettings = {
schemaValidation: 'error',
},
};

export const completionSettingDefault: CompletionSettings = {
__experimental__fillLeafsOnComplete: false,
};
Loading