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

Add Stylelint checks and checks output consistency #377

Merged
merged 100 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
100 commits
Select commit Hold shift + click to select a range
7e3aaef
Add stylelint check
Josehower Apr 11, 2023
716c456
Fix function name
Josehower Apr 11, 2023
6ba0ca6
Fix casing on stilelint
Josehower Apr 12, 2023
12d4edf
Merge branch 'main' into add-stylelint-check
Josehower Apr 18, 2023
e8323b9
Add stylelint config validation
Josehower Apr 18, 2023
0a9492e
Add additional file extentions
Josehower Apr 18, 2023
1ce0624
Update test
Josehower Apr 18, 2023
0e33e62
Add exeptions
Josehower Apr 18, 2023
ab3a404
Merge branch 'main' into add-stylelint-check
Josehower Apr 19, 2023
9e6adb6
Add colors
Josehower Apr 19, 2023
efdedc9
Fix error in colors
Josehower Apr 19, 2023
d5ee5cb
Merge branch 'main' into add-stylelint-check
karlhorky Apr 19, 2023
4ad180a
Add icons
Josehower Apr 19, 2023
100eb41
Merge branch 'main' into add-stylelint-check
Josehower Apr 19, 2023
fe2703c
Revert "Add icons"
Josehower Apr 19, 2023
121137d
Add falback renderer
Josehower Apr 19, 2023
93c05a5
Update src/checks/stylelingConfigIsValid.ts
karlhorky Apr 19, 2023
036d42b
Update src/checks/stylelingConfigIsValid.ts
karlhorky Apr 19, 2023
98685c1
Update src/checks/stylelingConfigIsValid.ts
karlhorky Apr 19, 2023
3a011d6
Update snapshot
Josehower Apr 19, 2023
ccdcd97
Merge branch 'main' into add-stylelint-check
karlhorky Apr 19, 2023
f7b08fb
Fix snapshot
karlhorky Apr 19, 2023
2e003d0
Update src/checks/stylelint.ts
karlhorky Apr 19, 2023
ff50c24
Update filename
Josehower Apr 19, 2023
7223e6a
Update src/checks/stylelintConfigIsValid.ts
karlhorky Apr 19, 2023
8668e22
Update e2e.test.ts.snap
karlhorky Apr 19, 2023
0f177ff
Merge branch 'main' into add-stylelint-check
Josehower Apr 19, 2023
3a70b0a
Use yarn until package is updated
Josehower Apr 19, 2023
bfa325a
Update snapshot
Josehower Apr 19, 2023
f0dbdb4
Use Json output format instead of string
Josehower Apr 20, 2023
68f0436
Update string matching stylelint disable
Josehower Apr 20, 2023
235c684
Use pnpm instad of yarn
Josehower Apr 20, 2023
cab51a4
Simplify variable
Josehower Apr 20, 2023
b70fd5e
Fix eslint errors
Josehower Apr 20, 2023
ac8c711
Get type from stylelint
Josehower Apr 20, 2023
58f22fa
Update supported extentions
Josehower Apr 20, 2023
1de52df
Use join() instead of toString()
Josehower Apr 20, 2023
e27e30f
Use version without period
Josehower Apr 20, 2023
01e6fe2
Use Stylelint instead of ESLint
Josehower Apr 20, 2023
31b91f6
Use periodless pattern and add file deletion
Josehower Apr 20, 2023
01b4935
Use parameter
Josehower Apr 20, 2023
098ee24
Add conditional to check react projects
Josehower Apr 20, 2023
4e0263f
Move conditional to the top of the file
Josehower Apr 20, 2023
a1834a0
Update fileExtensions
Josehower Apr 20, 2023
8894a6f
Update variable names
Josehower Apr 20, 2023
3ab3487
Fix indentation and variable names
Josehower Apr 20, 2023
fe92614
Fix capitalization
Josehower Apr 20, 2023
4f60f7e
update variable names
Josehower Apr 20, 2023
b43292c
Update conditional to match codebase
Josehower Apr 20, 2023
1c086c7
Update error message to improve clarity
Josehower Apr 20, 2023
49cca45
Remve unnecesary if exists in message
Josehower Apr 20, 2023
60fea13
Add early return in stylelint check too
Josehower Apr 20, 2023
18d25ba
Update src/checks/stylelint.ts
Josehower Apr 20, 2023
200a1b7
Update src/checks/eslintConfigIsValid.ts
Josehower Apr 20, 2023
2a7b55f
Update src/checks/stylelintConfigIsValid.ts
Josehower Apr 20, 2023
4341d21
Merge branch 'main' into add-stylelint-check
Josehower Apr 20, 2023
d0d08f4
Update command to pnpm
Josehower Apr 20, 2023
7f8a26e
Fix error in command
Josehower Apr 20, 2023
85ce0f7
Update message to match codebase
Josehower Apr 20, 2023
241b7a7
Update src/checks/stylelint.ts
Josehower Apr 20, 2023
9004208
Group and remove checks based on project
Josehower Apr 20, 2023
048811e
Revert "Group and remove checks based on project"
Josehower Apr 20, 2023
b93eaec
Remove stylelint for no react projects
Josehower Apr 20, 2023
9fa5740
Remove extra line
Josehower Apr 20, 2023
3777e9d
Simplify pattern
Josehower Apr 20, 2023
42eff47
Use variable in regex
Josehower Apr 20, 2023
1c9b999
Simplify variable names
Josehower Apr 20, 2023
2807a58
Simplify logic
Josehower Apr 20, 2023
edd7eb6
Simplefy logic
Josehower Apr 20, 2023
e4fdce9
Update path to remove unnecesary extra information
Josehower Apr 20, 2023
c62cb54
Update Snapshot
Josehower Apr 20, 2023
e61eb09
Update prettier output to match pattern
Josehower Apr 20, 2023
b2c8786
Remove unnecesary comments
Josehower Apr 20, 2023
aea1845
Revert "Update Snapshot"
Josehower Apr 21, 2023
f12e8a8
Merge branch 'main' into add-stylelint-check
Josehower Apr 21, 2023
2140b0d
Update eslint check with Json output
Josehower Apr 21, 2023
b8a2d49
Simplify pretier path logic
Josehower Apr 21, 2023
1725584
Update variable name
Josehower Apr 21, 2023
e26e6b1
Use named import
karlhorky Apr 21, 2023
5b3f8c5
Shortcircuit algolia for @upleveled/react-scripts
Josehower Apr 21, 2023
da0bbac
Update src/checks/noDependencyProblems/noDependenciesWithoutTypes.ts
Josehower Apr 21, 2023
778935c
Fix potential break
Josehower Apr 21, 2023
f15f743
Update noDependenciesWithoutTypes.ts
karlhorky Apr 21, 2023
514f855
Fix warning
Josehower Apr 21, 2023
90bbed5
Move comment over the map
Josehower Apr 21, 2023
393195c
Update pattern
Josehower Apr 21, 2023
d3e8412
Simplify path manipulation
karlhorky Apr 21, 2023
761243f
2.1.0-0
karlhorky Apr 21, 2023
505c565
Add Windows paths examples
karlhorky Apr 21, 2023
b7b1fc8
Fix paths, add windows path
karlhorky Apr 21, 2023
c2edc82
Remove obsolete ignored files
karlhorky Apr 21, 2023
6f20af3
Parse the output to do more clear checks
Josehower Apr 21, 2023
83189bc
Update stylelint check and fix errors
Josehower Apr 21, 2023
f18804d
Validate ESLint JSON data for another edge case
karlhorky Apr 21, 2023
bec907d
Improve comment clarity
karlhorky Apr 21, 2023
e9ecf51
Fix spelling error
karlhorky Apr 21, 2023
560b36a
Copy pattern
karlhorky Apr 21, 2023
3b08941
Verify shape of Stylelint data earlier
karlhorky Apr 21, 2023
7a1eb7a
Merge branch 'main' into add-stylelint-check
Josehower Apr 21, 2023
1a2ff81
Merge branch 'main' of github.com:upleveled/preflight into add-stylel…
karlhorky Apr 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions __tests__/__snapshots__/e2e.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,6 @@

