Skip to content

Commit

Permalink
fix: a few major bugs with SDL and remote schema in the language serv…
Browse files Browse the repository at this point in the history
…er (#2055)
  • Loading branch information
acao committed Nov 25, 2021
1 parent 2e349a9 commit f82bd7a
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 59 deletions.
11 changes: 11 additions & 0 deletions .changeset/cuddly-games-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'graphql-language-service-cli': patch
'graphql-language-service-server': patch
---

this fixes the URI scheme related bugs and make sure schema as sdl config works again.

`fileURLToPath` had been introduced by a contributor and I didnt test properly, it broke sdl file loading!

definitions, autocomplete, diagnostics, etc should work again
also hides the more verbose logging output for now
23 changes: 12 additions & 11 deletions packages/graphql-language-service-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ await startServer({
// rootDir is same as `configDir` before, the path where the graphql config file would be found by cosmic-config
rootDir: 'config/',
// or - the relative or absolute path to your file
filePath: 'exact/path/to/config.js (also supports yml, json)',
filePath: 'exact/path/to/config.js' // (also supports yml, json, ts, toml)
// myPlatform.config.js/json/yaml works now!
configName: 'myPlatform',
},
Expand All @@ -134,8 +134,9 @@ module.exports = {
// a function that returns rules array with parameter `ValidationContext` from `graphql/validation`
"customValidationRules": require('./config/customValidationRules')
"languageService": {
// should the language service read from source files? if false, it generates a schema from the project/config schema
useSchemaFileDefinitions: false
// should the language service read schema for lookups from a cached file based on graphql config output?
cacheSchemaFileForLookup: true
// NOTE: this will disable all definition lookup for local SDL files
}
}
}
Expand All @@ -147,14 +148,14 @@ we also load `require('dotenv').config()`, so you can use process.env variables

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

| Parameter | Default | Description |
| ---------------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------------------------ |
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
| `vsode-graphql.useSchemaFileDefinitions` | `false` | whether the LSP server will use source files, or generate an SDL from `config.schema`/`project.schema` |
| Parameter | Default | Description |
| ---------------------------------------- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `graphql-config.load.baseDir` | workspace root or process.cwd() | the path where graphql config looks for config files |
| `graphql-config.load.filePath` | `null` | exact filepath of the config file. |
| `graphql-config.load.configName` | `graphql` | config name prefix instead of `graphql` |
| `graphql-config.load.legacy` | `true` | backwards compatibility with `graphql-config@2` |
| `graphql-config.dotEnvPath` | `null` | backwards compatibility with `graphql-config@2` |
| `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 |

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

Expand Down
4 changes: 2 additions & 2 deletions packages/graphql-language-service-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@
"dependencies": {
"@babel/parser": "^7.13.13",
"dotenv": "8.2.0",
"glob": "^7.1.2",
"graphql-config": "^4.1.0",
"graphql-language-service": "^3.2.3",
"graphql-language-service-utils": "^2.6.2",
"mkdirp": "^1.0.4",
"node-fetch": "^2.6.1",
"nullthrows": "^1.0.0",
"vscode-jsonrpc": "^5.0.1",
"vscode-languageserver": "^6.1.1"
"vscode-languageserver": "^6.1.1",
"fast-glob": "^3.2.7"
},
"devDependencies": {
"@types/mkdirp": "^1.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class GraphQLCache implements GraphQLCacheInterface {
getGraphQLConfig = (): GraphQLConfig => this._graphQLConfig;

getProjectForFile = (uri: string): GraphQLProjectConfig => {
return this._graphQLConfig.getProjectForFile(fileURLToPath(uri));
return this._graphQLConfig.getProjectForFile(new URL(uri).pathname);
};

getFragmentDependencies = async (
Expand Down
9 changes: 6 additions & 3 deletions packages/graphql-language-service-server/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ export class Logger implements VSCodeLogger {
const logMessage = `${timestamp} [${severity}] (pid: ${pid}) graphql-language-service-usage-logs: ${stringMessage}\n`;
// write to the file in tmpdir
fs.appendFile(this._logFilePath, logMessage, _error => {});
this._getOutputStream(severity).write(logMessage, err => {
err && console.error(err);
});
// @TODO: enable with debugging
if (severityKey !== SEVERITY.Hint) {
this._getOutputStream(severity).write(logMessage, err => {
err && console.error(err);
});
}
}

_getOutputStream(severity: DiagnosticSeverity): Socket {
Expand Down
109 changes: 77 additions & 32 deletions packages/graphql-language-service-server/src/MessageProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

import mkdirp from 'mkdirp';
import { readFileSync, existsSync, writeFileSync, writeFile } from 'fs';
import { fileURLToPath, pathToFileURL } from 'url';
import { pathToFileURL } from 'url';
import * as path from 'path';
import glob from 'fast-glob';
import {
CachedContent,
Uri,
Expand Down Expand Up @@ -191,7 +192,7 @@ export class MessageProcessor {
throw new Error('GraphQL Language Server is not initialized.');
}

this._logger.log(
this._logger.info(
JSON.stringify({
type: 'usage',
messageType: 'initialize',
Expand Down Expand Up @@ -584,7 +585,7 @@ export class MessageProcessor {
) {
const uri = change.uri;

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

await this._updateFragmentDefinition(uri, contents);
Expand Down Expand Up @@ -863,37 +864,83 @@ export class MessageProcessor {
return path.resolve(projectTmpPath);
}
}
/**
* Safely attempts to cache schema files based on a glob or path
* Exits without warning in several cases because these strings can be almost
* anything!
* @param uri
* @param project
*/
async _cacheSchemaPath(uri: string, project: GraphQLProjectConfig) {
try {
const files = await glob(uri);
if (files && files.length > 0) {
await Promise.all(
files.map(uriPath => this._cacheSchemaFile(uriPath, project)),
);
} else {
try {
this._cacheSchemaFile(uri, project);
} catch (err) {
// this string may be an SDL string even, how do we even evaluate this? haha
}
}
} catch (err) {}
}
async _cacheObjectSchema(
pointer: { [key: string]: any },
project: GraphQLProjectConfig,
) {
await Promise.all(
Object.keys(pointer).map(async schemaUri =>
this._cacheSchemaPath(schemaUri, project),
),
);
}
async _cacheArraySchema(
pointers: UnnormalizedTypeDefPointer[],
project: GraphQLProjectConfig,
) {
await Promise.all(
pointers.map(async schemaEntry => {
if (typeof schemaEntry === 'string') {
await this._cacheSchemaPath(schemaEntry, project);
} else if (schemaEntry) {
await this._cacheObjectSchema(schemaEntry, project);
}
}),
);
}

async _cacheSchemaFilesForProject(project: GraphQLProjectConfig) {
const schema = project?.schema;
const config = project?.extensions?.languageService;
/**
* By default, let's only cache the full graphql config schema.
* This allows us to rely on graphql-config's schema building features
* And our own cache implementations
* This prefers schema instead of SDL first schema development, but will still
* work with lookup of the actual .graphql schema files if the option is enabled,
* however results may vary.
* By default, we look for schema definitions in SDL files
*
* with the opt-in feature `cacheSchemaOutputFileForLookup` enabled,
* the resultant `graphql-config` .getSchema() schema output will be cached
* locally and available as a single file for definition lookup and peek
*
* The default temporary schema file seems preferrable until we have graphql source maps
* this is helpful when your `graphql-config` `schema` input is:
* - a remote or local URL
* - compiled from graphql files and code sources
* - otherwise where you don't have schema SDL in the codebase or don't want to use it for lookup
*
* it is disabled by default
*/
const useSchemaFileDefinitions =
(config?.useSchemaFileDefinitions ??
this?._settings?.useSchemaFileDefinitions) ||
const cacheSchemaFileForLookup =
config?.cacheSchemaFileForLookup ??
this?._settings?.cacheSchemaFileForLookup ??
false;
if (!useSchemaFileDefinitions) {
if (cacheSchemaFileForLookup) {
await this._cacheConfigSchema(project);
} else {
if (Array.isArray(schema)) {
Promise.all(
schema.map(async (uri: UnnormalizedTypeDefPointer) => {
await this._cacheSchemaFile(uri, project);
}),
);
} else {
const uri = schema.toString();
await this._cacheSchemaFile(uri, project);
}
} else if (typeof schema === 'string') {
await this._cacheSchemaPath(schema, project);
} else if (Array.isArray(schema)) {
await this._cacheArraySchema(schema, project);
} else if (schema) {
await this._cacheObjectSchema(schema, project);
}
}
/**
Expand Down Expand Up @@ -995,7 +1042,7 @@ export class MessageProcessor {
if (config?.projects) {
return Promise.all(
Object.keys(config.projects).map(async projectName => {
const project = await config.getProject(projectName);
const project = config.getProject(projectName);
await this._cacheSchemaFilesForProject(project);
await this._cacheDocumentFilesforProject(project);
}),
Expand Down Expand Up @@ -1057,13 +1104,11 @@ export class MessageProcessor {
contents,
});
}
} else if (textDocument?.version) {
return this._textDocumentCache.set(uri, {
version: textDocument.version,
contents,
});
}
return null;
return this._textDocumentCache.set(uri, {
version: textDocument.version ?? 0,
contents,
});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ projects:
schema:
- __schema__/StarWarsSchema.graphql
- 'directive @customDirective on FRAGMENT_SPREAD'
testWithGlobSchema:
schema:
- __schema__/*.graphql
- 'directive @customDirective on FRAGMENT_SPREAD'
testWithEndpoint:
schema: https://example.com/graphql
testWithEndpointAndSchema:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Logger', () => {

it('logs to stdout', () => {
const logger = new Logger(tmpdir());
logger.log('log test');
logger.info('log test');

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

expect(mockedStdoutWrite.mock.calls.length).toBe(0);
expect(mockedStderrWrite.mock.calls.length).toBe(4);
expect(mockedStderrWrite.mock.calls.length).toBe(3);
expect(mockedStderrWrite.mock.calls[0][0]).toContain('info test');
expect(mockedStderrWrite.mock.calls[1][0]).toContain('warn test');
expect(mockedStderrWrite.mock.calls[2][0]).toContain('log test');
expect(mockedStderrWrite.mock.calls[3][0]).toContain('error test');
expect(mockedStderrWrite.mock.calls[2][0]).toContain('error test');
});
});
23 changes: 17 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3270,7 +3270,7 @@
tslib "^2"

"@graphiql/toolkit@file:packages/graphiql-toolkit":
version "0.4.1"
version "0.4.2"
dependencies:
"@n1ru4l/push-pull-async-iterable-iterator" "^3.1.0"
meros "^1.1.4"
Expand Down Expand Up @@ -10014,6 +10014,17 @@ fast-glob@^3.1.1:
micromatch "^4.0.2"
picomatch "^2.2.1"

fast-glob@^3.2.7:
version "3.2.7"
resolved "https://registry.yarnpkg.com/fast-glob/-/fast-glob-3.2.7.tgz#fd6cb7a2d7e9aa7a7846111e85a196d6b2f766a1"
integrity sha512-rYGMRwip6lUMvYD3BTScMwT1HtAs2d71SMv66Vrxs0IekGZEjhM0pcMfjQPnknBt2zeCwQMEupiN02ZP4DiT1Q==
dependencies:
"@nodelib/fs.stat" "^2.0.2"
"@nodelib/fs.walk" "^1.2.3"
glob-parent "^5.1.2"
merge2 "^1.3.0"
micromatch "^4.0.4"

fast-json-stable-stringify@2.x, fast-json-stable-stringify@^2.0.0:
version "2.1.0"
resolved "https://registry.yarnpkg.com/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz#874bf69c6f404c2b5d99c481341399fd55892633"
Expand Down Expand Up @@ -10744,7 +10755,7 @@ glob-parent@^5.0.0, glob-parent@~5.1.0:
dependencies:
is-glob "^4.0.1"

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

"graphiql@file:packages/graphiql":
version "1.5.2"
version "1.5.5"
dependencies:
"@graphiql/toolkit" "^0.4.1"
"@graphiql/toolkit" "^0.4.2"
codemirror "^5.58.2"
codemirror-graphql "^1.2.0"
codemirror-graphql "^1.2.3"
copy-to-clipboard "^3.2.0"
dset "^3.1.0"
entities "^2.0.0"
escape-html "^1.0.3"
graphql-language-service "^3.2.1"
graphql-language-service "^3.2.3"
markdown-it "^12.2.0"

graphql-config@^4.1.0:
Expand Down

2 comments on commit f82bd7a

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.