-
Notifications
You must be signed in to change notification settings - Fork 48
feat: automatically updating tsconfig.json's paths mapping for core workspaces.
#587
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
base: main
Are you sure you want to change the base?
Conversation
| expect(await context.isLWCTypeScript(document)).toBeTruthy(); | ||
|
|
||
| // lwc .ts outside namespace root | ||
| document = readAsTextDocument(CORE_ALL_ROOT + '/ui-force-components/modules/force/input-phone/input-phone.ts'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand why this is outside of the namespace root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to test that if a ts file that is outside of the VSCode workspace is modified, we do not update tsconfig.json. Here the context workspace root is CORE_PROJECT_ROOT which is ui-global-components so if a file in ui-force-components is modified the return value of isLWCTypeScript should be false. I'll add an inline comment to explain this.
| import {obj7} from 'namespace/cmpName'; | ||
| `); | ||
| const imports = await collectImportsForDocument(document); | ||
| expect(imports.size).toEqual(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "lightning/confirm" excluded because it is an off-core component? Saw your comment below about special cases.
| * Gets all paths for TypeScript LWC components on core. | ||
| */ | ||
| get componentEntries(): string[] { | ||
| const defaultSource = normalize(`${this.coreRoot}/*/modules/*/*/*.ts`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming a folder layout like this
/ui-force-components
-->/modules
---->/force
------>/one
-------->/one.ts
-------->/util/util.ts
/*/modules/*/*/*.ts would not include any nested folders(util.ts) underneath a component folder. I think that is fine because the importer only can import the main entry file of a module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct. We only want to include the main component entry ts file and ignore all other utility files.
ravijayaramappa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I would defer to the language-server team to approve since there are aspects here that I am not familiar with.
|
Related but slightly off topic. This issue is also prevalent in SFDX workspaces and makes the DX painful. Would love to see similar ported to SFDX workspaces especially with the move to TS |
What does this PR do?
Impact: this PR only has impact on core workspaces for Salesforce internal users working on core. For external users working on SFDX workspaces, this PR should have no impact.
This PR adds a new feature for automatically updating
tsconfig.json'spathsmapping for core workspaces.LWC team is working on supporting TypeScript for authoring LWCs on core. In order for TypeScript compiler to work with LWC’s folder structure, we need to add a series of
pathsentries intsconfig.jsonthat re-map imports of LWC components to relative file paths of the imported components. For example, if componentone/cmpAimportsforce/wireUtils, the import statement inone/cmpAwill be likeimport { someUtil } from ‘force/wireUtils’;and for TypeScript to know where to look forforce/wireUtilswe need to add a path entry intsconfig.jsonas the following.Maintaining this mapping in
tsconfig.jsonmanually is not convenient and a less ideal DX for developers on core. This PR adds a new indexer in thelwc-language-serverthat maintains a mapping between LWCs and their paths intsconfig.json, automatically updating thetsconfig.jsonfile when language server is initialized and whenever a LWC TypeScript file is changed. Thus developers won't need to create entries forpathsmanually when creating new LWCs in TypeScript.This new feature will update
tsconfig.jsonfile for thepathsattribute for all the core modules in the workspace when language server is initialized. Then whenever a LWC TS file is changed, all the imports are analyzed and any necessary LWC imports will be added totsconfig.jsonautomatically.Here is a short demo video link on this feature. In the video, the test workspace
lightning-language-server/test-workspace/core-like-workspace/coreTS/core(created in this PR) is opened in VSCode, there are two modulesui-force-componentsandui-global-componentsin the workspace. Bothtsconfig.jsonfiles for them don't have apathsentries. After the indexer's init function is executed, bothtsconfig.jsonfiles are updated with thepathsmappings. After that, if I add an import statement in a ts file, thetsconfig.jsonfile is also updated to have the mapping for the new import.Testing this locally will require this PR for adding TS files to file watcher.
What issues does this PR fix or reference?
@W-14958700@