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

[Story] Check ESLint consistency between IDE and CLI output #2789

Closed
1 of 2 tasks
tschaffter opened this issue Sep 3, 2024 · 6 comments · Fixed by #2793
Closed
1 of 2 tasks

[Story] Check ESLint consistency between IDE and CLI output #2789

tschaffter opened this issue Sep 3, 2024 · 6 comments · Fixed by #2793
Assignees
Labels
formatting Source formatter issues linting typescript

Comments

@tschaffter
Copy link
Member

tschaffter commented Sep 3, 2024

What product(s) is this story for?

Sage Monorepo

As a user, I want

No response

Description

No response

Acceptance criteria

No response

Tasks

  • Evaluate how much time nx format:check takes after identifying files that must be ignored by Prettier

Anything else?

No response

Have you linked this story to a GitHub Project?

  • I have linked this story to a GitHub Project and set its metadata.
@tschaffter tschaffter added typescript formatting Source formatter issues linting labels Sep 3, 2024
@tschaffter tschaffter self-assigned this Sep 3, 2024
@tschaffter
Copy link
Member Author

tschaffter commented Sep 3, 2024

ESLint extension for VS Code

It's active by default but it's not specifically defined in VS Code config file, which we should do.

Here is information from the extension:

[Info  - 9:12:58 PM] ESLint server is starting.
[Info  - 9:13:00 PM] ESLint server running in node v20.14.0
[Info  - 9:13:00 PM] ESLint server is running.
[Info  - 9:13:01 PM] ESLint library loaded from: /workspaces/sage-monorepo/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/api.js

Observation:

  • The VS Code plugin is using a different version of Node.js (v20.14.0) than the Node.js version installed in the dev container (v20.17.0)
  • The VS Code plugin runs [email protected] while the workspace installs v8.57.0.

EDIT: I downgraded eslint in the workspace and rebuilt the dev container. I can confirm that the ESLint extension for VS Code is using the eslint binary installed in the workspace.

ESLint extension for VS Code output when opening a TS file:

[Info  - 3:52:10 PM] ESLint server is starting.
[Info  - 3:52:11 PM] ESLint server running in node v20.14.0
[Info  - 3:52:11 PM] ESLint server is running.
[Info  - 3:52:12 PM] ESLint library loaded from: /workspaces/sage-monorepo/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/api.js

This confirms what the ESLint docs says:

The extension uses the ESLint library installed in the opened workspace folder. If the folder doesn't provide one the extension looks for a global install version. If you haven't installed ESLint either locally or globally do so by running npm install eslint in the workspace folder for a local install or npm install -g eslint for a global install.

Remaining questions:

  • How does the ESLint plugin use a different version of Node.js than the one installed in the dev container?
  • How can I enforce VS Code to use the ESLint plugin instead of relying on it being used as the default?

node version does not match that in terminal

According to this comment:

by default the extension uses the node runtime shipped with VS Code

The same post should provide the solution:

The way how to use a different node runtime is the eslint.runtime property. Usually using node as a value should pick the default version even if you are using a node version manager.

UPDATE: It's not clear whether setting the runtime or nodePath is applied. The plugin shows this error message if the path to node is incorrect. But the output below also shows that the version of node printed is still the one shipped with VS Code.

When setting eslint.nodePath to foo:

[Info  - 4:40:02 PM] ESLint server is starting.
[Info  - 4:40:03 PM] ESLint server running in node v20.14.0
[Info  - 4:40:03 PM] ESLint server is running.
Uncaught exception received.
Error: spawn /vscode/vscode-server/bin/linux-x64/fee1edb8d6d72a0ddff41e5f71a671c23ed924b9/node ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
[Info  - 4:40:04 PM] ESLint library loaded from: /workspaces/sage-monorepo/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/api.js

For now I will let the ESLint server use the node version shipped with VS Code to avoid increasing the complexity of the workspace without confirmed benefits.

@tschaffter
Copy link
Member Author

ESLint formatter feature

