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.

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

Do not reload project on unchanged bsconfig.json contents #1355

Merged
merged 7 commits into from
Nov 25, 2024
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
8 changes: 8 additions & 0 deletions src/lsp/LspProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions src/lsp/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions src/lsp/ProjectManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
35 changes: 28 additions & 7 deletions src/lsp/ProjectManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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,
Expand All @@ -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
});
Expand All @@ -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))
Expand Down
15 changes: 15 additions & 0 deletions src/lsp/worker/WorkerThreadProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
*/
Expand Down