Skip to content

Commit

Permalink
fix(scripts-lint-staged): correctly resolve package groupping based o…
Browse files Browse the repository at this point in the history
…n provided git stagged files
  • Loading branch information
Hotell committed May 29, 2024
1 parent 287ce45 commit 1776647
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 42 deletions.
14 changes: 8 additions & 6 deletions lint-staged.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ const commands = {
lint: 'node ./scripts/lint-staged/src/eslint',
};

const nonJsExtensions = [
prettierSupportedFileExtensionsByContext.stylesheets,
prettierSupportedFileExtensionsByContext.markdown,
prettierSupportedFileExtensionsByContext.others,
].flat();

// https://www.npmjs.com/package/lint-staged
module.exports = {
[`**/*.{${[].concat(
prettierSupportedFileExtensionsByContext.stylesheets,
prettierSupportedFileExtensionsByContext.markdown,
prettierSupportedFileExtensionsByContext.others,
)}}`]: [commands.format],
[`**/*.{${prettierSupportedFileExtensionsByContext.js}}`]: [commands.format, commands.lint],
[`**/*.{${nonJsExtensions}}`]: [commands.format],
[`**/*.{${prettierSupportedFileExtensionsByContext.js}}`]: [/* commands.format, */ commands.lint],
};
3 changes: 2 additions & 1 deletion scripts/lint-staged/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"type-check": "just-scripts type-check"
},
"dependencies": {
"@fluentui/scripts-monorepo": "*"
"@fluentui/scripts-monorepo": "*",
"@fluentui/scripts-utils": "*"
},
"devDependencies": {}
}
9 changes: 5 additions & 4 deletions scripts/lint-staged/src/eslint-for-package.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// @ts-check

const fs = require('fs');
const path = require('path');

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

/**
Expand All @@ -21,8 +21,9 @@ const micromatch = require('micromatch');
async function run() {
// Get information needed to determine whether files in this package should be linted
const packagePath = process.cwd();
const packageJson = fs.readJSONSync(path.join(packagePath, 'package.json'));
const lintScript = packageJson.scripts.lint || '';
/** @type {{scripts?:Record<string,string>}} */
const packageJson = JSON.parse(fs.readFileSync(path.join(packagePath, 'package.json'), 'utf-8'));
const lintScript = packageJson.scripts?.lint ?? '';
/** @type {import('eslint').ESLint} */
let eslint;
/** @type {string} */
Expand All @@ -43,7 +44,7 @@ async function run() {
eslint = new ESLint({ fix: true, cache: lintScript.includes('--cache') });
}

// files are provided by @see `./eslint.js` via cli
// files are provided by @see `file://./eslint.js` via cli
const cliFiles = process.argv.slice(2);
// Filter out files with non-linted extensions
const files = cliFiles.filter(file => micromatch.isMatch(file, includePattern));
Expand Down
62 changes: 31 additions & 31 deletions scripts/lint-staged/src/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const os = require('os');
const path = require('path');
const { promisify } = require('util');

const { getWorkspaceProjects, workspaceRoot } = require('@fluentui/scripts-monorepo');
const { findConfig } = require('@fluentui/scripts-utils');
const { default: PQueue } = require('p-queue');
const exec = promisify(child_process.exec);

Expand All @@ -16,69 +16,64 @@ const exec = promisify(child_process.exec);
*/
const eslintForPackageScript = path.join(__dirname, 'eslint-for-package.js');

const files = process.argv.slice(2);

/**
* Since we have an eslint config file per package we need to respect this when running
* eslint for staged files. To do this we group the files per package name. This function takes
* a list of package names and returns an object with the package root as the key and the files
* in that package as the value.
*
* @param {string[]} files
* @returns {{ [packagePath: string]: string[] }}
*/
function groupFilesByPackage() {
function groupFilesByPackage(files) {
/** @type {{ [packagePath: string]: string[] }} */
const filesByPackage = {};

const packagesWithEslint = [];
const projects = getWorkspaceProjects();
for (const [, projectConfig] of projects) {
const absoluteRootPath = path.join(workspaceRoot, projectConfig.root);
if (fs.readdirSync(absoluteRootPath).some(f => f.startsWith('.eslintrc'))) {
packagesWithEslint.push(absoluteRootPath);
for (const file of files) {
const packagePath = findConfig('project.json', file)?.replace('/project.json', '');

if (!packagePath) {
continue;
}
}

for (const file of files) {
// eslint-disable-next-line no-shadow
const packagePath = packagesWithEslint.find(packagePath => {
// if file lives within searched package we will get only shortened absolute path `/src/abc.ts`
// we add `.` to make it relative and thus have match pattern to check upon
const normalizedFilePath = file.replace(packagePath, '.');
return normalizedFilePath.startsWith('./');
});
const hasEslintConfig =
fs.existsSync(path.join(packagePath, '.eslintrc.json')) || fs.existsSync(path.join(packagePath, '.eslintrc.js'));

// Exclude files in a package without an eslintrc (or not in a package at all)
if (packagePath) {
if (!filesByPackage[packagePath]) {
filesByPackage[packagePath] = [];
}
filesByPackage[packagePath].push(file);
if (!hasEslintConfig) {
continue;
}

if (!filesByPackage[packagePath]) {
filesByPackage[packagePath] = [];
}
filesByPackage[packagePath].push(file);
}

return filesByPackage;
}

/**
* Runs eslint for the staged files in the packages that require it.
* @param {string[]} files
*/
async function runEslintOnFilesGroupedPerPackage() {
const filesGroupedByPackage = groupFilesByPackage();
async function runEslintOnFilesGroupedPerPackage(files) {
const filesGroupedByPackage = groupFilesByPackage(files);

// Log an empty line on error to make the eslint output look better
console.log('');
console.log('', filesGroupedByPackage);

const queue = new PQueue({ concurrency: os.cpus().length / 2 });
let hasError = false;

await queue.addAll(
// eslint-disable-next-line no-shadow
Object.entries(filesGroupedByPackage).map(([packagePath, files]) => async () => {
Object.entries(filesGroupedByPackage).map(([packageRootAbsolutePath, files]) => async () => {
// This script handles running eslint on ONLY touched files for each package.
const cmd = `node ${eslintForPackageScript} ${files.join(' ')}`;
console.log(`${cmd}`);

return (
exec(cmd, { cwd: packagePath })
exec(cmd, { cwd: packageRootAbsolutePath })
// Log severity:error lint reports including severity:warn
// this will also result in killing the process
.catch((/** @type {{ stdout: string, stderr: string }} */ err) => {
Expand Down Expand Up @@ -106,7 +101,12 @@ async function runEslintOnFilesGroupedPerPackage() {
}

function main() {
runEslintOnFilesGroupedPerPackage().catch(err => {
/**
* Files that are staged for commit - Absolute paths
*/
const files = process.argv.slice(2);

runEslintOnFilesGroupedPerPackage(files).catch(err => {
console.error(err);
process.exit(1);
});
Expand Down

0 comments on commit 1776647

Please sign in to comment.