nextcloud / standards

1 stars 0 forks source link

Frontend code style #11

Open susnux opened 1 month ago

susnux commented 1 month ago

Prologue

For backend development we are using the PHP CS fixer with our code style for PHP.

Problem

For frontend code we have no established code style. Currently some apps and some libraries are using our eslint-config which currently (as of 2024) also includes some stylistic rules from the standardjs project. Also our stylelint configuration includes some stylistic rules.

As we mostly use Vue we tried to follow some of the Vue styleguide rules, but we do not enforce them all.

But in general both ESLint and Stylelint decided or followed the "mindset" to separate linters (for code quality and issues) and formatters (for code style). This means e.g. with current Stylelint there are no stylelistic rules anymore and ESLint with v9 also deprecated and removed a lot of stylistic rules.

Proposal

  1. Adjust our rules to match closer with the Vue styleguide, for the reasons mentioned on the rules.
  2. We should join ESLint and Stylelint and use them only for linting, meaning for rules that prevent issues and bugs. For stylistic rules, speaking so the code style, we should use a formatter.

I propose to use the very well known Prettier. From my point of view it creates a good code style especially it creates code that is easier to review (diff). It does not let much room for discussion about what to configure, this is a benefit and a negative point however you want to see it.

An example configuration can be found here: Prettier config It is built with current (v8) ESLint rules in mind to reduce the diff when applying the new config.

Alternative

There are also ways to bring back stylistic rules to ESLint and stylelint.

susnux commented 1 month ago

I personally like Prettier for its output and think it helps with having a common code style making reviews a lot easier. An example of how this could look like can be found here: Example on forms

Either-way I personally would like to see more rules of the ESLint vue plugin enforced. And especially first-attribute-linebreak is strongly recommended but we do the opposite of the recommended: Instead of having consistent styling we mix beside and below for multi-line attributes.

pulsejet commented 1 month ago

I'd second Prettier for Vue; it's pretty convenient to have an opinionated tool that you can't argue with. In my experience the only annoying thing is calls to l10n in Vue templates with long text. As an example.

For PHP, on the other hand, I found that well defined CS Fixer rules work much better (it understands semantics, not just syntax).

ShGKme commented 1 month ago
  1. Adjust our rules to match closer with the Vue styleguide, for the reasons mentioned on the rules.

Agree. In general, the only 2 rules we ignore are:

But this could be quite painful, requiring renaming a lot of files, not just changing the code.

ShGKme commented 1 month ago
  1. We should join ESLint and Stylelint and use them only for linting, meaning for rules that prevent issues and bugs. For stylistic rules, speaking so the code style, we should use a formatter.

In general, it is quite arguable in Frontend ecosystem. Though ESLint decided to get rid of formatting, a lot of plugins has no alternative for formatting outside ESLint.

ShGKme commented 1 month ago

it creates code that is easier to review (diff).

I'd say, it's the opposite. If we have multiline array or import, or function args, then Prettier will automatically make it one-line once they are short, which is very not diff-safe.

Example:

Having

import {
  aaaaaa,
  bbbbbb,
  cccccc,
  dddddd,
  eeeeee,
  ffffff,
} from 'some-module-name'

removing one import results in

+ import { aaaaaa, bbbbbb, cccccc, dddddd, eeeeee } from 'some-module-name'
- import {
-   aaaaaa,
-   bbbbbb,
-   cccccc,
-   dddddd,
-   eeeeee,
-   ffffff,
- } from 'some-module-name'

instead of

import {
  aaaaaa,
  bbbbbb,
  cccccc,
  dddddd,
  eeeeee,
-   ffffff,
} from 'some-module-name'

This is not easier to review, not diff-safe at all...

ShGKme commented 1 month ago

Speaking about Prettier in general, I would vote for Prettier. Though it has some unpleasant results in multiline/single-line formatting sometimes (especially in Vue), I use prettier in personal my projects and I like the idea of having fixed formatting automatically with less freedom for developers to format it bad (Prettier's prerogative :D).

But on the other hand, migrating to Prettier would be quite painful in meaning of backports. Especially on server we even don't use actually ESLint 🥲

susnux commented 1 month ago

I'd say, it's the opposite. If we have multiline array or import, or function args, then Prettier will automatically make it one-line once they are short, which is very not diff-safe.

The only part of Prettier I also do not like and where I understand that people created a plug-in to fix that 😅