exports[`Passes in the react-passing test project 1`] = `
"πŸš€ UpLeveled Preflight
[COMPLETED] All changes committed to Git
[COMPLETED] ESLint
[COMPLETED] ESLint config is latest version
[COMPLETED] GitHub repo has deployed project link under About
[COMPLETED] No dependencies without types
[COMPLETED] No dependency problems
[COMPLETED] No extraneous files committed to Git
[COMPLETED] No secrets committed to Git
[COMPLETED] No unused dependencies
[COMPLETED] Preflight is latest version
[COMPLETED] Prettier
[COMPLETED] Project folder name matches correct format
[COMPLETED] Use single package manager
[COMPLETED] node_modules/ folder ignored in Git
[STARTED] All changes committed to Git
[STARTED] ESLint
[STARTED] ESLint config is latest version
Expand All @@ -28,8 +14,26 @@ exports[`Passes in the react-passing test project 1`] = `
[STARTED] Preflight is latest version
[STARTED] Prettier
[STARTED] Project folder name matches correct format
[STARTED] Stylelint
[STARTED] Stylelint config is latest version
[STARTED] Use single package manager
[STARTED] node_modules/ folder ignored in Git"
[STARTED] node_modules/ folder ignored in Git
[SUCCESS] All changes committed to Git
[SUCCESS] ESLint
[SUCCESS] ESLint config is latest version
[SUCCESS] GitHub repo has deployed project link under About
[SUCCESS] No dependencies without types
[SUCCESS] No dependency problems
[SUCCESS] No extraneous files committed to Git
[SUCCESS] No secrets committed to Git
[SUCCESS] No unused dependencies
[SUCCESS] Preflight is latest version
[SUCCESS] Prettier
[SUCCESS] Project folder name matches correct format
[SUCCESS] Stylelint
[SUCCESS] Stylelint config is latest version
[SUCCESS] Use single package manager
[SUCCESS] node_modules/ folder ignored in Git"
`;

exports[`Passes in the react-passing test project 2`] = `""`;
91 changes: 91 additions & 0 deletions src/checks/stylelingConfigIsValid.ts
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { promises as fs } from 'node:fs';
import { createRequire } from 'node:module';
import { execaCommand } from 'execa';
import readdirp from 'readdirp';
import semver from 'semver';

const require = createRequire(`${process.cwd()}/`);

export const title = 'Stylelint config is latest version';
karlhorky marked this conversation as resolved.
Show resolved Hide resolved

export default async function stylelintConfigIsValid() {
const { stdout: remoteVersion } = await execaCommand(
'npm show stylelint-config-upleveled version',
);

let localVersion;

try {
const stylelintConfigPackageJsonPath = require.resolve(
'stylelint-config-upleveled/package.json',
);

localVersion = JSON.parse(
await fs.readFile(stylelintConfigPackageJsonPath, 'utf-8'),
).version;
} catch (err) {}

if (typeof localVersion === 'undefined') {
throw new Error(
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I just realized this is going to be a bug for the Node.js projects

this is what the UpLeveled ESLint Config installer does:

if ('react-scripts' in projectDependencies || 'next' in projectDependencies) {
  newDevDependenciesToInstall.push('stylelint', 'stylelint-config-upleveled');
}

https://github.com/upleveled/eslint-config-upleveled/blob/df447996c8a01039f84489f31161e316be6cddd8/bin/install.js#L51-L53

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`The UpLeveled Stylelint config has not been installed. Please install using the instructions on https://www.npmjs.com/package/eslint-config-upleveled
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
`,
);
}

if (semver.gt(remoteVersion, localVersion)) {
throw new Error(
`Your current version of the UpLeveled styleLint config (${localVersion}) is out of date. The latest version is ${remoteVersion}. Upgrade by running:
karlhorky marked this conversation as resolved.
Show resolved Hide resolved

