Update 'JavaScript coding style' page #936
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romaricpascal
changed the title
romaricpascal
force-pushed
the
javascript-coding-style
branch
2 times, most recently
from
ea46a9a
to
8c6bdee
Compare
As the ecosystem for JavaScript linting has evolved, with ESLint offering the possibility to do extra linting than standard and Prettier providing opinionated formatting, the section tries to present the different alternatives, but keeps emphasising that we shouldn't spend our time picking rules. It also adds a little more details on when to run linting.
Add mention of editorconfig files to help ensure whitespace is consistent
Adds extra section for easier findability and updates the syntax of examples to ES6
It's no longer as trendy as it used to be, with the JavaScript ecosystem now focusing more on native JavaScript or TypeScript
We're advising against the use of jQuery so our examples should follow suite
romaricpascal
force-pushed
the
javascript-coding-style
branch
from
8c6bdee
to
4150b37
Compare
andysellick
reviewed
Oct 4, 2024
|
|
||
| #### Prettier | ||
|
|
||
| Prettier's only preoccupation is with only with [code formatting, not code quality][prettier-comparison]. |
There was a problem hiding this comment.
is with only with - should this be is with?
| #### Prettier | ||
|
|
||
| Prettier's only preoccupation is with only with [code formatting, not code quality][prettier-comparison]. | ||
| It can be used as a complement to ESLint for further automated formatting, with much more advanced decisions in terms of indentation, spaces, line breaks,... |
There was a problem hiding this comment.
This line ends with ,... is that meant to be there? If it's markdown apologies, not familiar with it.
|
|
||
| Prettier's only preoccupation is with only with [code formatting, not code quality][prettier-comparison]. | ||
| It can be used as a complement to ESLint for further automated formatting, with much more advanced decisions in terms of indentation, spaces, line breaks,... | ||
| It runs as a separate command (`npx prettier`) and the []`eslint-config-prettier`][prettier-linters] ensures there'll be no conflicts between the |
There was a problem hiding this comment.
Should
[]`eslint-config-prettier`][prettier-linters]
be
[][`eslint-config-prettier`][prettier-linters]
?
There was a problem hiding this comment.
I think it should be:
Suggested change
| It runs as a separate command (`npx prettier`) and the []`eslint-config-prettier`][prettier-linters] ensures there'll be no conflicts between the | |
| It runs as a separate command (`npx prettier`) and [`eslint-config-prettier`][prettier-linters] ensures there'll be no conflicts between the |
|
|
||
| Prettier's only preoccupation is with only with [code formatting, not code quality][prettier-comparison]. | ||
| It can be used as a complement to ESLint for further automated formatting, with much more advanced decisions in terms of indentation, spaces, line breaks,... | ||
| It runs as a separate command (`npx prettier`) and the []`eslint-config-prettier`][prettier-linters] ensures there'll be no conflicts between the |
There was a problem hiding this comment.
This sentence seems incomplete? ...ensures there'll be no conflicts between the
| #### On CI | ||
|
|
||
| Running standard in CI ensures that all pull requests meet our code conventions before getting merged on the `main` branch. | ||
| You should at least have this set up on your project. |
There was a problem hiding this comment.
Suggestion: I'd maybe reword this as You should have this configured as part of your project.
|
|
||
| Waiting for CI to know if the code follows the convention can take a bit of time. | ||
| A pre-commit Git hook allows to get quicker feedback, directly on developers' machines. | ||
| Errors that are automatically fixable can be fixed at that stage without human intervention, as well, |
There was a problem hiding this comment.
Suggestion: Errors that are automatically fixable can be fixed at that stage without human intervention, reducing the effort of linting for developers.
| reducing the effort of linting for developers. | ||
|
|
||
| Tools like [Husky][] and [lint-staged][] can help consistently run linting before commit by respectively: | ||
| - setting up the hooks when dependencies get installed |
There was a problem hiding this comment.
Question: will a list render correctly like this in markdown without a line break between it and the preceding paragraph? (I can't check the rendered version for some reason)
There was a problem hiding this comment.
it doesn't render as a list on my machine, so I think it does need the blank line
|
|
||
| The standard docs have a [complete list of rules and some reasoning behind them](https://standardjs.com/rules.html). | ||
| To get even quicker feedback, editor plugins can highlight issues while editing files. | ||
| They also allow to fix automatically fixable errors on save, saving further fixing effort. |
There was a problem hiding this comment.
Suggestion: They can correct automatically fixable errors on save, saving further development effort.
|
|
||
| Standard is also [widely used (warning: large file)](https://github.com/feross/standard-packages/blob/master/all.json) (which means community familiarity) and has a [good ecosystem of plugins](https://standardjs.com/awesome.html). | ||
| Each of the tools previously listed has plugin to help integrate with editors: |
There was a problem hiding this comment.
Should this be plugins?
| var name = 'thing' | ||
| function query () { ... } | ||
| ``` | ||
| **Why:** This lets future developers know how to interact with objects and sets the appropriate affordances. It also follows the conventions of the standard library. |
There was a problem hiding this comment.
Nitpick: is there an alternative word we could use to affordances? I had to look up what it means, maybe we could use something more commonly used?
DavidBiddle
reviewed
Oct 4, 2024
| ```bash | ||
| #### StandardJS's command line interface | ||
|
|
||
| If you're not looking to use make any amends to the StandardJS conventions, you can use [StandardJS' `standard` command line interface][standard-cli] to lint files in your repository without extra set up. |
There was a problem hiding this comment.
I think the word 'use' might be left over from a previous edit:
Suggested change
| If you're not looking to use make any amends to the StandardJS conventions, you can use [StandardJS' `standard` command line interface][standard-cli] to lint files in your repository without extra set up. | |
| If you're not looking to make any amends to the StandardJS conventions, you can use [StandardJS' `standard` command line interface][standard-cli] to lint files in your repository without extra set up. |
DavidBiddle
reviewed
Oct 4, 2024
| ### When to use standardx | ||
|
|
||
| You should use [standardx][] when the standard's rule set conflicts with the browsers your project supports. For example, the [no-var](https://eslint.org/docs/rules/no-var) rule in standard - which prefers `let` or `const` over `var` - prevents JavaScript usage in versions of Internet Explorer < 11. Adopting this rule would mean explicitly breaking JavaScript for those browsers. | ||
| If the StandardJS's rule set conflicts with the browsers your project supports, you can use [standardx][] to amend which rules are running. |
There was a problem hiding this comment.
I think this should be either the StandardJS rule set or StandardJS's rule set:
Suggested change
| If the StandardJS's rule set conflicts with the browsers your project supports, you can use [standardx][] to amend which rules are running. | |
| If the StandardJS rule set conflicts with the browsers your project supports, you can use [standardx][] to amend which rules are running. |
DavidBiddle
reviewed
Oct 4, 2024
|
|
||
| [eslintrc]: https://eslint.org/docs/user-guide/configuring | ||
| [govuk-warnings]: https://github.com/alphagov/govuk_publishing_components/commit/ea7f0becc76f73780b6cb33701bea9e58f15f91a | ||
| ESLint is the most widely use JavaScript linter, and actually what StandardJS uses under the hood. |
There was a problem hiding this comment.
Small typo:
Suggested change
| ESLint is the most widely use JavaScript linter, and actually what StandardJS uses under the hood. | |
| ESLint is the most widely used JavaScript linter, and actually what StandardJS uses under the hood. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the 'JavaScript coding style' page upper sections based on feedback from Frontend Community (either live or through this document (internal link)). Each commit updates a separate section for easier review.
The biggest update is the 'Linting' section, which now tries to paint a clear picture of the available options between 'standard', 'ESLint' and 'Prettier', as well as where to run the linters.
The sections about 'Modules' and 'Strict mode' could use a big overhaul as well, which I'll do in a separate PR to focus the discussion:
exporting features andimporting other modules) from the classes that provide functionalities and are called modules as well so far