Skip to content

Add jest & mocha eslint plugins #207

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.

Sign up for GitHub

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

Draft
francoisferrand wants to merge 2 commits into
base: development/8.3
Choose a base branch
from
Draft

Add jest & mocha eslint plugins #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdfa2d3
Add jest & mocha eslint plugins
francoisferrand Jan 31, 2025
f3725b6
Fix eslint to properly detect brace-less one-line if
francoisferrand Nov 18, 2021
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
17 changes: 15 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
const globals = require('globals');

// Auto-detect plugin name, to avoid duplicate errors
const testPlugin = (() => {
try {
require.resolve('jest');
return 'jest';
} catch {
return 'mocha';
}
})();

module.exports = {
globals: {
...Object.fromEntries(Object.entries(globals.browser).map(([key]) => [key, 'off'])),
...globals.node,
...globals.mocha,
},
plugins: ['jest', 'mocha'],
extends: [
`plugin:${testPlugin}/recommended`,
Copy link
Copy Markdown
Contributor Author

@francoisferrand francoisferrand Feb 21, 2025

Choose a reason for hiding this comment

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

Maybe too strong, causes lots of errors...
→ just "mocha/no-exclusive-tests" and "jest/no-focused-tests" ?
→ or extend recomended plugins, then disable a few (no arrow...)?

Copy link
Copy Markdown

@ghost ghost Apr 3, 2025

Choose a reason for hiding this comment

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

Maybe we can review on some repos what errors this produces and agree on the rules we want to disable? Hopefully not too many, as relaxed rules tend to cause heterogeneous code styles across our projects...

Some are quite obvious to disable, the others, if their number is ok, should be kept. We might as well generalize prettier, so it automates most of the work...

],
rules: {
'constructor-super': 'error',
'for-direction': 'error',
Expand Down Expand Up @@ -87,7 +100,7 @@ module.exports = {
2,
'always'
],
'curly': 1,
'curly': [1, 'all'],
'no-mixed-spaces-and-tabs': 2,
'max-len': [
2,
Expand Down
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
},
"dependencies": {
"commander": "11.1.0",
"markdownlint": "0.31.1"
"markdownlint": "0.31.1",
"eslint-plugin-jest": "^28.11.0",
"eslint-plugin-mocha": "^10.5.0"
Comment on lines +21 to +22
Copy link
Copy Markdown
Contributor Author

@francoisferrand francoisferrand Feb 21, 2025

Choose a reason for hiding this comment

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

need to be added to dependencies (and/or peerDependencies?) for the plugins to be automatically added to the "top-level" node_modules, and thus available when running eslint...

Copy link
Copy Markdown

@BourgoisMickael BourgoisMickael Feb 28, 2025

Choose a reason for hiding this comment

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

as we don't need both, they should go in peerDependencies and eslint should go there too

Copy link
Copy Markdown
Contributor Author

@francoisferrand francoisferrand Mar 3, 2025

Choose a reason for hiding this comment

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

if we don't add it to dependencies, these plugins will not be included when depending on this plugin (in the "user" package, like cloudserver/...), as peer dependencies do not get installed (just display a warning) : and thus will have more dependencies to include in every package...

The peerDependencies is not really needed here, what I really want is a dependency : and peerDependency may be added simply to avoid having duplicating versions of the dependency...

Copy link
Copy Markdown

@BourgoisMickael BourgoisMickael Mar 3, 2025

Choose a reason for hiding this comment

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

But that's the point that we don't want to automatically add it by default, it should be the source repo that picks if it wants to install the plugin jest or mocha based on what it needs to avoid unnecessary dependency.

But maybe they are small enough to ignore an extra unused dependency.

Copy link
Copy Markdown
Contributor Author

@francoisferrand francoisferrand Mar 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather it "just works", and the dependencies are added automatically ; but indeed they may pull quite a few deps. Maybe we actually already have everything, so not much difference; but I am not sure... I will double check how much gets pull.

The only "safeguard" of sorts is that this is this whole package is a dev dependency, so will not have any effect on prod.

That said, if we do not have the dependencies, the user (dev) experience will not be very good : unless the appropriate plugin is explicitely specified, then eslint will most likely fail to run with a cryptic message (or worse, the extra check will silently not be performed)

},
"devDependencies": {
"eslint": "^9.9.1"
Expand Down
Loading