pnpm install stylelint-config-upleveled@${remoteVersion}`,
);
}

let stylelintConfigMatches;

try {
stylelintConfigMatches =
(await fs.readFile('./stylelint.config.cjs', 'utf-8')).trim() ===
`/** @type { import('stylelint').Config } */
const config = {
extends: ['stylelint-config-upleveled'],
};

module.exports = config;`;
} catch (err) {
throw new Error(
`Error reading your stylelint.config.cjs file. Please reinstall the config using the instructions on https://www.npmjs.com/package/eslint-config-upleveled
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I guess this error should mention that the config file should be deleted if it exists
  2. Probably we want to mention this for the same error in the ESLint config check
  3. the period can be changed to this pattern: d242231

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`,
);
}

if (!stylelintConfigMatches) {
throw new Error(
`Your Stylelint config file stylelint.config.cjs does not match the configuration file template. Please reinstall the config using the instructions on https://www.npmjs.com/package/eslint-config-upleveled
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I guess this error should mention that the config file should be deleted if it exists
  2. Probably we want to mention this for the same error in the ESLint config check
  3. the period can be changed to this pattern: d242231

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`,
);
}

const stylelintDisableOccurrences = [];

for await (const { path } of readdirp('.', {
directoryFilter: ['!.git', '!.next', '!node_modules'],
fileFilter: ['*.css', '*.scss', '*.sass', '*.less', '*.js', '*.tsx'],
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

})) {
const fileContents = await fs.readFile(path, 'utf-8');
if (
/stylelint-disable|stylelint [a-z0-9@/-]+: (0|off)/.test(fileContents)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the second part of this pattern a valid way to ignore code in Stylelint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

) {
stylelintDisableOccurrences.push(path);
}
}

if (stylelintDisableOccurrences.length > 0) {
throw new Error(
`Stylelint has been disabled in the following files:
${stylelintDisableOccurrences.join('\n')}

