This ES lint plugin defines custom rules we use at Criteo and exposes a recommended set of rules.
npm install eslint eslint-plugin-criteo --save-devLike any library, keep it updated to make sure your project follows the latest coding recommendations.
Add criteo to the plugins section of your .eslintrc configuration file and apply:
-
plugin:criteo/recommended-angular-appif the project is an Angular application -
plugin:criteo/recommended-angular-libif the project is an Angular library -
plugin:criteo/recommended-react-appif the project is a React application -
plugin:criteo/recommended-react-libif the project is a React library -
plugin:criteo/recommendedfor general-purpose rules which are included in all the above recommended configs -
plugin:criteo/recommended-angular-templatefor Angular HTML templates
{
"plugins": ["criteo"],
"overrides": [
{
"files": ["*.ts"],
"extends": ["plugin:criteo/recommended-angular-app"], // or recommended-react-app, recommended-angular-lib...
[...]
},
{
"files": ["*.html"],
"extends": ["plugin:criteo/recommended-angular-template"],
[...]
}
]
}Then configure/disable the rules under the rules section, following your project's context and constraints.
{
"rules": {
"criteo/rule-1": ["error", { "custom-config-key": "custom-config-value" }],
"criteo/rule-2": "off"
}
}As a developer, it can be very frustrating to get a change rejected by QA bot because the code does not respect an ES lint rule. Even more if the failing rule has an auto-fix! To avoid it, you can configure your project to run ES lint automatically on pre-commit. Because it will target only staged files, it is quite fast!
- Define the pre-commit hook script
.git-hook-lint-stagedat the root of the first/main UI project of the repository. It will make it available to all developers since Git hooks cannot be pushed directly.
#!/bin/sh
cd project1 && npx lint-staged \
&& cd ../project2 && npx lint-staged # If the repository hosts several projects, add each of them- Also define the
postinstallNPM script in thescriptssection ofpackage.json:
"scripts": {
"postinstall": "npx shx cp .git-hook-lint-staged ../.git/hooks/lint-staged"
}
It will enable the hook automatically after npm install.
-
Still in the same project, install
shxto make the copy command work on all environments:npm install --save-dev shx -
Install
lint-stagedin each UI project of the repository:npm install --save-dev lint-staged -
In each project, also define its configuration by declaring in
package.json:
"lint-staged": {
"**.{ts,component.html}": "eslint --fix"
}
You can also declare "*.{ts,component.html,json,scss}": "prettier --write" if you use Prettier.
🙌
Cypress: disallow using of 'force: true' option.
Why?
The Cypress ESLint plugin provides this rule, with the following explanation:
Using
force: trueon inputs appears to be confusing rather than helpful. It usually silences the actual problem instead of providing a way to overcome it. See Cypress Core Concepts.
Unfortunately, the Cypress plugin's version only works when calling the function like cy.get(...).click({ force: true }),
i.e. with cy in the same line. But in our code we often use helper functions, so the rule does not detect our usages.
This new version of the rule leverages TypeScript to work out if the method is called on a Cypress.Chainable object.
Ensure file names are valid and consistent.
Config:
-
pattern: allowed name pattern (default:/^[a-z0-9\.\-]+$/)
Why?
Improve the file tree readability. For example, a component called MyButtonComponent should be defined under the path */my-button/my-button.component.{html,ts,css,scss,...}.
Ensure file names reflect the named exported members.
Config:
-
removeFromFilename: text removed from file names before check (default:[])
Why?
The name of the file should describe its content for readability.
Ensure feature folders are independent by preventing imports between each other. Features folders imports are also forbidden from the shared modules.
Config:
-
basePath: base path (from the project) applied to allfeatureFoldersandsharedFolders(default:./) -
featureFolders: array of the feature folders paths that should remain independent (default:[]) -
sharedFolders: array of the shared folders paths that cannot use any feature folder (default:[])
Sample :
'criteo/independent-folders': [
'error',
{
basePath: './src/app/',
featureFolders: ['broad-targeting-audience', 'similar-audience'],
sharedFolders: ['shared'],
},
],
Why?
To avoid messy and circular imports:
- Feature folders should be independent or use each others using the public API.
- Shared folders should not rely on feature folders.
Ensure Angular components have a display property set.
Config:
-
ignore: classes whose names match this regular expression (defined as string) will be ignored (default:'^.*(?:Dialog|Modal)Component$') -
propertyName: name of the display property (default:'cdsDisplay')
Why?
- By default, custom components are displayed as
inlineby the browser which is rarely what we expect. For example, it makes impossible to define a width or margins on them. E.g.<my-component class="w-100 cds-mb-3"></my-component>would have no effect. - The workaround of wrapping them in
<div></div>should be avoided to not make the DOM and the bundle file heavier.
Forbid using styles, styleUrl or styleUrls in a component's metadata: favour the Criteo design system instead.
Why?
To maintain a coherent user experience, we aim to use the classes and components from the shared component library as much as possible, as opposed to custom styles.
Forbid accessing an enum thanks to an index: Enum[0].
Why?
The names of the enum values should not be "data" in the context of the application at runtime. This brings a lack of clarity, a lack of readability, maintenance issues, and can be error-prone.
Forbid using the NGXS @Select() decorator: @ViewSelectSnapshot() should be preferred.
Why?
@Select() exposes an Observable that must be subscribed using the async pipe in the template. It makes it verbose and creates one subscription per usage. However, the @ViewSelectSnapshot() exposes the raw value directly and refreshes the view on every change ; making it more concise and easier to use.
Forbid comparisons with null and undefined.
Why?
The difference between null and undefined is specific to Javascript and can be tricky for juniors/backend developers. Most of the time, we don't need to distinguish these 2 values, so using isNil from lodash is safer.
Ensure that when using the @Selector() decorator, the number of selectors passed in matches the number of arguments passed to the selector function.
Why?
It can be tricky to pin down the source of an error when using the @Selector() decorator. While this rule can't make sure you put all the parameters in the right order, it does avoid the most obvious mistakes.
Ensure that reducers do not mistakenly have O(n^2) complexity.
Why?
When using .reduce(), it may be tempting to do something like this for the sake of brevity:
const mappedById = myArray.reduce((acc, entity) => ({ ...acc, [entity.id]: entity }), {});
However, spreading the accumulator at every iteration results in an operation with O(n^2) time & spatial complexity.
This rule helps ensure that .reduce() is an O(n) operation. For example:
const mappedById = myArray.reduce((acc, entity) => { acc[entity.id] = entity; return acc; }, {});
Ensure that comments with TODO or FIXME specify a JIRA ticket in which the work will be completed.
Why?
Commits with TODO comments indicating that a portion of functionality has yet to be implemented are easy to overlook later on. This rule encourages all outstanding work to be tracked by an external ticket as well as in code comments.
Ensure that appropriate decorated properties are readonly.
Config:
-
decorators: array of decorator names that should always bereadonly(default:['Output', 'ViewSelectSnapshot'])
Why?
Some decorated properties should be readonly because their decorator handles the value assignment and should never be assigned by hand (ex: @ViewSelectSnapshot()), or their value should never change (ex: @Output()) to prevent errors.
Ensure @UntilDestroy() decorator is not forgotten, nor applied when it is not necessary.
Why?
@UntilDestroy() defines mandatory properties in decorated class to make the untilDestroyed operator work.
Nevertheless, the decorator should not be applied when it is not necessary because it would inject useless code.
In addition to the rules defined above, we have chosen some rules from external libraries which we activate by default. Some of these have custom config to better address our specific use cases.