All: Document coding style #6657
Conversation
Moves style guidelines from https://discourse.joplinapp.org/t/code-style-things-that-arent-being-caught-by-eslint/26414
readme/coding_style.md
Outdated
| ## Declare variables just before their usage | ||
|
|
||
| > What about numeric constants? E.g. | ||
| > ```ts | ||
| > const gravityAcceleration = 9.8; // m/s | ||
| > ``` |
There was a problem hiding this comment.
I've gotten into the habit of declaring constant literals at the top of files (when it makes sense). I would like clarification here.
There was a problem hiding this comment.
If they are used multiple times in the file, it's ok to declare them at the top (or even in a different file, and import that).
If it's used only once, it should be declared only before it's used.
readme/coding_style.md
Outdated
|
|
||
|
|
||
| ## Prefer `() => {}` to `function() { ... }` | ||
| Prefer arrow functions to `function() { ... }` in new code. |
There was a problem hiding this comment.
Could you add this explanation please:
It avoids having to deal with the
thiskeyword. Not having it make it easier to refactor class components into React Hooks, because any use ofthis(used in classes) will be correctly detected as invalid by TypeScript.
readme/coding_style.md
Outdated
| > **VSCode**: In `vscode`, be sure to check whether new files are created with `tab` or `space` indentation! [Spaces can be converted to tabs using the command palette.](https://code.visualstudio.com/docs/editor/codebasics#_autodetection) | ||
|
|
||
|
|
||
| ## Avoid `==` |
There was a problem hiding this comment.
This section could be called Use strict equality.
With that explanation maybe:
Using strict equality ensures that the correct types are used throughout the code base.
There was a problem hiding this comment.
Is there an exception for comparing to null (to allow undefined, but to not allow 0)?
(If a variable can be undefined, null, or have a value, is it best to use if (variable) { ... }, if (variable != null) { ... }, or if (variable !== null && variable !== undefined) { ... }? The solution might be to avoid such cases if possible.)
readme/coding_style.md
Outdated
|
|
||
| ## Use TypeScript for new files | ||
|
|
||
| > Even if you are **modifying** a file that was originally in JavaScript you should ideally convert it first to TypeScript before modifying it. |
There was a problem hiding this comment.
If this is a large file however please ask first if it needs to be converted. Some very old and large JS files are tricky to convert properly due to poorly defined types, so in some cases it's better to leave that for another day (or another PR).
readme/coding_style.md
Outdated
| @@ -0,0 +1,170 @@ | |||
| > Coding style is mostly enforced by a pre-commit hook that runs `eslint`. This hook is installed whenever running `yarn install` on any of the application directory. If for some reason the pre-commit hook didn't get installed, you can manually install it by running `yarn install` at the root of the repository. | |||
There was a problem hiding this comment.
Please don't use blockquotes, even if it's quoted from elsewhere it's ok to add it without blockquote.
| * Example : [`types.ts`](https://github.com/laurent22/joplin/blob/dev/packages/server/src/utils/types.ts) | ||
|
|
||
|
|
||
| ## Use `camelCase` for `const`ants in new code |
There was a problem hiding this comment.
Another section maybe:
Use the same name for imported and exported members
If you create a file that exports a function called processData(), the file should be called processData.ts and when you import it, you should import it as processData too. Basically be consistent with naming, even though JS allows you to name everything differently.
readme/coding_style.md
Outdated
| ## Declare variables just before their usage | ||
|
|
||
| > What about numeric constants? E.g. | ||
| > ```ts | ||
| > const gravityAcceleration = 9.8; // m/s | ||
| > ``` |
There was a problem hiding this comment.
If they are used multiple times in the file, it's ok to declare them at the top (or even in a different file, and import that).
If it's used only once, it should be declared only before it's used.
|
Another one: Avoid default parametersAs much as possible, avoid default parameters in function calls and interface definitions. When all parameters are required, it makes it a lot easier to refactor the code since the compiler will automatically catch any missing parameter. |
Should function overloading also be avoided? Edit: Yes, for the same reason that default parameters should be avoided (where possible). |
|
That's all very good so let's merge. Thanks for adding this @personalizedrefrigerator! |
Rationale
Not all stylistic issues are caught by
eslint. This pull request documents some of these issues. See the forum post for the original style guidelines and initial discussion.