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

feat: configure sfdx workspace to write tsconfig files #591

Merged
merged 10 commits into from
Jul 8, 2024
38 changes: 38 additions & 0 deletions packages/lightning-lsp-common/src/__tests__/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,3 +336,41 @@ it('configureCoreAll()', async () => {
// const launchContent = fs.readFileSync(launchPath, 'utf8');
// expect(launchContent).toContain('"name": "SFDC (attach)"');
});

it('configureProjectForTs()', async () => {
const context = new WorkspaceContext('test-workspaces/sfdx-workspace');
const baseTsconfigPathForceApp = 'test-workspaces/sfdx-workspace/.sfdx/tsconfig.sfdx.json';
const tsconfigPathForceApp = FORCE_APP_ROOT + '/lwc/tsconfig.json';
const tsconfigPathUtils = UTILS_ROOT + '/lwc/tsconfig.json';
const tsconfigPathRegisteredEmpty = REGISTERED_EMPTY_FOLDER_ROOT + '/lwc/tsconfig.json';
const forceignorePath = 'test-workspaces/sfdx-workspace/.forceignore';

// configure and verify typings/jsconfig after configuration:
await context.configureProjectForTs();

// verify tsconfig.sfdx.json
const baseTsConfigForceAppContent = fs.readJsonSync(baseTsconfigPathForceApp);
expect(baseTsConfigForceAppContent).toEqual({
compilerOptions: {
target: 'ESNext',
paths: {
'c/*': [],
},
},
});

//verify newly create tsconfig.json
const tsconfigForceAppContent = fs.readJsonSync(tsconfigPathForceApp);
expect(tsconfigForceAppContent).toEqual({
extends: '../../../../.sfdx/tsconfig.sfdx.json',
include: ['**/*.ts', '../../../../.sfdx/typings/lwc/**/*.d.ts'],
exclude: ['**/__tests__/**'],
});

// clean up artifacts
fs.removeSync(baseTsconfigPathForceApp);
fs.removeSync(tsconfigPathForceApp);
fs.removeSync(tsconfigPathUtils);
fs.removeSync(tsconfigPathRegisteredEmpty);
fs.removeSync(forceignorePath);
});
31 changes: 31 additions & 0 deletions packages/lightning-lsp-common/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,14 @@ export class WorkspaceContext {
await this.writeTypings();
}

/**
* Configures LWC project to support TypeScript
*/
public async configureProjectForTs(): Promise<void> {
// TODO: This should be moved into configureProject after dev preview
await this.writeTsconfigJson();
}

/**
* Acquires list of absolute modules directories, optimizing for workspace type
* @returns Promise
Expand Down Expand Up @@ -449,6 +457,29 @@ export class WorkspaceContext {
}
}

private async writeTsconfigJson(): Promise<void> {
switch (this.type) {
case WorkspaceType.SFDX:
// Write tsconfig.sfdx.json first
const baseTsConfigPath = path.join(this.workspaceRoots[0], '.sfdx', 'tsconfig.sfdx.json');
const baseTsConfig = await fs.readFile(utils.getSfdxResource('tsconfig-sfdx.base.json'), 'utf8');
this.updateConfigFile(baseTsConfigPath, baseTsConfig);
// Write to the tsconfig.json in each module subdirectory
const tsConfigTemplate = await fs.readFile(utils.getSfdxResource('tsconfig-sfdx.json'), 'utf8');
const forceignore = path.join(this.workspaceRoots[0], '.forceignore');
// TODO: We should only be looking through modules that have TS files
const modulesDirs = await this.getModulesDirs();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In a follow-up PR I plan to only check for lwc directories that actually have TS files.

For now, I'm writing to all lwc directories.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should also only be generating either a jsconfig.json or a tsconfig.json but not both, this can be done once we've moved to GA.

for (const modulesDir of modulesDirs) {
const tsConfigPath = path.join(modulesDir, 'tsconfig.json');
const relativeWorkspaceRoot = utils.relativePath(path.dirname(tsConfigPath), this.workspaceRoots[0]);
const tsConfigContent = this.processTemplate(tsConfigTemplate, { project_root: relativeWorkspaceRoot });
this.updateConfigFile(tsConfigPath, tsConfigContent);
await this.updateForceIgnoreFile(forceignore);
Copy link
Member

Choose a reason for hiding this comment

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

What is the point here to call this.updateForceIgnoreFile? The file you are adding is tsconfig.json here. but the function is to add jsconfig and eslintrc into ignore file. Are you trying to ignore the tsconfig.json files?

Copy link
Member

Choose a reason for hiding this comment

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

nvm I saw the PR you created

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the same behavior as with jsconfig.json, if the language server detects a tsconfig.json it will add it to the ignore file.

}
break;
}
}

private async writeSettings(): Promise<void> {
switch (this.type) {
case WorkspaceType.CORE_ALL:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"target": "ESNext",
"paths": {
"c/*": []
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"extends": "${project_root}/.sfdx/tsconfig.sfdx.json",
"include": [
"**/*.ts",
"${project_root}/.sfdx/typings/lwc/**/*.d.ts"
],
"exclude": [
"**/__tests__/**"
]
}
50 changes: 44 additions & 6 deletions packages/lwc-language-server/src/__tests__/lwc-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,39 +12,47 @@ import {
} from 'vscode-languageserver';

import { URI } from 'vscode-uri';
import { sync } from 'fast-glob';
import * as fsExtra from 'fs-extra';
import * as path from 'path';

const filename = path.resolve('../../test-workspaces/sfdx-workspace/force-app/main/default/lwc/todo/todo.html');
const SFDX_WORKSPACE_ROOT = '../../test-workspaces/sfdx-workspace';
const filename = path.resolve(SFDX_WORKSPACE_ROOT + '/force-app/main/default/lwc/todo/todo.html');
const uri = URI.file(filename).toString();
const document: TextDocument = TextDocument.create(uri, 'html', 0, fsExtra.readFileSync(filename).toString());

const jsFilename = path.resolve('../../test-workspaces/sfdx-workspace/force-app/main/default/lwc/todo/todo.js');
const jsFilename = path.resolve(SFDX_WORKSPACE_ROOT + '/force-app/main/default/lwc/todo/todo.js');
const jsUri = URI.file(jsFilename).toString();
const jsDocument: TextDocument = TextDocument.create(uri, 'javascript', 0, fsExtra.readFileSync(jsFilename).toString());

const auraFilename = path.resolve('../../test-workspaces/sfdx-workspace/force-app/main/default/aura/todoApp/todoApp.app');
const auraFilename = path.resolve(SFDX_WORKSPACE_ROOT + '/force-app/main/default/aura/todoApp/todoApp.app');
const auraUri = URI.file(auraFilename).toString();
const auraDocument: TextDocument = TextDocument.create(auraFilename, 'html', 0, fsExtra.readFileSync(auraFilename).toString());

const hoverFilename = path.resolve('../../test-workspaces/sfdx-workspace/force-app/main/default/lwc/lightning_tree_example/lightning_tree_example.html');
const hoverFilename = path.resolve(SFDX_WORKSPACE_ROOT + '/force-app/main/default/lwc/lightning_tree_example/lightning_tree_example.html');
const hoverUri = URI.file(hoverFilename).toString();
const hoverDocument: TextDocument = TextDocument.create(hoverFilename, 'html', 0, fsExtra.readFileSync(hoverFilename).toString());

const server: Server = new Server();

let mockTypeScriptSupportConfig = false;

