-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
ESLint v10 support #3230
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.
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
base: main
Are you sure you want to change the base?
ESLint v10 support #3230
Changes from 2 commits
dde5fc7
d8d7294
6f9ca49
e66a4a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,7 +81,7 @@ | |
| "chai": "^4.3.10", | ||
| "cross-env": "^4.0.0", | ||
| "escope": "^3.6.0", | ||
| "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9", | ||
| "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9 || ^10", | ||
| "eslint-doc-generator": "^1.6.1", | ||
| "eslint-import-resolver-node": "file:./resolvers/node", | ||
| "eslint-import-resolver-typescript": "^1.0.2 || ^1.1.1", | ||
|
|
@@ -91,6 +91,8 @@ | |
| "eslint-plugin-eslint-plugin": "^2.3.0", | ||
| "eslint-plugin-import": "2.x", | ||
| "eslint-plugin-json": "^2.1.2", | ||
| "eslint-plugin-jsonc": "^2.21.1", | ||
| "espree": "^9", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this pin needed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These become unnecessary if we use a fixture as in #3252 |
||
| "find-babel-config": "=1.2.0", | ||
| "fs-copy-file-sync": "^1.1.1", | ||
| "glob": "^7.2.3", | ||
|
|
@@ -112,7 +114,7 @@ | |
| "typescript-eslint-parser": "^15 || ^20 || ^22" | ||
| }, | ||
| "peerDependencies": { | ||
| "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9" | ||
| "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9 || ^10" | ||
| }, | ||
| "dependencies": { | ||
| "@rtsao/scc": "^1.1.0", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,9 @@ export default function processSpecifier(specifier, astNode, exportMap, namespac | |
| })); | ||
| return; | ||
| case 'ExportAllDeclaration': | ||
| exportMap.namespace.set(specifier.exported.name || specifier.exported.value, namespace.add(exportMeta, specifier.source.value)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this means it's not longer doing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to #3250 |
||
| exportMap.namespace.set(specifier.exported.name || specifier.exported.value, Object.defineProperty(exportMeta, 'namespace', { | ||
| get() { return namespace.resolveImport(specifier.source.value); }, | ||
| })); | ||
| return; | ||
| case 'ExportSpecifier': | ||
| if (!astNode.source) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,9 +57,12 @@ export default class ImportExportVisitorBuilder { | |
| }, | ||
| ExportAllDeclaration() { | ||
| const getter = captureDependency(astNode, astNode.exportKind === 'type', this.remotePathResolver, this.exportMap, this.context, this.thunkFor); | ||
| if (getter) { this.exportMap.dependencies.add(getter); } | ||
| if (astNode.exported) { | ||
| // `export * as ns from './mod'` — named namespace, not a star-export | ||
| processSpecifier(astNode, astNode.exported, this.exportMap, this.namespace); | ||
| } else if (getter) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, Is this intentional? If so, does the lazy getter in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cycle detection reads LMK if I should comment inline or update the "ExportMap export * as fix" section in the PR to clarify/note this!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Broken out into #3250 |
||
| // `export * from './mod'` — star-export flattens into current module | ||
| this.exportMap.dependencies.add(getter); | ||
| } | ||
| }, | ||
| /** capture namespaces in case of later export */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import readPkgUp from 'eslint-module-utils/readPkgUp'; | |
| import values from 'object.values'; | ||
| import includes from 'array-includes'; | ||
| import flatMap from 'array.prototype.flatmap'; | ||
| import minimatch from 'minimatch'; | ||
|
|
||
| import ExportMapBuilder from '../exportMap/builder'; | ||
| import recursivePatternCapture from '../exportMap/patternCapture'; | ||
|
|
@@ -147,6 +148,86 @@ function listFilesWithLegacyFunctions(src, extensions) { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Walk a directory recursively, collecting file paths that match the given extensions. | ||
| * Skips node_modules and dot-directories. | ||
| * @param {string} dir - directory to walk | ||
| * @param {string[]} extensions - list of supported file extensions | ||
| * @param {string[]} results - accumulator for matched file paths | ||
| * @param {object} fs - Node.js fs module | ||
| * @param {Function} join - path.join | ||
| * @param {Function} extname - path.extname | ||
| * @returns {string[]} list of matched file paths | ||
| */ | ||
| function walkDirectory(dir, extensions, results, fs, join, extname) { | ||
| let entries; | ||
| try { | ||
| entries = fs.readdirSync(dir, { withFileTypes: true }); | ||
| } catch (e) { | ||
| return results; | ||
| } | ||
|
|
||
| for (let i = 0; i < entries.length; i++) { | ||
| const entry = entries[i]; | ||
| if (entry.name[0] === '.' || entry.name === 'node_modules') { | ||
| continue; | ||
| } | ||
| const fullPath = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walkDirectory(fullPath, extensions, results, fs, join, extname); | ||
| } else if (entry.isFile() && extensions.indexOf(extname(fullPath)) > -1) { | ||
| results.push(fullPath); | ||
| } | ||
| } | ||
|
|
||
| return results; | ||
| } | ||
|
|
||
| /** | ||
| * List files using Node.js fs and minimatch as a fallback when FileEnumerator | ||
| * and legacy ESLint APIs are unavailable (ESLint v10+). | ||
| * @param {string[]} src - list of file paths, directories, or glob patterns | ||
| * @param {string[]} extensions - list of supported file extensions | ||
| * @returns {string[]} list of matched file paths | ||
| */ | ||
| function listFilesWithNodeFs(src, extensions) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why wouldn't this function work on eslint 9?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would work, just not reached as a fallback. Updated comment to clarify! |
||
| const fs = require('fs'); | ||
| const { join, resolve, extname } = require('path'); | ||
| const isGlob = require('is-glob'); | ||
|
|
||
| extensions = extensions.map((ext) => ext.startsWith('.') ? ext : `.${ext}`); | ||
| const results = []; | ||
|
|
||
| src.forEach((pattern) => { | ||
| if (isGlob(pattern)) { | ||
| // For glob patterns, walk the base directory and filter with minimatch | ||
| // Extract the base directory from the glob (everything before the first glob character) | ||
| const base = pattern.replace(/[*?{[].*/g, '').replace(/\/[^/]*$/, '') || '.'; | ||
| const resolvedBase = resolve(base); | ||
| const allFiles = walkDirectory(resolvedBase, extensions, [], fs, join, extname); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The glob base extraction Also,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it okay to just use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can also use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use glob if you use a version that's old enough to support the same node versions as us.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the base extraction (and brace expansion / recursion) so it no longer falls back to |
||
| allFiles.forEach((file) => { | ||
| if (minimatch(file, resolve(pattern)) || minimatch(file, pattern)) { | ||
| results.push(file); | ||
| } | ||
| }); | ||
| } else { | ||
| const resolved = resolve(pattern); | ||
| try { | ||
| const stat = fs.statSync(resolved); | ||
| if (stat.isDirectory()) { | ||
| walkDirectory(resolved, extensions, results, fs, join, extname); | ||
| } else if (stat.isFile() && extensions.indexOf(extname(resolved)) > -1) { | ||
| results.push(resolved); | ||
| } | ||
| } catch (e) { | ||
| // Path doesn't exist, skip it | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return results; | ||
| } | ||
|
|
||
| /** | ||
| * Given a src pattern and list of supported extensions, return a list of files to process | ||
| * with this rule. | ||
|
|
@@ -162,7 +243,15 @@ function listFilesToProcess(src, extensions) { | |
| return listFilesUsingFileEnumerator(FileEnumerator, src, extensions); | ||
| } | ||
| // If not, then we can try even older versions of this capability (listFilesToProcess) | ||
| return listFilesWithLegacyFunctions(src, extensions); | ||
| try { | ||
| return listFilesWithLegacyFunctions(src, extensions); | ||
| } catch (e) { | ||
| // If legacy functions are also unavailable (ESLint v10+), use Node.js fs as a fallback | ||
| if (e.code === 'MODULE_NOT_FOUND' || e.code === 'ERR_PACKAGE_PATH_NOT_EXPORTED') { | ||
| return listFilesWithNodeFs(src, extensions); | ||
| } | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| const EXPORT_DEFAULT_DECLARATION = 'ExportDefaultDeclaration'; | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import jsoncPlugin from 'eslint-plugin-jsonc'; | ||
|
|
||
| export default [ | ||
| ...jsoncPlugin.configs['flat/recommended-with-json'], | ||
| { | ||
| files: ['tests/files/just-json-files/*.json'], | ||
| rules: { | ||
| 'import/no-unused-modules': [ | ||
| 'error', | ||
| { | ||
| missingExports: false, | ||
| unusedExports: true, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| ]; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably need to exclude node 18 as well since eslint 10 supports from node 20
https://github.com/eslint/eslint/releases/tag/v10.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the test suite still runs and passes on Node 18 & 19 + eslint 10, so I left these in, but defer to @ljharb! See e68ea07, it's just the
@babel/eslint-parser-dependent ones that are skipped.