Add a command to install eslint plugin #62
Conversation
|
@maliroteh-sf @dbreese @khawkins May I get help from you on the text copies please? |
| "commandPalette": [ | ||
| { | ||
| "command": "salesforcedx-vscode-offline-app.configureLintingTools", | ||
| "when": "sfdx_project_opened" |
There was a problem hiding this comment.
Oh cool, I didn't know about this functionality. That's a clean way to specify a precondition here.
package.json
Outdated
| { | ||
| "command": "salesforcedx-vscode-offline-app.configureLintingTools", | ||
| "title": "%extension.commands.config-linting-tools.title%", | ||
| "category": "%extension.commands.config-wizard.category%" |
There was a problem hiding this comment.
Hmmm, I'm not sure about this. In theory this doesn't just apply to the Starter Kit—or really at all, because the Starter Kit already has the linting rules configured out of the box. I wonder if we should define a new category, perhaps called "Salesforce Mobile Offline"?
| const configureLintingToolsCommand = | ||
| 'salesforcedx-vscode-offline-app.configureLintingTools'; | ||
| const eslintDependencies = [ | ||
| ['@salesforce/eslint-plugin-lwc-graph-analyzer', '^0.9.0'], |
There was a problem hiding this comment.
These are configuration items where at least the version values would be better represented in contributes.configuration settings. That way a) we take hard-coded data out of the code, and b) users can update these values in their own personal configuration settings, to override the versions we select.
| static async configure(): Promise<boolean> { | ||
| try { | ||
| if (!WorkspaceUtils.lwcFolderExists()) { | ||
| await this.showMessage('LWC folder does not exist.'); |
There was a problem hiding this comment.
Do these messages need any further context? I don't know what the message title/header will look like, if any. But on their own, these messages seem a little sparse/ambiguous.
| } | ||
|
|
||
| try { | ||
| this.updateEslintrc(); |
There was a problem hiding this comment.
We should be tracking whether this file was updated or not too, and reporting to the user accordingly—even if the report is built into a single message. We could be updating this but not the devDependencies, and vice versa. We should be clear to the user, exactly what's being updated and what is not.
There was a problem hiding this comment.
Showing 3 messages now at a maximum, one for updating devdependencies, one for updating .eslintrc.json, and one for running npm. I thought maybe I should mash the three. But localizing string was going to be tricky so instead went with three.
Co-authored-by: Kevin Hawkins <[email protected]>
Co-authored-by: Kevin Hawkins <[email protected]>
Co-authored-by: Kevin Hawkins <[email protected]>
Co-authored-by: Kevin Hawkins <[email protected]>
| "configuration": { | ||
| "title": "%salesforce.mobile.extensions%", | ||
| "properties": { | ||
| "mobileOfflineLinting.eslint-plugin-lwc-graph-analyzer": { |
There was a problem hiding this comment.
Without preceding mobileOfflineLinting separated by a dot the configuration doesn't get rendered in the settings page of VSCode. With it it becomes category name separated by a colon.
Co-authored-by: Kevin Hawkins <[email protected]>
Co-authored-by: Kevin Hawkins <[email protected]>
This command will add an item in
devDependenciesofpackage.json. That item iseslint-plugin-lwc-graph-analyzer.If there is no
.eslintrc.jsonor is missing to add configuration there that would be added too.