Skip to content

Updates to support ESLint 9.x#61

Merged
khawkins merged 4 commits intosalesforce:mainfrom
khawkins:eslint_9
Jun 17, 2025
Merged

Updates to support ESLint 9.x#61
khawkins merged 4 commits intosalesforce:mainfrom
khawkins:eslint_9

Conversation

@khawkins
Copy link
Contributor

What does this PR do?

Updates our ESLint support to 9.x. I'll add comments inline.

@khawkins khawkins requested review from a team, dbreese, maliroteh-sf and sfdctaka as code owners June 10, 2025 20:46
const js = require('@eslint/js');
const nodePlugin = require('eslint-plugin-n');
const eslintPlugin = require('eslint-plugin-eslint-plugin');
const tseslint = require('typescript-eslint');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added TypeScript linting for the index.d.ts type declarations.

*/

const js = require('@eslint/js');
const nodePlugin = require('eslint-plugin-n');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

eslint-plugin-node is a project that has apparently fallen by the wayside. So the ESLint community forked it and went with eslint-plugin-n. This project, by contrast, gets a goodly amount of attention.

* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

'use strict';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are going to be a lot of these to ignore, unfortunately. Apparently this isn't required for modules, and throws linting exceptions if you keep them.

@@ -0,0 +1,21 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

base.js was migrated into base-legacy.js. This is the configuration that ESLint 8.x consumers will need to use.

*/

'use strict';
const bundleAnalyzer = require('../processor');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

base.js is now the ESLint flat config, supported by ESLint 9. ESLint 9 support is now the default.

* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

module.exports = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

recommended-legacy.js is the same as base-legacy.js. ESLint 8.x consumers will need to use these configs.

*/

'use strict';
const baseConfig = require('./base');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and recommended.js becomes the flat config for ESLint 9.x support.

const recommended = require('./configs/recommended');
const LwcBundle = require('./lwc-bundle');

const noGetterContainsMoreThanReturnStatement = require('./rules/no-getter-contains-more-than-return-statement');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These have all been moved into an aggregate export in rules/index.js. Makes the plugin spec easier to read.

recommended
base: require('./configs/base'),
recommended: require('./configs/recommended'),
'base-legacy': require('./configs/base-legacy'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both "current" and "legacy" configs are now available. Any ESLint engine >=7 will work with the plugin, as long as you use the right config.

Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to update the readme.md to let the user now how to use legacy config in ESLint 8. For example, for ESLint 8, use

{
    "extends": ["eslint:recommended", "plugin:@salesforce/lwc-graph-analyzer/recommended-legacy"]
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea. I'll update.

* by the ESLint CLI, or a dummy value (__placeholder__.js) if called programatically.
* @returns A one-dimensional flattened array of messages.
*/
// eslint-disable-next-line no-unused-vars
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason no-unused-vars no longer flags here, and honestly I can't figure out why. ¯\(ツ)

@@ -0,0 +1,40 @@
module.exports = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the rule definitions moved in here. Keep all the clutter in one place.

* @param {string} blockFilename - The filename of the code block being processed
* @returns {boolean} True if the code block should be processed (has .js or .html extension), false otherwise
*/
function createLinter(rulename) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the options specified in here were legacy 8.x options, I just got rid of the method and added const linter = new Linter(); where it was necessary.

linter.defineParser('@babel/eslint-parser', babelParser);
linter.defineRule(rulename, lwcGraphAnalyzer.rules[rulename]);
return linter;
function filterCodeBlock(blockFilename) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out this (and its use below) is important for calling Linter.verify() programmatically. Without it, ESLint will just silently drop any files coming out of preprocessing, that don't have a .js extension. So .html files were not working.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the linter.verify(srcCode, config, { filename: testPath, filterCodeBlock }), the config already has the html file configured. If that is not enough, does that mean lgpt-core lint has the same issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not enough, and lgpt-core will have the same issue. We just hadn't run into it before, because we hadn't been working with linting the .html files. I'm going to create a work item to be able to provide those options as part of linting.


return linter.verify(srcCode, config, {
filename: testPath,
preprocess: lwcGraphAnalyzer.processors.bundleAnalyzer.preprocess,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has all just been normalized to standard ESLint config, so the config returned by createLinterConfig() is sufficient to capture the proper options now.

const { RuleTester } = require('eslint');
const { RULE_TESTER_CONFIG } = require('./shared');
const lwcGraphAnalyzer = require('../../../lib/index');
const bundleAnalyzer = lwcGraphAnalyzer.processors.bundleAnalyzer;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Starting from here, you'll see this across the rest of the test/lib/rules tests. The updates are all the same: remove the weird configuration override from 8.x where there was no other way to specify a processor, in favor of the ESLint config in shared.js, which defines it.

babelOptions: {
parserOpts: {
plugins: [['decorators', { decoratorsBeforeExport: false }]]
languageOptions: {
Copy link
Contributor Author

@khawkins khawkins Jun 11, 2025

Choose a reason for hiding this comment

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

Here's the shared ESLint config update that replaces all of those test configs above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to define the 'files' sections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can. It was part oversight, part the fact that there aren't other configurations that interfere with ours in a test environment. But per my previous comment, I think it would be beneficial to standardize this config in both production and test, now that we can.

const lwcGraphAnalyzer = require('../../../lib/index');
const bundleAnalyzer = lwcGraphAnalyzer.processors.bundleAnalyzer;
const ruleTester = new RuleTester(RULE_TESTER_CONFIG);

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the ESLint 9 flat config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is an ESLint 9 flat config. But you got me thinking, now that configs are way more normalized between different scenarios (production vs. test configs, etc.) it would probably be good to have a common config, just so that we're implicitly testing our config setup as part of our testing. I'll make a change here.

Copy link
Contributor

@haifeng-li-at-salesforce haifeng-li-at-salesforce left a comment

Choose a reason for hiding this comment

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

The changes look great! The only remaining update is to the Readme.md to reflect support for both ESLint v8 and v9.

@khawkins
Copy link
Contributor Author

Alright, @haifeng-li-at-salesforce let me know if you want to take another look. I updated the README with ESLint version-specific configuration instructions, and standardized the test configs to work from the base config that we ship.

@khawkins khawkins merged commit 6e8ec0d into salesforce:main Jun 17, 2025
7 checks passed
@khawkins khawkins deleted the eslint_9 branch June 17, 2025 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants