eslint-plugin-criteo

This ES lint plugin defines custom rules we use at Criteo and exposes a recommended set of rules.

Installation

npm install eslint eslint-plugin-criteo --save-dev

Like any library, keep it updated to make sure your project follows the latest coding recommendations.

Usage

Add criteo to the plugins section of your .eslintrc configuration file and apply:

• plugin:criteo/recommended-angular-app if the project is an Angular application (formerly, now deprecated, recommended-app)
• plugin:criteo/recommended-angular-lib if the project is an Angular library (formerly, now deprecated, recommended-lib)
• plugin:criteo/recommended-react-app if the project is a React application
• plugin:criteo/recommended-react-lib if the project is a React library
• plugin:criteo/recommended for general-purpose rules which are included in all the above recommended configs
• plugin:criteo/recommended-angular-template for Angular HTML templates (formerly, now deprecated, recommended-template)

{
  "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"
  }
}

Pre-commit Git hook

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!

1. Define the pre-commit hook script .git-hook-lint-staged at 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

2. Also define the postinstall NPM script in the scripts section of package.json:

"scripts": {
  "postinstall": "npx shx cp .git-hook-lint-staged ../.git/hooks/lint-staged"
}

It will enable the hook automatically after npm install.

3. Still in the same project, install shx to make the copy command work on all environments: npm install --save-dev shx

4. Install lint-staged in each UI project of the repository: npm install --save-dev lint-staged

5. 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.

🙌

Criteo rules

cypress-no-force

Cypress: disallow using of 'force: true' option.

Why?

The Cypress ESLint plugin provides this rule, with the following explanation:

Using force: true on 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.

filename

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,...}.

filename-match-export

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.

independent-folders

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 all featureFolders and sharedFolders (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.

ngx-component-display

Ensure Angular components have a display property set.

Config:

• ignore: classes whose names match this regular expression (defined as string) will be ignored (default: '^.*DialogComponent$')
• propertyName: name of the display property (default: 'cdsDisplay')

Why?

• By default, custom components are displayed as inline by 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.

ngx-no-styles-in-component

Forbid using styles 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.

no-ngxs-select-decorator

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.

no-null-undefined-comparison

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.

ngxs-selector-array-length

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.

no-spreading-accumulators

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; }, {});

no-todo-without-ticket

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.

prefer-readonly-decorators

Ensure that appropriate decorated properties are readonly.

Config:

• decorators: array of decorator names that should always be readonly (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.

until-destroy

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.

External rules

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.