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

@@ -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';

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

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

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);
+    });
 });

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<void> {
         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))

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
      */