Skip to content

Commit

Permalink
fix(scripts-lint-staged): dont fail lint process if it contains only …
Browse files Browse the repository at this point in the history
…severity 'warn' violations and improve output format (microsoft#30173)
  • Loading branch information
Hotell authored Jan 5, 2024
1 parent 398758e commit 6512201
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 16 deletions.
22 changes: 17 additions & 5 deletions scripts/lint-staged/eslint-for-package.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// @ts-check

const path = require('path');

const { eslintConstants } = require('@fluentui/scripts-monorepo');
const { ESLint } = require('eslint');
const fs = require('fs-extra');
const micromatch = require('micromatch');
const { eslintConstants } = require('@fluentui/scripts-monorepo');

/**
* Run ESLint for certain files from a particular package.
Expand Down Expand Up @@ -57,19 +58,30 @@ async function run() {

// Lint files then fix all auto-fixable issues
const results = await eslint.lintFiles(filteredFiles);
const hasSeverityError = results.some(lintResult => lintResult.errorCount > 0);

await ESLint.outputFixes(results);

// Format results
const formatter = await eslint.loadFormatter();
const resultText = formatter.format(results);
if (resultText) {
throw new Error(resultText);

if (!resultText) {
return;
}

if (!hasSeverityError) {
// print lint output warnings
// - logging is handled by ./eslint.js. you'll see this output only if you directly call this script
console.log(resultText);
return;
}

// this error throw will be processed by parent process (./eslint.js)
// print lint errors ( and warnings if present )
throw new Error(resultText);
}

run().catch(err => {
// logging is handled by ./eslint.js. If you wanna directly call this script and see errors uncomment following line ↓
// console.error(err);
throw new Error(err);
});
35 changes: 24 additions & 11 deletions scripts/lint-staged/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ const { rollup: lernaAliases } = require('lerna-alias');
const { default: PQueue } = require('p-queue');
const exec = promisify(child_process.exec);

/**
* @see file://./eslint-for-package.js
*/
const eslintForPackageScript = path.join(__dirname, 'eslint-for-package.js');

const files = process.argv.slice(2);
Expand Down Expand Up @@ -69,20 +72,26 @@ async function runEslintOnFilesGroupedPerPackage() {
await queue.addAll(
// eslint-disable-next-line no-shadow
Object.entries(filesGroupedByPackage).map(([packagePath, files]) => async () => {
// This script handles running eslint on ONLY touched files for each package.
const cmd = `node ${eslintForPackageScript} ${files.join(' ')}`;

// This script handles running eslint on ONLY the appropriate files for each package.
// See its comments for more details.
return exec(cmd, { cwd: packagePath }).catch((/** @type {{ stdout: string, stderr: string }} */ err) => {
// The subprocess should already have handled logging. Just mark that there was an error.
hasError = true;
throw new Error(err.stderr);
});
return (
exec(cmd, { cwd: packagePath })
// Log severity:error lint reports including severity:warn
// this will also result in killing the process
.catch((/** @type {{ stdout: string, stderr: string }} */ err) => {
hasError = true;
// child throws twice so to mitigate `Error: Error: ` output we remove the 1st one
const report = err.stderr.replace('Error: ', '');
console.error(report);
})
);
}),
);

await queue
.onEmpty()
// handle any internal exec errors
.catch(error => {
console.error(error);
hasError = true;
Expand All @@ -94,7 +103,11 @@ async function runEslintOnFilesGroupedPerPackage() {
});
}

runEslintOnFilesGroupedPerPackage().catch(err => {
console.error(err);
process.exit(1);
});
function main() {
runEslintOnFilesGroupedPerPackage().catch(err => {
console.error(err);
process.exit(1);
});
}

main();

0 comments on commit 6512201

Please sign in to comment.