The extension can be used as a formatter, but it's not enable by default. For example, when opening a TypeScript file:

image

To enable the formatter feature:

"eslint.format.enable": true,

VS Code now shows:

image

@tschaffter
Copy link
Member Author

tschaffter commented Sep 4, 2024

Format on Save

Currently, ESLint is formatting TS files on save, though this is something I would like to configure specifically.

The formatter feature of the ESLint extension controlled by eslint.format.enable does not affect formatting on save.

ESLint does not respect this config (it will still format on save):

  "[typescript]": {
    "editor.defaultFormatter": "dbaeumer.vscode-eslint",
    "editor.formatOnSave": false
  },

This section describes how to enable and disable auto fix: https://github.com/microsoft/vscode-eslint?tab=readme-ov-file#version-204

This comment from the author of the extension suggest an ESLint package to use to format JSON files: microsoft/vscode-eslint#1173 (comment)

@tschaffter
Copy link
Member Author

tschaffter commented Sep 4, 2024

Format JSON files with ESLint-Prettier

This ESLint configuration work to format the files test.json and test/test.json.

.eslintrc.json:

{
  "root": true,
  "overrides": [
    {
      "files": ["*.json", "*.jsonc", "*.json5", ".vscode/test.json"],
      "parser": "jsonc-eslint-parser",
      "extends": ["plugin:prettier/recommended"],
      "rules": {}
    }
  ]
}

Example:

image

Saving the file will format it thanks to the VS Code configuration we have previously identified.

Issue

The file .vscode/test.json, which has the same content as the previous file, is not formatted for an unknown reason.

image

EDIT: Renaming test/test.json to .test/test.json prevent ESLint-Prettier from validating the file in VS Code. This observation has been previously reported in prettier/prettier-vscode#818.

Solution: https://stackoverflow.com/a/59651542 (it works)

@tschaffter
Copy link
Member Author

nx format:check and nx format:write

This script formats all the files in the monorepo using Prettier.

Source: https://github.com/nrwl/nx/blob/master/packages/nx/src/command-line/format/format.ts

Example:

$ time nx format:check --all
.devcontainer/devcontainer.json
.eslintrc.json
.github/PULL_REQUEST_TEMPLATE/default.md
.github/renovate.json
.github/workflows/build-docs.yml
.github/workflows/ci.yml
.github/workflows/lint-dockerfiles.yml
.github/workflows/lint-pr.yml
.github/workflows/schematic-api-ci.yml
.github/workflows/sonar-scan-pull-request.yml
.github/workflows/sonar-scan-push.yml
.github/workflows/update-oc-db-csv-files.yml
.prettierrc
.vscode/extensions.json
apps/agora/apex/project.json
apps/agora/api-docs/project.json
apps/agora/api/src/api.ts
apps/agora/api/src/components/dataversion.ts
apps/agora/api/src/models/dataversion.ts
apps/agora/api/webpack.config.js
apps/agora/app/src/app/app.config.ts
apps/agora/app/src/app/app.routes.ts
apps/agora/app/src/main.ts
apps/agora/data/.venv/lib/python3.11/site-packages/agora_data-0.1.0.dist-info/direct_url.json
apps/agora/data/.venv/lib/python3.11/site-packages/httpcore-1.0.5.dist-info/licenses/LICENSE.md
apps/agora/data/.venv/lib/python3.11/site-packages/httpx-0.27.0.dist-info/licenses/LICENSE.md
apps/agora/data/.venv/lib/python3.11/site-packages/opentelemetry/sdk/metrics/_internal/exponential_histogram/mapping/ieee_754.md
apps/agora/data/README.md
apps/iatlas/api/README.md
apps/iatlas/data/.venv/lib/python3.10/site-packages/decorator-5.1.1.dist-info/pbr.json
apps/iatlas/data/.venv/lib/python3.10/site-packages/numpy/random/LICENSE.md
apps/iatlas/data/.venv/lib/python3.10/site-packages/opentelemetry/sdk/metrics/_internal/exponential_histogram/mapping/ieee_754.md
apps/iatlas/data/README.md
apps/model-ad/apex/project.json
apps/model-ad/api-docs/project.json
apps/model-ad/api/project.json
apps/model-ad/api/src/main/resources/openapi.yaml
apps/model-ad/api/templates/config.yaml
apps/model-ad/app/server.ts
apps/model-ad/app/src/app/app.config.ts
apps/model-ad/app/src/app/app.routes.ts
apps/model-ad/app/src/index.html
apps/model-ad/app/src/main.ts
apps/model-ad/mongo/project.json
apps/openchallenges/apex/README.md
apps/openchallenges/api-docs/README.md
apps/openchallenges/app/e2e/home.spec.ts
apps/openchallenges/app/server.ts
apps/openchallenges/app/src/_app-theme.scss
apps/openchallenges/app/src/app/app.component.ts
apps/openchallenges/app/src/app/app.config.ts
apps/openchallenges/app/src/app/app.routes.ts
apps/openchallenges/app/src/app/google-tag-manager/google-tag-manager-id.provider.ts
apps/openchallenges/app/src/app/initialize-keycloak.factory.ts
apps/openchallenges/app/src/assets/silent-check-sso.html
apps/openchallenges/app/src/index.html
apps/openchallenges/app/src/main.ts
apps/openchallenges/auth-service/docker-compose.yml
apps/openchallenges/challenge-service/src/main/resources/db/README.md
apps/openchallenges/challenge-service/src/main/resources/openapi.yaml
apps/openchallenges/challenge-service/templates/config.yaml
apps/openchallenges/challenge-to-elasticsearch-service/docker-compose.yml
apps/openchallenges/config-server-repository/README.md
apps/openchallenges/config-server/README.md
apps/openchallenges/core-service/docker-compose.yml

