Skip to content

Commit f82bd7a

Browse files
authored
fix: a few major bugs with SDL and remote schema in the language server (#2055)
1 parent 2e349a9 commit f82bd7a

File tree

9 files changed

+134
-59
lines changed

9 files changed

+134
-59
lines changed

.changeset/cuddly-games-protect.md

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'graphql-language-service-cli': patch
3+
'graphql-language-service-server': patch
4+
---
5+
6+
this fixes the URI scheme related bugs and make sure schema as sdl config works again.
7+
8+
`fileURLToPath` had been introduced by a contributor and I didnt test properly, it broke sdl file loading!
9+
10+
definitions, autocomplete, diagnostics, etc should work again
11+
also hides the more verbose logging output for now

packages/graphql-language-service-server/README.md

+12-11
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ await startServer({
117117
// rootDir is same as `configDir` before, the path where the graphql config file would be found by cosmic-config
118118
rootDir: 'config/',
119119
// or - the relative or absolute path to your file
120-
filePath: 'exact/path/to/config.js (also supports yml, json)',
120+
filePath: 'exact/path/to/config.js' // (also supports yml, json, ts, toml)
121121
// myPlatform.config.js/json/yaml works now!
122122
configName: 'myPlatform',
123123
},
@@ -134,8 +134,9 @@ module.exports = {
134134
// a function that returns rules array with parameter `ValidationContext` from `graphql/validation`
135135
"customValidationRules": require('./config/customValidationRules')
136136
"languageService": {
137-
// should the language service read from source files? if false, it generates a schema from the project/config schema
138-
useSchemaFileDefinitions: false
137+
// should the language service read schema for lookups from a cached file based on graphql config output?
138+
cacheSchemaFileForLookup: true
139+
// NOTE: this will disable all definition lookup for local SDL files
139140
}
140141
}
141142
}
@@ -147,14 +148,14 @@ we also load `require('dotenv').config()`, so you can use process.env variables
147148

148149
The LSP Server reads config by sending `workspace/configuration` method when it initializes.
149150

150-
| Parameter | Default | Description |
151-
| ---------------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------------------------ |
152-
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
153-
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
154-
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
155-
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
156-
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
157-
| `vsode-graphql.useSchemaFileDefinitions` | `false` | whether the LSP server will use source files, or generate an SDL from `config.schema`/`project.schema` |
151+
| Parameter | Default | Description |
152+
| ---------------------------------------- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
153+
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
154+
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
155+
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
156+
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
157+
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
158+
| `vsode-graphql.cacheSchemaFileForLookup` | `false` | generate an SDL file based on your graphql-config schema configuration for schema definition lookup and other features. useful when your `schema` config are urls |
158159

159160
all the `graphql-config.load.*` configuration values come from static `loadConfig()` options in graphql config.
160161

packages/graphql-language-service-server/package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@
3232
"dependencies": {
3333
"@babel/parser": "^7.13.13",
3434
"dotenv": "8.2.0",
35-
"glob": "^7.1.2",
3635
"graphql-config": "^4.1.0",
3736
"graphql-language-service": "^3.2.3",
3837
"graphql-language-service-utils": "^2.6.2",
3938
"mkdirp": "^1.0.4",
4039
"node-fetch": "^2.6.1",
4140
"nullthrows": "^1.0.0",
4241
"vscode-jsonrpc": "^5.0.1",
43-
"vscode-languageserver": "^6.1.1"
42+
"vscode-languageserver": "^6.1.1",
43+
"fast-glob": "^3.2.7"
4444
},
4545
"devDependencies": {
4646
"@types/mkdirp": "^1.0.1",

packages/graphql-language-service-server/src/GraphQLCache.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export class GraphQLCache implements GraphQLCacheInterface {
106106
getGraphQLConfig = (): GraphQLConfig => this._graphQLConfig;
107107

108108
getProjectForFile = (uri: string): GraphQLProjectConfig => {
109-
return this._graphQLConfig.getProjectForFile(fileURLToPath(uri));
109+
return this._graphQLConfig.getProjectForFile(new URL(uri).pathname);
110110
};
111111

112112
getFragmentDependencies = async (

packages/graphql-language-service-server/src/Logger.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,12 @@ export class Logger implements VSCodeLogger {
7171
const logMessage = `${timestamp} [${severity}] (pid: ${pid}) graphql-language-service-usage-logs: ${stringMessage}\n`;
7272
// write to the file in tmpdir
7373
fs.appendFile(this._logFilePath, logMessage, _error => {});
74-
this._getOutputStream(severity).write(logMessage, err => {
75-
err && console.error(err);
76-
});
74+
// @TODO: enable with debugging
75+
if (severityKey !== SEVERITY.Hint) {
76+
this._getOutputStream(severity).write(logMessage, err => {
77+
err && console.error(err);
78+
});
79+
}
7780
}
7881

7982
_getOutputStream(severity: DiagnosticSeverity): Socket {

packages/graphql-language-service-server/src/MessageProcessor.ts

+77-32
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99

1010
import mkdirp from 'mkdirp';
1111
import { readFileSync, existsSync, writeFileSync, writeFile } from 'fs';
12-
import { fileURLToPath, pathToFileURL } from 'url';
12+
import { pathToFileURL } from 'url';
1313
import * as path from 'path';
14+
import glob from 'fast-glob';
1415
import {
1516
CachedContent,
1617
Uri,
@@ -191,7 +192,7 @@ export class MessageProcessor {
191192
throw new Error('GraphQL Language Server is not initialized.');
192193
}
193194

194-
this._logger.log(
195+
this._logger.info(
195196
JSON.stringify({
196197
type: 'usage',
197198
messageType: 'initialize',
@@ -584,7 +585,7 @@ export class MessageProcessor {
584585
) {
585586
const uri = change.uri;
586587

587-
const text = readFileSync(fileURLToPath(uri), { encoding: 'utf8' });
588+
const text = readFileSync(new URL(uri).pathname).toString();
588589
const contents = this._parser(text, uri);
589590

590591
await this._updateFragmentDefinition(uri, contents);
@@ -863,37 +864,83 @@ export class MessageProcessor {
863864
return path.resolve(projectTmpPath);
864865
}
865866
}
867+
/**
868+
* Safely attempts to cache schema files based on a glob or path
869+
* Exits without warning in several cases because these strings can be almost
870+
* anything!
871+
* @param uri
872+
* @param project
873+
*/
874+
async _cacheSchemaPath(uri: string, project: GraphQLProjectConfig) {
875+
try {
876+
const files = await glob(uri);
877+
if (files && files.length > 0) {
878+
await Promise.all(
879+
files.map(uriPath => this._cacheSchemaFile(uriPath, project)),
880+
);
881+
} else {
882+
try {
883+
this._cacheSchemaFile(uri, project);
884+
} catch (err) {
885+
// this string may be an SDL string even, how do we even evaluate this? haha
886+
}
887+
}
888+
} catch (err) {}
889+
}
890+
async _cacheObjectSchema(
891+
pointer: { [key: string]: any },
892+
project: GraphQLProjectConfig,
893+
) {
894+
await Promise.all(
895+
Object.keys(pointer).map(async schemaUri =>
896+
this._cacheSchemaPath(schemaUri, project),
897+
),
898+
);
899+
}
900+
async _cacheArraySchema(
901+
pointers: UnnormalizedTypeDefPointer[],
902+
project: GraphQLProjectConfig,
903+
) {
904+
await Promise.all(
905+
pointers.map(async schemaEntry => {
906+
if (typeof schemaEntry === 'string') {
907+
await this._cacheSchemaPath(schemaEntry, project);
908+
} else if (schemaEntry) {
909+
await this._cacheObjectSchema(schemaEntry, project);
910+
}
911+
}),
912+
);
913+
}
866914

867915
async _cacheSchemaFilesForProject(project: GraphQLProjectConfig) {
868916
const schema = project?.schema;
869917
const config = project?.extensions?.languageService;
870918
/**
871-
* By default, let's only cache the full graphql config schema.
872-
* This allows us to rely on graphql-config's schema building features
873-
* And our own cache implementations
874-
* This prefers schema instead of SDL first schema development, but will still
875-
* work with lookup of the actual .graphql schema files if the option is enabled,
876-
* however results may vary.
919+
* By default, we look for schema definitions in SDL files
920+
*
921+
* with the opt-in feature `cacheSchemaOutputFileForLookup` enabled,
922+
* the resultant `graphql-config` .getSchema() schema output will be cached
923+
* locally and available as a single file for definition lookup and peek
877924
*
878-
* The default temporary schema file seems preferrable until we have graphql source maps
925+
* this is helpful when your `graphql-config` `schema` input is:
926+
* - a remote or local URL
927+
* - compiled from graphql files and code sources
928+
* - otherwise where you don't have schema SDL in the codebase or don't want to use it for lookup
929+
*
930+
* it is disabled by default
879931
*/
880-
const useSchemaFileDefinitions =
881-
(config?.useSchemaFileDefinitions ??
882-
this?._settings?.useSchemaFileDefinitions) ||
932+
const cacheSchemaFileForLookup =
933+
config?.cacheSchemaFileForLookup ??
934+
this?._settings?.cacheSchemaFileForLookup ??
883935
false;
884-
if (!useSchemaFileDefinitions) {
936+
if (cacheSchemaFileForLookup) {
885937
await this._cacheConfigSchema(project);
886-
} else {
887-
if (Array.isArray(schema)) {
888-
Promise.all(
889-
schema.map(async (uri: UnnormalizedTypeDefPointer) => {
890-
await this._cacheSchemaFile(uri, project);
891-
}),
892-
);
893-
} else {
894-
const uri = schema.toString();
895-
await this._cacheSchemaFile(uri, project);
896-
}
938+
} else if (typeof schema === 'string') {
939+
await this._cacheSchemaPath(schema, project);
940+
} else if (Array.isArray(schema)) {
941+
await this._cacheArraySchema(schema, project);
942+
} else if (schema) {
943+
await this._cacheObjectSchema(schema, project);
897944
}
898945
}
899946
/**
@@ -995,7 +1042,7 @@ export class MessageProcessor {
9951042
if (config?.projects) {
9961043
return Promise.all(
9971044
Object.keys(config.projects).map(async projectName => {
998-
const project = await config.getProject(projectName);
1045+
const project = config.getProject(projectName);
9991046
await this._cacheSchemaFilesForProject(project);
10001047
await this._cacheDocumentFilesforProject(project);
10011048
}),
@@ -1057,13 +1104,11 @@ export class MessageProcessor {
10571104
contents,
10581105
});
10591106
}
1060-
} else if (textDocument?.version) {
1061-
return this._textDocumentCache.set(uri, {
1062-
version: textDocument.version,
1063-
contents,
1064-
});
10651107
}
1066-
return null;
1108+
return this._textDocumentCache.set(uri, {
1109+
version: textDocument.version ?? 0,
1110+
contents,
1111+
});
10671112
}
10681113
}
10691114

packages/graphql-language-service-server/src/__tests__/.graphqlrc.yml

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ projects:
33
schema:
44
- __schema__/StarWarsSchema.graphql
55
- 'directive @customDirective on FRAGMENT_SPREAD'
6+
testWithGlobSchema:
7+
schema:
8+
- __schema__/*.graphql
9+
- 'directive @customDirective on FRAGMENT_SPREAD'
610
testWithEndpoint:
711
schema: https://example.com/graphql
812
testWithEndpointAndSchema:

packages/graphql-language-service-server/src/__tests__/Logger-test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('Logger', () => {
3030

3131
it('logs to stdout', () => {
3232
const logger = new Logger(tmpdir());
33-
logger.log('log test');
33+
logger.info('log test');
3434

3535
expect(mockedStdoutWrite.mock.calls.length).toBe(1);
3636
expect(mockedStdoutWrite.mock.calls[0][0]).toContain('log test');
@@ -51,14 +51,14 @@ describe('Logger', () => {
5151
const logger = new Logger(tmpdir(), stderrOnly);
5252
logger.info('info test');
5353
logger.warn('warn test');
54+
// log is only logged to file now :)
5455
logger.log('log test');
5556
logger.error('error test');
5657

5758
expect(mockedStdoutWrite.mock.calls.length).toBe(0);
58-
expect(mockedStderrWrite.mock.calls.length).toBe(4);
59+
expect(mockedStderrWrite.mock.calls.length).toBe(3);
5960
expect(mockedStderrWrite.mock.calls[0][0]).toContain('info test');
6061
expect(mockedStderrWrite.mock.calls[1][0]).toContain('warn test');
61-
expect(mockedStderrWrite.mock.calls[2][0]).toContain('log test');
62-
expect(mockedStderrWrite.mock.calls[3][0]).toContain('error test');
62+
expect(mockedStderrWrite.mock.calls[2][0]).toContain('error test');
6363
});
6464
});

yarn.lock

+17-6
Original file line numberDiff line numberDiff line change
@@ -3270,7 +3270,7 @@
32703270
tslib "^2"
32713271

32723272
"@graphiql/toolkit@file:packages/graphiql-toolkit":
3273-
version "0.4.1"
3273+
version "0.4.2"
32743274
dependencies:
32753275
"@n1ru4l/push-pull-async-iterable-iterator" "^3.1.0"
32763276
meros "^1.1.4"
@@ -10014,6 +10014,17 @@ fast-glob@^3.1.1:
1001410014
micromatch "^4.0.2"
1001510015
picomatch "^2.2.1"
1001610016

10017+
fast-glob@^3.2.7:
10018+
version "3.2.7"
10019+
resolved "https://registry.yarnpkg.com/fast-glob/-/fast-glob-3.2.7.tgz#fd6cb7a2d7e9aa7a7846111e85a196d6b2f766a1"
10020+
integrity sha512-rYGMRwip6lUMvYD3BTScMwT1HtAs2d71SMv66Vrxs0IekGZEjhM0pcMfjQPnknBt2zeCwQMEupiN02ZP4DiT1Q==
10021+
dependencies:
10022+
"@nodelib/fs.stat" "^2.0.2"
10023+
"@nodelib/fs.walk" "^1.2.3"
10024+
glob-parent "^5.1.2"
10025+
merge2 "^1.3.0"
10026+
micromatch "^4.0.4"
10027+
1001710028
[email protected], fast-json-stable-stringify@^2.0.0:
1001810029
version "2.1.0"
1001910030
resolved "https://registry.yarnpkg.com/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz#874bf69c6f404c2b5d99c481341399fd55892633"
@@ -10744,7 +10755,7 @@ glob-parent@^5.0.0, glob-parent@~5.1.0:
1074410755
dependencies:
1074510756
is-glob "^4.0.1"
1074610757

10747-
glob-parent@^5.1.0:
10758+
glob-parent@^5.1.0, glob-parent@^5.1.2:
1074810759
version "5.1.2"
1074910760
resolved "https://registry.yarnpkg.com/glob-parent/-/glob-parent-5.1.2.tgz#869832c58034fe68a4093c17dc15e8340d8401c4"
1075010761
integrity sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==
@@ -10992,16 +11003,16 @@ grapheme-splitter@^1.0.4:
1099211003
integrity sha512-bzh50DW9kTPM00T8y4o8vQg89Di9oLJVLW/KaOGIXJWP/iqCN6WKYkbNOF04vFLJhwcpYUh9ydh/+5vpOqV4YQ==
1099311004

1099411005
"graphiql@file:packages/graphiql":
10995-
version "1.5.2"
11006+
version "1.5.5"
1099611007
dependencies:
10997-
"@graphiql/toolkit" "^0.4.1"
11008+
"@graphiql/toolkit" "^0.4.2"
1099811009
codemirror "^5.58.2"
10999-
codemirror-graphql "^1.2.0"
11010+
codemirror-graphql "^1.2.3"
1100011011
copy-to-clipboard "^3.2.0"
1100111012
dset "^3.1.0"
1100211013
entities "^2.0.0"
1100311014
escape-html "^1.0.3"
11004-
graphql-language-service "^3.2.1"
11015+
graphql-language-service "^3.2.3"
1100511016
markdown-it "^12.2.0"
1100611017

1100711018
graphql-config@^4.1.0:

0 commit comments

Comments
 (0)