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.

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

Bug: [flat config] ESLint continues linting files after an error occurs #17621

Closed
1 task done
mdjermanovic opened this issue Oct 5, 2023 · 4 comments · Fixed by #18155
Closed
1 task done

Bug: [flat config] ESLint continues linting files after an error occurs #17621

mdjermanovic opened this issue Oct 5, 2023 · 4 comments · Fixed by #18155
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features repro:yes

Comments

@mdjermanovic
Copy link
Member

mdjermanovic commented Oct 5, 2023

Environment

Node version: v20.7.0
npm version: v10.1.0
Local ESLint version: v8.50.0 (Currently used)
Global ESLint version: Not found
Operating System: win32 10.0.19045

What parser are you using?

Default (Espree)

What did you do?

I updated the no-undef rule to throw an error on the first file it gets. Then I ran npx eslint .

What did you expect to happen?

ESLint to print the error and then finish right after.

What actually happened?

ESLint prints the error, but after that it takes 20 seconds for the command prompt to appear again because ESLint keeps linting other files after the error is thrown. This seems unnecessary as it will not change the outcome.

Link to Minimal Reproducible Example

git fetch origin
git checkout origin/issue17621-repro
npx eslint .

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

I believe this happens because all tasks have already been added to the queue. If we want to fix this behavior, the change would probably be in this part of the code:

/*
* Because we need to process multiple files, including reading from disk,
* it is most efficient to start by reading each file via promises so that
* they can be done in parallel. Then, we can lint the returned text. This
* ensures we are waiting the minimum amount of time in between lints.
*/
const results = await Promise.all(
filePaths.map(({ filePath, ignored }) => {
/*
* If a filename was entered that matches an ignore
* pattern, then notify the user.
*/
if (ignored) {
if (warnIgnored) {
return createIgnoreResult(filePath, cwd);
}
return void 0;
}
const config = configs.getConfig(filePath);
/*
* Sometimes a file found through a glob pattern will
* be ignored. In this case, `config` will be undefined
* and we just silently ignore the file.
*/
if (!config) {
return void 0;
}
// Skip if there is cached result.
if (lintResultCache) {
const cachedResult =
lintResultCache.getCachedLintResults(filePath, config);
if (cachedResult) {
const hadMessages =
cachedResult.messages &&
cachedResult.messages.length > 0;
if (hadMessages && fix) {
debug(`Reprocessing cached file to allow autofix: ${filePath}`);
} else {
debug(`Skipping file since it hasn't changed: ${filePath}`);
return cachedResult;
}
}
}
// set up fixer for fixTypes if necessary
let fixer = fix;
if (fix && fixTypesSet) {
// save original value of options.fix in case it's a function
const originalFix = (typeof fix === "function")
? fix : () => true;
fixer = message => shouldMessageBeFixed(message, config, fixTypesSet) && originalFix(message);
}
return fs.readFile(filePath, "utf8")
.then(text => {
// do the linting
const result = verifyText({
text,
filePath,
configs,
cwd,
fix: fixer,
allowInlineConfig,
reportUnusedDisableDirectives,
linter
});
/*
* Store the lint result in the LintResultCache.
* NOTE: The LintResultCache will remove the file source and any
* other properties that are difficult to serialize, and will
* hydrate those properties back in on future lint runs.
*/
if (lintResultCache) {
lintResultCache.setCachedLintResults(filePath, config, result);
}
return result;
});
})
);

@fasttime
Copy link
Member

fasttime commented Oct 5, 2023

I am able to reproduce this issue. Marking as accepted as there is agreement that this is a bug in the comments to #17560.

@fasttime fasttime added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion repro:yes and removed repro:needed labels Oct 5, 2023
@arka1002
Copy link
Contributor

arka1002 commented Nov 3, 2023

Hi, I just had a question - the throw which is coming from no-undef.js, that isnt failing the Promise.all ?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all#promise.all_fail-fast_behavior

@mdjermanovic
Copy link
Member Author

Hi, I just had a question - the throw which is coming from no-undef.js, that isnt failing the Promise.all ?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all#promise.all_fail-fast_behavior

It does fail, but it doesn't abort pending promises.

console.time("track");

Promise.all([
    Promise.reject(),
    new Promise(resolve =>
        setTimeout(() => {
            resolve();
            console.timeLog("track", "settled second Promise");
        }, 10000)
    )
]).catch(() => {
    console.timeLog("track", "failed Promise.all");
});

process.on("exit", () => {
    console.timeLog("track", "exit");
});

/*

track: 0.496ms failed Promise.all
track: 10.006s settled second Promise
track: 10.007s exit

*/

@snitin315 snitin315 self-assigned this Jan 2, 2024
@snitin315
Copy link
Contributor

I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features repro:yes
Projects
Archived in project
5 participants
@fasttime @mdjermanovic @snitin315 @arka1002 and others