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.

Sign up for GitHub

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

Jump to bottom

feat: config lwc mobile eslint plugin #68

Merged
merged 3 commits into from
Jun 28, 2024
Merged

feat: config lwc mobile eslint plugin #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86f03e9
feat: config lwc mobile eslint plugin
ben-zhang-at-salesforce Jun 25, 2024
7efca93
fix: touch up wording
ben-zhang-at-salesforce Jun 25, 2024
6ac6338
code tweak for pr comments
ben-zhang-at-salesforce Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@
"configuration": {
"title": "%salesforce.mobile.extensions%",
"properties": {
"mobileOfflineLinting.eslint-plugin-lwc-mobile": {
"type": "string",
"default": "^1.0.0",
"description": "%extension.commands.salesforce-mobile-offline.lwc-mobile.version%"
},
"mobileOfflineLinting.eslint-plugin-lwc-graph-analyzer": {
"type": "string",
"default": "^0.9.0",
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"extension.commands.config-wizard.category": "Offline Starter Kit",
"extension.commands.config-linting-tools.title": "Configure Linting Tools",
"extension.commands.salesforce-mobile-offline.category": "Salesforce Mobile Offline",
"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",
"salesforce.mobile.extensions": "Salesforce Mobile Extensions",
Expand Down
90 changes: 53 additions & 37 deletions src/commands/lint/configureLintingToolsCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,48 @@ import * as path from 'path';
import { WorkspaceUtils } from '../../utils/workspaceUtils';
import { JSON_INDENTATION_SPACES } from '../../utils/constants';

const config = workspace.getConfiguration();
const eslintPluginLwcGraphAnalyzer =
'@salesforce/eslint-plugin-lwc-graph-analyzer';
const eslintPluginLwcGraphAnalyzerConfig =
'mobileOfflineLinting.eslint-plugin-lwc-graph-analyzer';
const eslintPluginLwcGraphAnalyzerVersion = config.get(
eslintPluginLwcGraphAnalyzerConfig
) as string;

const eslint = 'eslint';
const eslintConfig = 'mobileOfflineLinting.eslint';
const eslintVersion = config.get(eslintConfig) as string;

const configureLintingToolsCommand =
'salesforcedx-vscode-offline-app.configureLintingTools';
const eslintDependencies = [
[eslintPluginLwcGraphAnalyzer, eslintPluginLwcGraphAnalyzerVersion],
[eslint, eslintVersion]
];

const lwcGraphAnalyzerRecommended: string =
'plugin:@salesforce/lwc-graph-analyzer/recommended';
const eslintRecommended = 'eslint:recommended';
const config = workspace.getConfiguration();

class EslintDependencyConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In TypeScript, these kinds of data structures are better represented as types (or type aliases, technically). I actually like their types cheat sheet, once your head is around the basic concept. Nice go-to reference for syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, let me do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

read a bit about type/type aliases, I didn't figure out a way to let type to have function as a member. I think class is still better due to we need to have version value to be dynamically set based. so uses class with a function getVersion() support it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, it'd be a bit different in the type setup. Instead of having a "lazy load" method to get the version, you'd simply initialize the version with the logic that it contains. Arguably, that's the only reason the packageConfigPropertyId exists. So it would instead look something like this:

type EslintDependencyConfig = {
    name: string;
    version: string;
    eslintConfigToExtend: string;
};

const eslintDependencies: EslintDependencyConfig[] = [
    {
        name: '@salesforce/eslint-plugin-lwc-mobile',
        eslintConfigToExtend: 'plugin:@salesforce/lwc-mobile/recommended',
        version: config.get('mobileOfflineLinting.eslint-plugin-lwc-mobile')
    },
    // etc.
];

But it's not a big deal. It works fine the way you have it too.

readonly name: string;
readonly packageConfigPropertyId: string;
readonly eslintConfigToExtend: string;

constructor(
name: string,
packageConfigPropertyId: string,
eslintConfigToExtend: string
) {
this.name = name;
this.packageConfigPropertyId = packageConfigPropertyId;
this.eslintConfigToExtend = eslintConfigToExtend;
}

getVersion(): string {
return config.get(this.packageConfigPropertyId) as string;
}
}

const eslintDependencies: EslintDependencyConfig[] = [
new EslintDependencyConfig(
'@salesforce/eslint-plugin-lwc-mobile',
'mobileOfflineLinting.eslint-plugin-lwc-mobile',
'plugin:@salesforce/lwc-mobile/recommended'
),
new EslintDependencyConfig(
'@salesforce/eslint-plugin-lwc-graph-analyzer',
'mobileOfflineLinting.eslint-plugin-lwc-graph-analyzer',
'plugin:@salesforce/lwc-graph-analyzer/recommended'
),
new EslintDependencyConfig(
'eslint',
'mobileOfflineLinting.eslint',
'eslint:recommended'
)
];

interface PackageJson {
devDependencies?: Record<string, string>;
Expand Down Expand Up @@ -64,7 +83,7 @@ export class ConfigureLintingToolsCommand {

// Ask user to add eslint plugin
const result = await this.showMessage(
ben-zhang-at-salesforce marked this conversation as resolved.
Show resolved Hide resolved
'Do you want to add the ESLint plugin for LWC graph analysis to your project? This will give you linting feedback on code patterns that will not support your LWCs working offline, for mobile use cases.',
'Do you want to add Salesforce code linting guidance for Mobile and Offline capabilities? These tools will identify code patterns that cause problems in Mobile and Offline use cases.',
MessageType.InformationYesNo
);

Expand Down Expand Up @@ -135,10 +154,10 @@ export class ConfigureLintingToolsCommand {
let modified = false;

if (devDependencies) {
eslintDependencies.forEach((nameValuePair) => {
const [name, value] = nameValuePair;
eslintDependencies.forEach((dependencyConfig) => {
const { name } = dependencyConfig;
if (!devDependencies[name]) {
devDependencies[name] = value;
devDependencies[name] = dependencyConfig.getVersion();
modified = true;
}
});
Expand Down Expand Up @@ -170,15 +189,12 @@ export class ConfigureLintingToolsCommand {

let modified = false;

if (!eslintrcExtends.includes(eslintRecommended)) {
eslintrcExtends.push(eslintRecommended);
modified = true;
}

if (!eslintrcExtends.includes(lwcGraphAnalyzerRecommended)) {
eslintrc.extends.push(lwcGraphAnalyzerRecommended);
modified = true;
}
eslintDependencies.forEach((config) => {
if (!eslintrcExtends.includes(config.eslintConfigToExtend)) {
eslintrcExtends.push(config.eslintConfigToExtend);
modified = true;
}
});

if (modified) {
// Save json only if the content was modified.
Expand All @@ -192,11 +208,11 @@ export class ConfigureLintingToolsCommand {
} else {
// Create eslintrc
const eslintrc = {
extends: [
`${eslintRecommended}`,
`${lwcGraphAnalyzerRecommended}`
]
extends: eslintDependencies.map((config) => {
return `${config.eslintConfigToExtend}`;
})
};

const jsonString = JSON.stringify(
eslintrc,
null,
Expand Down
2 changes: 2 additions & 0 deletions src/test/suite/commands/lint/configureLintingToolsCommand.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ suite('Configure Linting Tools Command Test Suite', () => {
devDependencies: {
lwc: '1.2.3',
// eslint-disable-next-line @typescript-eslint/naming-convention
'@salesforce/eslint-plugin-lwc-mobile': '^1.0.0',
// eslint-disable-next-line @typescript-eslint/naming-convention
'@salesforce/eslint-plugin-lwc-graph-analyzer': '^0.9.0',
eslint: '^8.47.0'
}
Expand Down
Loading