Remove all comments disabling or modifying Stylelint rule configuration (eg. stylelint-disable and stylelint-disable-next-line comments) and fix the problems.
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
`,
);
}
}
40 changes: 40 additions & 0 deletions src/checks/stylelint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { execaCommand } from 'execa';

export const title = 'Stylelint';

export default async function stylelintCheck() {
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
try {
await execaCommand(
'pnpm stylelint **/*.{css,scss,less,js,tsx} --max-warnings 0',
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • did we also want to support .jsx here?
  • probably good to make this into a variable (maybe array of strings?) and export from this file to use also in src/checks/stylelintConfigIsValid.ts - so that it doesn't get outdated / inconsistent between the two files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

);
} catch (error) {
const { stdout } = error as { stdout: string };
const lines = stdout.split('\n');

// If no Stylelint problems detected, throw the error
if (
!/^\d+ problems? \(\d+ errors?, \d+ warnings\)$/.test(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if there are only errors or only warnings? the ESLint check is intentionally more permissive of a check to avoid this problem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines[lines.length - 2]!,
)
) {
throw error;
}

throw new Error(
`Stylelint problems found in the following files:

${lines
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if you'll need normalizeNewline() and to use stdout for Windows here like in the ESLint check:

${normalizeNewline(stdout)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.filter((line) => {
return (
line !== '' &&
!line.match(/^\d+ problems? \(\d+ errors?, \d+ warnings\)|\d+:\d+/)
);
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I guess this can probably be simplified, similar to the ESLint check .filter()
  • let's add comments above each line, like in the ESLint check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

})
.map((line) => `${process.cwd()}/${line}`)
Copy link
Member

@karlhorky karlhorky Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update the PR description with a screenshot of the ESLint check (showing problems in files)?

I think this is inconsistent with the path format of the ESLint check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.join('\n')}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to .reduce() for duplicate filenames? (like we do in the ESLint check) try testing a few different possibilities - I guess probably multiple errors and warnings in a single file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Open these files in your editor - there should be problems to fix.
karlhorky marked this conversation as resolved.
Show resolved Hide resolved
`,
);
}
}
6 changes: 5 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import * as noSecretsCommittedToGit from './checks/noSecretsCommittedToGit.js';
import * as preflightIsLatestVersion from './checks/preflightIsLatestVersion.js';
import * as prettier from './checks/prettier.js';
import * as projectFolderNameMatchesCorrectFormat from './checks/projectFolderNameMatchesCorrectFormat.js';
import * as stylelintConfigIsValid from './checks/stylelingConfigIsValid.js';
import * as stylelint from './checks/stylelint.js';
import * as useSinglePackageManager from './checks/useSinglePackageManager.js';
import { CtxParam } from './types/CtxParam.js';
import { TaskParam } from './types/TaskParam.js';
Expand Down Expand Up @@ -67,10 +69,12 @@ const listrTasks = [

// Linting
eslint,
stylelint,
prettier,
Josehower marked this conversation as resolved.
Show resolved Hide resolved

// Version and configuration checks
eslintConfigIsValid,
stylelintConfigIsValid,
preflightIsLatestVersion,
Josehower marked this conversation as resolved.
Show resolved Hide resolved
].map((module) => {
if ('task' in module) return module;
Expand All @@ -87,6 +91,6 @@ await new Listr(listrTasks, {
removeEmptyLines: false,
formatOutput: 'wrap',
},
fallbackRenderer: 'verbose',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably a merge mistake

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fallbackRenderer: 'verbose',


concurrent: 5,
}).run();