[heft-lint-plugin] Add support for ESLint 9 #5219
Conversation
…er/danade/UpdateEslint2
| // | ||
| // const { CONSTANT1, CONSTANT2 } = someNamespace.constants; | ||
| // | ||
| // Thus for now "property" is more like a variable than a class member. |
There was a problem hiding this comment.
Per ECMAScript, the above examples are, in fact, values of type "Property" so this comment is weird.
|
|
||
| // Genuine properties | ||
| { | ||
| selectors: ['parameterProperty', 'accessor'], |
There was a problem hiding this comment.
If parameterProperty the deprecated constructor(public foo: string) nonsense or destructured parameters, e.g. function ({ foo }: { foo: string })?
| import type * as TTypescript from 'typescript'; | ||
| import type * as TEslint from 'eslint'; | ||
| import path from 'node:path'; | ||
| import { createHash, type Hash } from 'crypto'; |
There was a problem hiding this comment.
| import { createHash, type Hash } from 'crypto'; | |
| import { createHash, type Hash } from 'node:crypto'; |
| import type * as TTypescript from 'typescript'; | ||
| import type * as TEslint from 'eslint'; | ||
| import path from 'node:path'; | ||
| import { createHash, type Hash } from 'crypto'; | ||
| import { performance } from 'perf_hooks'; |
There was a problem hiding this comment.
| import { performance } from 'perf_hooks'; | |
| import { performance } from 'node:perf_hooks'; |
| let programs: TTypescript.Program[] | undefined; | ||
| if (this.languageOptions?.parserOptions?.programs) { | ||
| programs = this.languageOptions.parserOptions.programs; | ||
| delete this.languageOptions.parserOptions.programs; |
There was a problem hiding this comment.
Can we set this to undefined instead? The delete operator is a guaranteed deopt of property accesses from the object, and JSON serialization ignores properties with a value of undefined.
| const eslintBaseConfiguration: any = await this._linter.calculateConfigForFile( | ||
| this._linterConfigFilePath | ||
| protected override async getCacheVersionAsync(): Promise<string> { | ||
| return `${this._eslintPackageVersion.version}_${process.version}`; |
There was a problem hiding this comment.
Is the version of Node.js really relevant to the eslint rule evaluation?
| // The eslint configuration object contains a toJSON() method that returns a serializable version of the | ||
| // configuration. However, we are manually injecting the TypeScript program into the parserOptions, which | ||
| // is not serializable. Patch the function to remove the program before returning the serializable version. |
There was a problem hiding this comment.
What happens if we instead inject it using Object.defineProperty and don't make it enumerable?
| this._fixesPossible = | ||
| this._fixesPossible || |
There was a problem hiding this comment.
| this._fixesPossible = | |
| this._fixesPossible || | |
| this._fixesPossible ||= |
| // if (message.source) { | ||
| // physicalLocation.region ??= {}; | ||
| // physicalLocation.region.snippet = { | ||
| // text: message.source |
There was a problem hiding this comment.
Is message.source no longer a field?
| const { defineConfig } = require('eslint/config'); | ||
| const friendlyLocalsMixin = require('local-eslint-config/flat/mixins/friendly-locals'); | ||
|
|
||
| module.exports = defineConfig([...friendlyLocalsMixin]); |
There was a problem hiding this comment.
Is there a purpose to duplicating the array or could we just pass it directly?
There was a problem hiding this comment.
Duplicating to allow for easy update in the future. Can be left as an array directly if we wanted to.
…er/danade/UpdateEslint2
…er/danade/UpdateEslint2
…er/danade/UpdateEslint2
Summary
NOTE: Once merged, a publish and bump of decoupled dependencies will be required to update the remaining projects. The new configs should be moved into the decoupled rig, which currently is the source for the configs for the
local-node-rigconfigs.How it was tested
This PR.
Impacted documentation
Any ESLint-related docs will need to be updated to indicate support.