jest.mock('vscode-languageserver', () => {
const actual = jest.requireActual('vscode-languageserver');
return {
...actual,
createConnection: jest.fn().mockImplementation(() => {
return {
onInitialize: (): boolean => true,
onInitialized: (): boolean => true,
onCompletion: (): boolean => true,
onCompletionResolve: (): boolean => true,
onHover: (): boolean => true,
onShutdown: (): boolean => true,
onDefinition: (): boolean => true,
workspace: {
getConfiguration: (): boolean => mockTypeScriptSupportConfig,
},
};
}),
TextDocuments: jest.fn().mockImplementation(() => {
Expand Down Expand Up @@ -81,8 +89,8 @@ describe('handlers', () => {
capabilities: {},
workspaceFolders: [
{
uri: URI.file(path.resolve('../../test-workspaces/sfdx-workspace/')).toString(),
name: path.resolve('../../test-workspaces/sfdx-workspace/'),
uri: URI.file(path.resolve(SFDX_WORKSPACE_ROOT)).toString(),
name: path.resolve(SFDX_WORKSPACE_ROOT),
},
],
};
Expand Down Expand Up @@ -324,6 +332,36 @@ describe('handlers', () => {
expect(location.range.start.character).toEqual(60);
});
});

describe('onInitialized()', () => {
it('skip tsconfig initialization when salesforcedx-vscode-lwc.preview.typeScriptSupport = false', async () => {
await server.onInitialize(initializeParams);
await server.onInitialized();

const baseTsconfigPath = SFDX_WORKSPACE_ROOT + '/.sfdx/tsconfig.sfdx.json';
const tsconfigPaths = sync(SFDX_WORKSPACE_ROOT + '/**/lwc/tsconfig.json');
expect(fsExtra.existsSync(baseTsconfigPath)).toBe(false);
expect(tsconfigPaths.length).toBe(0);
});

it('skip tsconfig initialization when salesforcedx-vscode-lwc.preview.typeScriptSupport = true', async () => {
// Enable feature flag
mockTypeScriptSupportConfig = true;
await server.onInitialize(initializeParams);
await server.onInitialized();

const baseTsconfigPath = SFDX_WORKSPACE_ROOT + '/.sfdx/tsconfig.sfdx.json';
const tsconfigPaths = sync(SFDX_WORKSPACE_ROOT + '/**/lwc/tsconfig.json');
expect(fsExtra.existsSync(baseTsconfigPath)).toBe(true);
// There are currently 3 lwc subdirectories under SFDX_WORKSPACE_ROOT
expect(tsconfigPaths.length).toBe(3);

// Clean up after test run
fsExtra.removeSync(baseTsconfigPath);
await Promise.all(tsconfigPaths.map(tsconfig => fsExtra.remove(tsconfig)));
mockTypeScriptSupportConfig = false;
});
});
});

describe('#capabilities', () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/lwc-language-server/src/lwc-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export default class Server {

constructor() {
this.connection.onInitialize(this.onInitialize.bind(this));
this.connection.onInitialized(this.onInitialized.bind(this));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like config values are not available during onInitialize which is why I'm using onInitialized instead.

Once the feature is GA, we can move everything back to onInitialize.

this.connection.onCompletion(this.onCompletion.bind(this));
this.connection.onCompletionResolve(this.onCompletionResolve.bind(this));
this.connection.onHover(this.onHover.bind(this));
Expand Down Expand Up @@ -130,6 +131,15 @@ export default class Server {
};
}

async onInitialized(): Promise<void> {
// The config value comes from salesforcedx-vscode/salesforcedx-vscode-lwc
const hasTsEnabled = await this.connection.workspace.getConfiguration('salesforcedx-vscode-lwc.preview.typeScriptSupport');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The entire feature is gated behind this feature flag, which hasn't been implemented in salesforcedx-vscode yet.

There is a follow-up PR to implement the flag, in the meantime, I've tested locally that when the config value is enabled it works correctly.

Copy link
Collaborator Author

@jmsjtu jmsjtu Jun 26, 2024

Choose a reason for hiding this comment

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

After discussing with the team, we should remove the tsconfig.json if salesforcedx-vscode-lwc.preview.typeScriptSupport = false.

// TODO: add onDidChangeConfiguration handler to detect changes in config value
if (hasTsEnabled) {
await this.context.configureProjectForTs();
}
}

async onCompletion(params: CompletionParams): Promise<CompletionList> {
const {
position,
Expand Down
Loading