real    1m1.142s
user    0m2.377s
sys     0m0.998s

Observations:

  • It takes about 1 minutes to format all the files given the current content of the monorepo.
  • There are files that we may not want to format, e.g. the pnpm lock file (see below).
    • Lock files are generally large and may take time to format for no benefit.
    • There are "machine" files like lock files that humans don't read where readability is not needed.
    • Formatted/prettyfied files may make the file larger (see example below)

image

Formatted pnpm lock file is larger:

# Before formatting
vscode@1fb8313797af:/workspaces/sage-monorepo$ ls -al pnpm-lock.yaml 
-rw-r--r--. 1 vscode vscode 1229364 Sep  5 00:05 pnpm-lock.yaml

# After formatting
vscode@1fb8313797af:/workspaces/sage-monorepo$ ls -al pnpm-lock.yaml 
-rw-r--r--. 1 vscode vscode 1158059 Sep  5 14:51 pnpm-lock.yaml

@tschaffter
Copy link
Member Author

Formatting Vs Linting

Prettier is a formatter. ESLint is a linter.

According to this thread, ESLint tried to do both formatting and linting, but is now focusing on linting.

The above thread also mentions concerns about using ESLint to "auto fix" issues, in particular auto fixing some linting issues may be sub-optimal.

Prettier vs. ESLint: A Comparison

Based on the above Reddit thread:

Feature Prettier ESLint
Primary Function Code formatting Code linting and formatting
Opinionated Highly opinionated (enforces a specific style) Less opinionated (allows customization)
Configurability Limited configuration options Extensive configuration options
Performance Generally faster Can be slower, especially with complex rulesets
Ease of Use Simple to set up and use Requires more configuration and setup
Focus Formatting (spacing, quotes, etc.) Linting (style guidelines, potential errors)
Compatibility Compatible with most editors and linters May conflict with certain formatting rules
Use Case Ensuring consistent code formatting Enforcing coding standards and detecting potential issues

In summary: Prettier is excellent for enforcing a consistent code style quickly and easily, while ESLint offers more flexibility and control for linting and style guidelines. Many developers use both tools together to achieve a balance between consistency and customization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment