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.

Sign up for GitHub

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

Jump to bottom

Revert "Initialize the project cache more selectively" #2808

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Revert "Initialize the project cache more selectively" #2808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions .changeset/twelve-peas-study.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'graphql-language-service-server': patch
'vscode-graphql': patch
---

fix graphql config init bug
15 changes: 4 additions & 11 deletions packages/graphql-language-service-server/src/MessageProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,19 +216,12 @@ export class MessageProcessor {
}

async handleDidChangeConfiguration(
params: DidChangeConfigurationParams,
_params: DidChangeConfigurationParams,
): Promise<DidChangeConfigurationRegistrationOptions> {
if (!params?.settings || params?.settings.length === 0) {
return {};
}

// reset all the workspace caches
// and prepare for them to lazily re-build based
// on where the user opens and saves files
await Promise.all(
Array.from(this._processors.values()).map(async processor => {
await processor._initializeConfig();
}),
Array.from(this._processors.values()).map(processor =>
processor._updateGraphQLConfig(),
),
);
this._logger.log(
JSON.stringify({
Expand Down
178 changes: 45 additions & 133 deletions packages/graphql-language-service-server/src/WorkspaceMessageProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
*/

import glob from 'fast-glob';
import { existsSync } from 'fs';
import { readFile, writeFile } from 'fs/promises';

import { existsSync, readFileSync, writeFile, writeFileSync } from 'fs';
import {
CachedContent,
FileChangeTypeKind,
Expand Down Expand Up @@ -68,9 +66,12 @@ import {
LoaderNoResultError,
ProjectNotFoundError,
} from 'graphql-config';
import { promisify } from 'util';
import { Logger } from './Logger';
import type { LoadConfigOptions } from './types';

const writeFileAsync = promisify(writeFile);

const configDocLink =
'https://www.npmjs.com/package/graphql-language-service-server#user-content-graphql-configuration-file';

Expand All @@ -81,19 +82,6 @@ type CachedDocumentType = {
function toPosition(position: VscodePosition): IPosition {
return new Position(position.line, position.character);
}
/**
* TODO: much of the logic in here that was added after 2020 or later
* later on should be moved to GraphQLLanguageService or GraphQLCache
* Also - test coverage took a hit with the init "cache warm"
*
* (see `MessageProcessor` file history where this came from)
*/

enum ProjectConfigCacheStatus {
'Loading' = 1,
'Loaded' = 2,
'Error' = 3,
}

export class WorkspaceMessageProcessor {
_connection: Connection;
Expand All @@ -102,7 +90,6 @@ export class WorkspaceMessageProcessor {
_languageService!: GraphQLLanguageService;
_textDocumentCache: Map<string, CachedDocumentType>;
_isInitialized: boolean;
_projectCacheStatuses: Map<string, ProjectConfigCacheStatus>;
_isGraphQLConfigMissing: boolean | null = null;
_logger: Logger;
_extensions?: GraphQLExtensionDeclaration[];
Expand Down Expand Up @@ -140,10 +127,6 @@ export class WorkspaceMessageProcessor {
this._tmpDirBase = path.join(this._tmpDir, 'graphql-language-service');
// use legacy mode by default for backwards compatibility
this._loadConfigOptions = { legacy: true, ...loadConfigOptions };
// track whether we've warmed the cache for each project
// much more performant than loading them all at once when
// you load the first file s
this._projectCacheStatuses = new Map();
if (
loadConfigOptions.extensions &&
loadConfigOptions.extensions?.length > 0
Expand All @@ -156,7 +139,8 @@ export class WorkspaceMessageProcessor {
}
this._rootPath = rootPath;
}
async _initializeConfig() {

async _updateGraphQLConfig() {
const settings = await this._connection.workspace.getConfiguration({
section: 'graphql-config',
scopeUri: this._rootPath,
Expand Down Expand Up @@ -193,84 +177,20 @@ export class WorkspaceMessageProcessor {
this._graphQLCache,
this._logger,
);
this._projectCacheStatuses = new Map();

// reset the project config caches so that they rebuild lazily
} catch (err) {
this._handleConfigError({ err });
}
}
/**
*
* @param uri
*/
async _loadAndCacheProjectConfig(project: GraphQLProjectConfig) {
if (!this._graphQLCache) {
await this._initializeConfig();
}
if (this._graphQLCache) {
// TODO: we can do some really neat things with these statuses
// if we can send them to the client
try {
this._setProjectCacheStatus(
project.name,
ProjectConfigCacheStatus.Loading,
);

await this._cacheSchemaFilesForProject(project);
await this._cacheDocumentFilesforProject(project);

this._setProjectCacheStatus(
project.name,
ProjectConfigCacheStatus.Loaded,
);
} catch (err) {
this._setProjectCacheStatus(
project.name,
ProjectConfigCacheStatus.Error,
);
}
}
}

// encapsulate this for fun later 😜
_setProjectCacheStatus(
projectName: string,
cacheStatus: ProjectConfigCacheStatus,
) {
this._logger.info(
`re-initializing project cache for ${projectName}: ${cacheStatus}`,
);
// TODO: tell the client about this for visual feedback some day?
// this._connection.sendNotification()
this._projectCacheStatuses.set(projectName, cacheStatus);
}
async _ensureProjectCached(project: GraphQLProjectConfig) {
// Lazily warm the cache when a file is opened or saved in that project
// this
try {
// only check has() here rather than the status,
// otherwise bad configs might try to keep re-loading,
// or pending requests might be duplicated or worse
// this means we only try once per project config
if (
this._isGraphQLConfigMissing !== true &&
!this._projectCacheStatuses?.has(project.name)
) {
await this._loadAndCacheProjectConfig(project);
} else {
// don't try to initialize again if we've already tried
// and the graphql config file or package.json entry isn't even there
// then initial call to load the config for this file
return null;
if (this._graphQLConfig || this._graphQLCache?.getGraphQLConfig) {
const config =
this._graphQLConfig ?? this._graphQLCache.getGraphQLConfig();
await this._cacheAllProjectFiles(config);
}
this._isInitialized = true;
} catch (err) {
this._logger.error(String(err));
this._handleConfigError({ err });
}
}

_handleConfigError({ err }: { err: unknown; uri?: string }) {
if (err instanceof ConfigNotFoundError) {
// TODO: obviously this needs to become a map by workspace from uri
// for workspaces support
this._isGraphQLConfigMissing = true;

this._logConfigError(err.message);
Expand Down Expand Up @@ -320,12 +240,20 @@ export class WorkspaceMessageProcessor {
* Initialize the LSP server when the first file is opened or saved,
* so that we can access the user settings for config rootDir, etc
*/

const project = this._graphQLCache.getProjectForFile(
params.textDocument.uri,
);

await this._ensureProjectCached(project);
try {
if (!this._isInitialized || !this._graphQLCache) {
// don't try to initialize again if we've already tried
// and the graphql config file or package.json entry isn't even there
if (this._isGraphQLConfigMissing !== true) {
// then initial call to update graphql config
await this._updateGraphQLConfig();
} else {
return null;
}
}
} catch (err) {
this._logger.error(String(err));
}

// Here, we set the workspace settings in memory,
// and re-initialize the language service when a different
Expand All @@ -352,25 +280,15 @@ export class WorkspaceMessageProcessor {

await this._invalidateCache(textDocument, uri, contents);
} else {
let hasPkgConfig = false;
try {
hasPkgConfig =
uri.endsWith('package.json') ??
JSON.parse(await readFile(uri, 'utf-8'))?.graphql;
} catch {
// if package.json is saved with a parsing error, let's ignore it
}
const configMatchers = [
'graphql.config',
'graphqlrc',
'package.json',
this._settings.load?.fileName,
].filter(Boolean);
if (configMatchers.some(v => uri.match(v)?.length) || hasPkgConfig) {
this._logger.info(
"graphql config changed. opening or saving a file will now re-initialize it's project cache",
);
this._initializeConfig();

if (configMatchers.some(v => uri.match(v)?.length)) {
this._logger.info('updating graphql config');
this._updateGraphQLConfig();
return { uri, diagnostics: [] };
}
// update graphql config only when graphql config is saved!
Expand All @@ -379,12 +297,13 @@ export class WorkspaceMessageProcessor {
contents = cachedDocument.contents;
}
}
if (this._isGraphQLConfigMissing) {
if (!this._graphQLCache) {
return { uri, diagnostics };
} else {
try {
const project = this._graphQLCache.getProjectForFile(uri);
if (
this._projectCacheStatuses.get(project.name) &&
this._isInitialized &&
project?.extensions?.languageService?.enableValidation !== false
) {
await Promise.all(
Expand Down Expand Up @@ -422,7 +341,7 @@ export class WorkspaceMessageProcessor {
async handleDidChangeNotification(
params: DidChangeTextDocumentParams,
): Promise<PublishDiagnosticsParams | null> {
if (this._isGraphQLConfigMissing ?? !this._graphQLCache) {
if (!this._isInitialized || !this._graphQLCache) {
return null;
}
// For every `textDocument/didChange` event, keep a cache of textDocuments
Expand All @@ -439,19 +358,14 @@ export class WorkspaceMessageProcessor {
'`textDocument`, `textDocument.uri`, and `contentChanges` arguments are required.',
);
}
const uri = params.textDocument.uri;

const textDocument = params.textDocument;
const contentChanges = params.contentChanges;
const contentChange = contentChanges[contentChanges.length - 1];

const project = this._graphQLCache.getProjectForFile(uri);
await this._ensureProjectCached(project);

// TODO: is this a good idea? what if a bulk change is incomplete?
// we should re-parse the entire document string

// As `contentChanges` is an array and we just want the
// latest update to the text, grab the last entry from the array.
const uri = textDocument.uri;

// If it's a .js file, try parsing the contents to see if GraphQL queries
// exist. If not found, delete from the cache.
Expand All @@ -468,6 +382,7 @@ export class WorkspaceMessageProcessor {
await this._updateFragmentDefinition(uri, contents);
await this._updateObjectTypeDefinition(uri, contents);

const project = this._graphQLCache.getProjectForFile(uri);
const diagnostics: Diagnostic[] = [];

if (project?.extensions?.languageService?.enableValidation !== false) {
Expand Down Expand Up @@ -501,7 +416,7 @@ export class WorkspaceMessageProcessor {
}

handleDidCloseNotification(params: DidCloseTextDocumentParams): void {
if (this._isGraphQLConfigMissing ?? !this._graphQLCache) {
if (!this._isInitialized || !this._graphQLCache) {
return;
}
// For every `textDocument/didClose` event, delete the cached entry.
Expand Down Expand Up @@ -654,19 +569,16 @@ export class WorkspaceMessageProcessor {
change.type === FileChangeTypeKind.Changed
) {
const uri = change.uri;
const project = this._graphQLCache.getProjectForFile(uri);
await this._ensureProjectCached(project);

const text = await readFile(URI.parse(uri).fsPath, 'utf-8');
const text = readFileSync(URI.parse(uri).fsPath, 'utf-8');
const contents = this._parser(text, uri);

// first update cache
await this._updateFragmentDefinition(uri, contents);
await this._updateObjectTypeDefinition(uri, contents);

const project = this._graphQLCache.getProjectForFile(uri);
let diagnostics: Diagnostic[] = [];

// then validate
if (project?.extensions?.languageService?.enableValidation !== false) {
diagnostics = (
await Promise.all(
Expand Down Expand Up @@ -894,7 +806,7 @@ export class WorkspaceMessageProcessor {
if (schemaDocument) {
version = schemaDocument.version++;
}
const schemaText = await readFile(uri, { encoding: 'utf-8' });
const schemaText = readFileSync(uri, { encoding: 'utf-8' });
this._cacheSchemaText(schemaUri, schemaText, version);
}
}
Expand Down Expand Up @@ -1026,14 +938,14 @@ export class WorkspaceMessageProcessor {
const cachedSchemaDoc = this._getCachedDocument(uri);

if (!cachedSchemaDoc) {
await writeFile(fsPath, schemaText, {
await writeFileAsync(fsPath, schemaText, {
encoding: 'utf-8',
});
await this._cacheSchemaText(uri, schemaText, 1);
}
// do we have a change in the getSchema result? if so, update schema cache
if (cachedSchemaDoc) {
await writeFile(fsPath, schemaText, {
writeFileSync(fsPath, schemaText, {
encoding: 'utf-8',
});
await this._cacheSchemaText(
Expand Down
6 changes: 0 additions & 6 deletions packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ jest.mock('@whatwg-node/fetch', () => ({
TextDecoder: global.TextDecoder,
}));

// jest.mock('cross-undici-fetch', () => ({
// fetch: require('fetch-mock').fetchHandler,
// AbortController: MockAbortController,
// TextDecoder: global.TextDecoder,
// }));

jest.mock('cross-fetch', () => ({
fetch: require('fetch-mock').fetchHandler,
AbortController: MockAbortController,
Expand Down
Loading