W-16323982 - Command for tooling hub Komaci Offline Analysis #114
Conversation
|
|
||
| // Deploy LWC so analysis runs against current code | ||
| // TODO: Commented out until TD-0215753 is delivered | ||
| // const result = await vscode.commands.executeCommand('sf.deploy.current.source.file'); |
There was a problem hiding this comment.
Curious, where would have result be in the following code? It seems as though it's not required to create toolingHubUrl?
There was a problem hiding this comment.
Result was originally going to be a boolean - true if deploy success, false if deploy false. This is in the air though, maybe they'd want to return a map with a status code.
It isn't necessary to build the url however - because we grab everything we need from the context (which file is open, namespace, etc).
|
@angelamuliu Can you run prettier please. |
|
@sfdctaka I ran prettier:write and verify, both passed locally. I see the CICD is having issues with it though? Lmk if this is something to be worried about |
|
@angelamuliu I made a PR to your branch. On my machine there was one file that needed to be formatted. Maybe that will get over this CI failures? I am certain that formatting will pass, however, this test failure I was also getting on my local machine. Do you see this too? |
I'm getting the test failure too. Wasn't sure if we just weren't at that point yet or not, but +1. |
package.nls.json
Outdated
| @@ -6,6 +6,7 @@ | |||
| "extension.commands.salesforce-mobile-offline.lwc-mobile.version": "Version of ESLint Plugin LWC Mobile to include in devDependencies", | |||
| "extension.commands.salesforce-mobile-offline.komaci.version": "Version of ESLint Plugin LWC Graph Analyzer to include in devDependencies", | |||
| "extension.commands.salesforce-mobile-offline.eslint.version": "Version of ESLint to include in devDependencies", | |||
| "extension.commands.salesforce-mobile-offline.toolinghub.liveKomaciAnalysis": "(TODO) Salesforce Mobile Offline: Live Komaci Analysis", | |||
There was a problem hiding this comment.
Nit: This is where you make use of the category configuration option in package.json, similar to the linting command. So you'd take out the "Salesforce Mobile Offline:" part here, and let the category value/configuration add the prefix.
There was a problem hiding this comment.
I had that originally actually - but while developing, I noticed it doesn't append the category consistently? I forget exactly which one out of command palette, editor, and context, but one or two of them was missing category.
I figure this is why the base project opts to have "SFDX:" coded into the command label itself.
https://github.com/forcedotcom/salesforcedx-vscode/blob/232e5af3d5d18d4816338e891175ade9bc3f37bf/packages/salesforcedx-vscode-core/package.nls.json
| }); | ||
| } | ||
|
|
||
| static getUriFromActiveEditor = (): vscode.Uri | undefined => { |
There was a problem hiding this comment.
Let's be consistent with the format of function definitions, unless there's a specific binding consideration here that I missed. The rest of the functions are defined as methods rather than arrow function properties.
|
|
||
| static getUriFromActiveEditor = (): vscode.Uri | undefined => { | ||
| const editor = vscode.window.activeTextEditor; | ||
| if (editor && editor.document.languageId !== 'forcesourcemanifest') { |
There was a problem hiding this comment.
Am I reading this correctly that we're only looking for the URIs of non-XML LWC files? It might be worth a brief comment here on the intention.
There was a problem hiding this comment.
I'm going to remove this, it's not necessary since I later added file filtering (see package.json where command only shows up for js/ts/css/html)
But for extra context, this code is only triggered when accessing via command palette, and uri for the other files are things like "javascript" and "html." I originally copy pasted this snippet from the deploy command so unsure why they didn't want to include the js-meta file
| static extractLwcNameFromSourceUri( | ||
| sourceUriInput: vscode.Uri | undefined | ||
| ): string { | ||
| const path = sourceUriInput?.path ? sourceUriInput?.path : null; |
There was a problem hiding this comment.
Nit: nullish coalescing would be cleaner here:
const path = sourceUriInput?.path ?? null;
|
Going to close this for the time being, as we're going to take a different approach for supporting its service. |
What does this PR do?
https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00001xNomIYAS/view