From 3ae8b7ca685ba794d432d419c42353198398f704 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 25 Nov 2024 15:56:37 -0500 Subject: [PATCH] Do not reload project on unchanged bsconfig.json contents (#1355) * Cancel requests to disposed projects * Fix broken package-lock * Do not reload project if bsconfig contents are unchanged --- src/index.ts | 2 +- src/lsp/LspProject.ts | 8 +++++ src/lsp/Project.ts | 13 ++++++++ src/lsp/ProjectManager.spec.ts | 43 +++++++++++++++++++++++++++ src/lsp/ProjectManager.ts | 35 +++++++++++++++++----- src/lsp/worker/WorkerThreadProject.ts | 15 ++++++++++ 6 files changed, 108 insertions(+), 8 deletions(-) diff --git a/src/index.ts b/src/index.ts index f6dbf87f5..cfcde7999 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,7 +18,7 @@ export * from './parser/Statement'; export * from './BsConfig'; export * from './deferred'; // convenience re-export from vscode -export { Range, Position, CancellationToken, CancellationTokenSource, DiagnosticSeverity, DiagnosticTag, SemanticTokenTypes, CodeAction } from 'vscode-languageserver'; +export { Range, Position, CancellationToken, CancellationTokenSource, DiagnosticSeverity, DiagnosticTag, SemanticTokenTypes, CodeAction, Diagnostic } from 'vscode-languageserver'; export * from './astUtils/visitors'; export * from './astUtils/stackedVisitor'; export * from './astUtils/reflection'; diff --git a/src/lsp/LspProject.ts b/src/lsp/LspProject.ts index fc68c46a3..34101e31e 100644 --- a/src/lsp/LspProject.ts +++ b/src/lsp/LspProject.ts @@ -56,6 +56,14 @@ export interface LspProject { */ bsconfigPath?: string; + /** + * The contents of the bsconfig.json file. This is used to detect when the bsconfig file has not actually been changed (even if the fs says it did). + * + * Only available after `.activate()` has completed. + * @deprecated do not depend on this property. This will certainly be removed in a future release + */ + bsconfigFileContents?: string; + /** * Initialize and start running the project. This will scan for all files, and build a full project in memory, then validate the project * @param options diff --git a/src/lsp/Project.ts b/src/lsp/Project.ts index 47743e33a..cee587d60 100644 --- a/src/lsp/Project.ts +++ b/src/lsp/Project.ts @@ -17,6 +17,7 @@ import type { SignatureInfoObj } from '../Program'; import type { BsConfig } from '../BsConfig'; import type { Logger, LogLevel } from '../logging'; import { createLogger } from '../logging'; +import * as fsExtra from 'fs-extra'; export class Project implements LspProject { public constructor( @@ -56,6 +57,11 @@ export class Project implements LspProject { //if the config file exists, use it and its folder as cwd if (this.bsconfigPath && await util.pathExists(this.bsconfigPath)) { cwd = path.dirname(this.bsconfigPath); + //load the bsconfig file contents (used for performance optimizations externally) + try { + this.bsconfigFileContents = (await fsExtra.readFile(this.bsconfigPath)).toString(); + } catch { } + } else { cwd = this.projectPath; //config file doesn't exist...let `brighterscript` resolve the default way @@ -474,6 +480,13 @@ export class Project implements LspProject { */ public bsconfigPath?: string; + /** + * The contents of the bsconfig.json file. This is used to detect when the bsconfig file has not actually been changed (even if the fs says it did). + * + * Only available after `.activate()` has completed. + * @deprecated do not depend on this property. This will certainly be removed in a future release + */ + public bsconfigFileContents?: string; /** * Find the path to the bsconfig.json file for this project diff --git a/src/lsp/ProjectManager.spec.ts b/src/lsp/ProjectManager.spec.ts index 2dc186e84..43b7c227a 100644 --- a/src/lsp/ProjectManager.spec.ts +++ b/src/lsp/ProjectManager.spec.ts @@ -862,4 +862,47 @@ describe('ProjectManager', () => { //test passes if the promise resolves }); + + it('properly handles reloading when bsconfig.json contents change', async () => { + fsExtra.outputJsonSync(`${rootDir}/bsconfig.json`, { + files: [ + 'one' + ] + }); + + //wait for the projects to finish syncing/loading + await manager.syncProjects([{ + workspaceFolder: rootDir, + enableThreading: false + }]); + + const stub = sinon.stub(manager as any, 'reloadProject').callThrough(); + + //change the file to new contents + fsExtra.outputJsonSync(`${rootDir}/bsconfig.json`, { + files: [ + 'two' + ] + }); + await manager.handleFileChanges([{ + srcPath: `${rootDir}/bsconfig.json`, + type: FileChangeType.Changed + }]); + + //the project was reloaded + expect(stub.callCount).to.eql(1); + + //change the file to the same contents + fsExtra.outputJsonSync(`${rootDir}/bsconfig.json`, { + files: [ + 'two' + ] + }); + await manager.handleFileChanges([{ + srcPath: `${rootDir}/bsconfig.json`, + type: FileChangeType.Changed + }]); + //the project was not reloaded this time + expect(stub.callCount).to.eql(1); + }); }); diff --git a/src/lsp/ProjectManager.ts b/src/lsp/ProjectManager.ts index bf63e3361..03cd083cc 100644 --- a/src/lsp/ProjectManager.ts +++ b/src/lsp/ProjectManager.ts @@ -18,6 +18,7 @@ import type { Logger } from '../logging'; import { createLogger } from '../logging'; import { Cache } from '../Cache'; import { ActionQueue } from './ActionQueue'; +import * as fsExtra from 'fs-extra'; const FileChangeTypeLookup = Object.entries(FileChangeType).reduce((acc, [key, value]) => { acc[value] = key; @@ -331,7 +332,7 @@ export class ProjectManager { maxActionDuration: 45_000 }); - public handleFileChanges(changes: FileChange[]) { + public handleFileChanges(changes: FileChange[]): Promise { this.logger.debug('handleFileChanges', changes.map(x => `${FileChangeTypeLookup[x.type]}: ${x.srcPath}`)); //this function should NOT be marked as async, because typescript wraps the body in an async call sometimes. These need to be registered synchronously @@ -347,7 +348,12 @@ export class ProjectManager { * Handle when files or directories are added, changed, or deleted in the workspace. * This is safe to call any time. Changes will be queued and flushed at the correct times */ - public async _handleFileChanges(changes: FileChange[]) { + private async _handleFileChanges(changes: FileChange[]) { + //normalize srcPath for all changes + for (const change of changes) { + change.srcPath = util.standardizePath(change.srcPath); + } + //filter any changes that are not allowed by the path filterer changes = this.pathFilterer.filter(changes, x => x.srcPath); @@ -363,15 +369,14 @@ export class ProjectManager { * Handle a single file change. If the file is a directory, this will recursively read all files in the directory and call `handleFileChanges` again */ private async handleFileChange(change: FileChange) { - const srcPath = util.standardizePath(change.srcPath); if (change.type === FileChangeType.Deleted) { //mark this document or directory as deleted - this.documentManager.delete(srcPath); + this.documentManager.delete(change.srcPath); //file added or changed } else { //if this is a new directory, read all files recursively and register those as file changes too - if (util.isDirectorySync(srcPath)) { + if (util.isDirectorySync(change.srcPath)) { const files = await fastGlob('**/*', { cwd: change.srcPath, onlyFiles: true, @@ -380,7 +385,7 @@ export class ProjectManager { //pipe all files found recursively in the new directory through this same function so they can be processed correctly await Promise.all(files.map((srcPath) => { return this.handleFileChange({ - srcPath: srcPath, + srcPath: util.standardizePath(srcPath), type: FileChangeType.Changed, allowStandaloneProject: change.allowStandaloneProject }); @@ -397,7 +402,23 @@ export class ProjectManager { } //reload any projects whose bsconfig.json was changed - const projectsToReload = this.projects.filter(x => x.bsconfigPath?.toLowerCase() === change.srcPath.toLowerCase()); + const projectsToReload = this.projects.filter(project => { + //this is a path to a bsconfig.json file + if (project.bsconfigPath?.toLowerCase() === change.srcPath.toLowerCase()) { + //fetch file contents if we don't already have them + if (!change.fileContents) { + try { + change.fileContents = fsExtra.readFileSync(project.bsconfigPath).toString(); + } finally { } + } + ///the bsconfig contents have changed since we last saw it, so reload this project + if (project.bsconfigFileContents !== change.fileContents) { + return true; + } + } + return false; + }); + if (projectsToReload.length > 0) { await Promise.all( projectsToReload.map(x => this.reloadProject(x)) diff --git a/src/lsp/worker/WorkerThreadProject.ts b/src/lsp/worker/WorkerThreadProject.ts index 69d95d05f..de8f4d3ef 100644 --- a/src/lsp/worker/WorkerThreadProject.ts +++ b/src/lsp/worker/WorkerThreadProject.ts @@ -15,6 +15,7 @@ import type { FileTranspileResult, SignatureInfoObj } from '../../Program'; import type { Position, Range, Location, DocumentSymbol, WorkspaceSymbol, CodeAction, CompletionList } from 'vscode-languageserver-protocol'; import type { Logger } from '../../logging'; import { createLogger } from '../../logging'; +import * as fsExtra from 'fs-extra'; export const workerPool = new WorkerPool(() => { return new Worker( @@ -70,6 +71,12 @@ export class WorkerThreadProject implements LspProject { this.filePatterns = activateResponse.data.filePatterns; this.logger.logLevel = activateResponse.data.logLevel; + //load the bsconfig file contents (used for performance optimizations externally) + try { + this.bsconfigFileContents = (await fsExtra.readFile(this.bsconfigPath)).toString(); + } catch { } + + this.activationDeferred.resolve(); return activateResponse.data; } @@ -100,6 +107,14 @@ export class WorkerThreadProject implements LspProject { */ public bsconfigPath?: string; + /** + * The contents of the bsconfig.json file. This is used to detect when the bsconfig file has not actually been changed (even if the fs says it did). + * + * Only available after `.activate()` has completed. + * @deprecated do not depend on this property. This will certainly be removed in a future release + */ + bsconfigFileContents?: string; + /** * The worker thread where the actual project will execute */