Add eslint.codeActionsOnSave.options by JavaScriptBach · Pull Request #1945 · microsoft/vscode-eslint

JavaScriptBach · 2024-10-30T00:37:27Z

Overview

Add an eslint.codeActionsOnSave.options that allows users to customize the options that ESLint runs with on save. This allows passing in a different set of options to ESLint on save.

If eslint.codeActionsOnSave.rules is specified, they will take priority over any rules specified in eslint.codeActionsOnSave.options. However, my use case for adding this is to specify a completely different config to provide faster on-save performance; more details in the test plan below.

See motivation in #1938 (comment).

Testing

Tested manually on my codebase with the following settings:

"editor.codeActionsOnSave": {
    "source.fixAll.eslint": "explicit"
},
"eslint.codeActionsOnSave.options": {
    "overrideConfigFile": "<workspaces path>/eslint.precommit.mjs",
    "fix": true,
    "fixTypes": ["problem", "suggestion", "layout"]
},

Verified that saving respected the precommit config and was faster than using the full config.

JavaScriptBach · 2024-10-30T01:25:15Z

@microsoft-github-policy-service agree [company="Vanta"]

JavaScriptBach · 2024-10-30T01:25:42Z

@microsoft-github-policy-service agree company="Vanta"

JavaScriptBach · 2024-10-30T02:36:17Z

@dbaeumer I think I've followed your instructions correctly in #1938 (comment). Is there a way I can build and install this extension locally so I can test whether the setting works?

dbaeumer · 2024-10-30T08:52:31Z

You can run from source

debug viewlet
Run Client lauch config

To build and install locally do the following:

dump the version number 3.0.15
npm run compile
install vsce globally
run vsce package This produces a local vsix file
Run command: Install from VSIX
Select the local vsix file

Let me know how it goes.

dbaeumer · 2024-10-30T09:02:52Z

I did a first quick look and one thing I noticed is: do we want to support to only provide options without any rule modifications. That would have an impact on the code here: https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslint.ts#L515

JavaScriptBach · 2024-10-30T16:51:27Z

I was thinking if eslint.codeActionsOnSave.options is set, then eslint.codeActionsOnSave.rules should be ignored. The user can supply their own config in options, which means they should still be able to customize the rules.

dbaeumer · 2024-10-31T09:38:48Z

I was thinking if eslint.codeActionsOnSave.options is set, then eslint.codeActionsOnSave.rules should be ignored. The user can supply their own config in options, which means they should still be able to customize the rules.

I think we should still allow this. If users already customized this and he simply wants to add some basic options I would still prefer to allow this.

JavaScriptBach · 2024-11-30T06:07:53Z

@dbaeumer sorry for the late update. This is ready for a review now.

MariaSolOs · 2025-03-20T01:24:23Z

@dbaeumer is there something blocking this PR from being merged?

If so (and if @JavaScriptBach is no longer interested) I'm more than happy to continue the remaining work.

dbaeumer · 2025-03-20T07:36:30Z

@MariaSolOs mostly time. I see if a can spare some time next week to catch up with the PR.

MariaSolOs · 2025-03-20T12:07:59Z

@MariaSolOs mostly time. I see if a can spare some time next week to catch up with the PR.

Thank you! I would really appreciate that.

MariaSolOs · 2025-03-27T17:08:43Z

+					"scope": "resource",
+					"type": "object",
+					"default": {},
+					"markdownDescription": "The eslint options object to use on save. (see https://eslint.org/docs/developer-guide/nodejs-api#eslint-class). eslint.codeActionsOnSave.rules, if specified, will take priority over any rule options here."


Nit:

Suggested change

"markdownDescription": "The eslint options object to use on save. (see https://eslint.org/docs/developer-guide/nodejs-api#eslint-class). eslint.codeActionsOnSave.rules, if specified, will take priority over any rule options here."

"markdownDescription": "The ESLint options object to use on save (see https://eslint.org/docs/developer-guide/nodejs-api#eslint-class). `eslint.codeActionsOnSave.rules`, if specified, will take priority over any rule options here."

MariaSolOs · 2025-03-27T17:10:22Z

+		let eslintOptions: ESLintClassOptions = {fix: true};
+		if (offRules !== undefined || overrideOptions !== undefined) {
+			if (overrideOptions !== undefined) {
+				eslintOptions = {...overrideOptions, ...eslintOptions};


We probably want to swap the order of the options here for the save options to override the other ones right?

Suggested change

eslintOptions = {...overrideOptions, ...eslintOptions};

eslintOptions = {...eslintOptions, ...overrideOptions};

MariaSolOs · 2025-03-27T17:10:37Z

 			}
 			return result;
-		}, settings, overrideConfig !== undefined ? { fix: true, overrideConfig } : { fix: true });
+		}, settings, eslintOptions );


