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

Frontend code style #11

Open
susnux opened this issue May 21, 2024 · 10 comments
Open

Frontend code style #11

susnux opened this issue May 21, 2024 · 10 comments
Labels
discussion Being discussed

Comments

@susnux
Copy link

susnux commented May 21, 2024

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 susnux added the discussion Being discussed label May 21, 2024
@susnux susnux changed the title JavaScript and Typescript code style Frontend code style May 21, 2024
@susnux
Copy link
Author

susnux commented May 21, 2024

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
Copy link
Member

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
Copy link

ShGKme commented May 22, 2024

  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
Copy link

ShGKme commented May 22, 2024

  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
Copy link

ShGKme commented May 22, 2024

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
Copy link

ShGKme commented May 22, 2024

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
Copy link
Author

susnux commented May 22, 2024

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 😅

@max-nextcloud
Copy link

We briefly discussed this in the office team team call today. No opposing voices yet.

There's also a PR open for text to move to prettier:

@max-nextcloud
Copy link

I gave oxlint a try and it's way faster than eslint. (500ms instead of 20sec. for text)

I like the idea of splitting the linting and the code formatting and having a super fast linter.

@susnux
Copy link
Author

susnux commented Oct 16, 2024

I gave oxlint a try and it's way faster than eslint. (500ms instead of 20sec. for text)

This would in the future also match our tooling with vite as they will, sometime in the future, migrate from Rollup to Rolldown, and apparently oxc is used by Rolldown. But currently this is too unstable to be used by us as they do not even provide all useful rules (vue and especially Typescript) we use.


But I would keep this here to the question:

  • Should we split linting and formatting?
  • If yes should we use Prettier as the formatting tool?

As said we could also stick with ESLint, with the benefit of being able to configure everything like we want. But with the downside of being able to configure everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Being discussed
Projects
None yet
Development

No branches or pull requests

4 participants