Nit:

Suggested change

}, settings, eslintOptions );

}, settings, eslintOptions);

MariaSolOs · 2025-03-28T16:21:58Z

@dbaeumer friendly ping about this PR 🙃

Once you review it, I'm happy to make the required changes.

dbaeumer · 2025-03-31T09:06:36Z

+				eslintOptions = {...overrideOptions, ...eslintOptions};
+			}
+			if (offRules !== undefined && offRules.size > 0) {
+				const overrideConfig = {rules: Object.create(null)};


We should keep the spaces here

dbaeumer · 2025-03-31T09:06:50Z

 			}
 			return result;
-		}, settings, overrideConfig !== undefined ? { fix: true, overrideConfig } : { fix: true });
+		}, settings, eslintOptions );


dbaeumer · 2025-03-31T09:09:16Z

 				}
 			}
-			return offRules.size > 0 ? { offRules, onRules } : undefined;
+			return (offRules.size > 0 || options) ? { offRules, onRules, options } : undefined;


Until now we had undefined when we didn't have any rule changes. If we now had options we will end up with to empty sets if no options are specified. Might not be too big of a problem. But may be e can avoid this.

MariaSolOs · 2025-04-01T18:06:52Z

@dbaeumer I opened a new PR in #1999 including the changes from here together with all the fixes from our reviews.

dbaeumer · 2025-04-04T10:15:30Z

I will close this PR then.

Add eslint.codeActionsOnSave.options

b85e8ac

JavaScriptBach marked this pull request as ready for review October 30, 2024 02:35

JavaScriptBach marked this pull request as draft October 31, 2024 04:59

fix

90de6bb

JavaScriptBach marked this pull request as ready for review November 30, 2024 06:07

privatenumber mentioned this pull request Mar 17, 2025

feat(order rule): add forceFix option for potentially unsafe import sorting un-ts/eslint-plugin-import-x#230

Open

Merge branch 'main' into on-save

1e6e0ce

MariaSolOs reviewed Mar 27, 2025

View reviewed changes

dbaeumer requested changes Mar 31, 2025

View reviewed changes

MariaSolOs mentioned this pull request Apr 1, 2025

Add eslint.codeActionsOnSave.options #1999

Merged

dbaeumer closed this Apr 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add eslint.codeActionsOnSave.options#1945

Add eslint.codeActionsOnSave.options#1945
JavaScriptBach wants to merge 3 commits into
microsoft:mainfrom
JavaScriptBach:on-save

JavaScriptBach commented Oct 30, 2024 •

edited

Loading

JavaScriptBach commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

dbaeumer commented Oct 30, 2024

dbaeumer commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

dbaeumer commented Oct 31, 2024

JavaScriptBach commented Nov 30, 2024

MariaSolOs commented Mar 20, 2025

dbaeumer commented Mar 20, 2025

MariaSolOs commented Mar 20, 2025

MariaSolOs Mar 27, 2025

MariaSolOs Mar 27, 2025

dbaeumer Mar 31, 2025

MariaSolOs Mar 27, 2025

dbaeumer Mar 31, 2025

MariaSolOs commented Mar 28, 2025

dbaeumer Mar 31, 2025

dbaeumer Mar 31, 2025

dbaeumer Mar 31, 2025

MariaSolOs commented Apr 1, 2025

dbaeumer commented Apr 4, 2025

Labels

3 participants

	"markdownDescription": "The eslint options object to use on save. (see https://eslint.org/docs/developer-guide/nodejs-api#eslint-class). eslint.codeActionsOnSave.rules, if specified, will take priority over any rule options here."
	"markdownDescription": "The ESLint options object to use on save (see https://eslint.org/docs/developer-guide/nodejs-api#eslint-class). `eslint.codeActionsOnSave.rules`, if specified, will take priority over any rule options here."

	eslintOptions = {...overrideOptions, ...eslintOptions};
	eslintOptions = {...eslintOptions, ...overrideOptions};

Uh oh!

Conversation

JavaScriptBach commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Testing

JavaScriptBach commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

dbaeumer commented Oct 30, 2024

dbaeumer commented Oct 30, 2024

JavaScriptBach commented Oct 30, 2024

dbaeumer commented Oct 31, 2024

JavaScriptBach commented Nov 30, 2024

MariaSolOs commented Mar 20, 2025

dbaeumer commented Mar 20, 2025

MariaSolOs commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MariaSolOs commented Mar 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MariaSolOs commented Apr 1, 2025

dbaeumer commented Apr 4, 2025

Labels

3 participants

JavaScriptBach commented Oct 30, 2024 •

edited

Loading