diff --git a/docs/guides/javascript/react/buildtools.md b/docs/guides/javascript/react/buildtools.md index afdbc59cc..45e04aa71 100644 --- a/docs/guides/javascript/react/buildtools.md +++ b/docs/guides/javascript/react/buildtools.md @@ -46,8 +46,9 @@ files. This is intentionally separate from `grunt watch` — esbuild maintains a incremental build graph internally, so rebuilds reuse the previous parse result rather than restarting from scratch on every change. -After each rebuild, ESLint runs on the changed source files in check-only mode -(no `--fix`) to avoid re-triggering esbuild with auto-fixed rewrites. +Before each rebuild, XO lints the sources in check-only mode (no `--fix`), so a +rewrite never re-triggers esbuild. If XO reports an error, the rebuild is skipped +and the existing bundles are left as they are. ```bash grunt react:watch @@ -61,7 +62,11 @@ grunt react:watch ### React linting {/* #react-linting */} -- `grunt eslint:react` — runs ESLint with `fix: true` (auto-fix) +React sources are linted with [XO](/general/development/policies/codingstyle/typescript), +not ESLint. `grunt react` lints first and does not build if XO reports an error. + +- `grunt xo` — lint only +- `grunt xo --fix` — apply the fixes XO can make itself ### Cross-component alias generation is automatic {/* #cross-component-alias-generation-is-automatic */} @@ -126,10 +131,11 @@ grunt react:dev # Watch React files with esbuild native watch grunt react:watch -# React lint with auto-fix -grunt eslint:react +# Lint React sources with XO, applying the fixes it can make +grunt xo --fix ``` ## See also {/* #see-also */} - [Grunt](/general/development/tools/nodejs#grunt) - How to install Grunt and Watchman +- [TypeScript coding style](/general/development/policies/codingstyle/typescript) - The XO rules and Moodle's overrides diff --git a/general/development/policies/codingstyle/index.md b/general/development/policies/codingstyle/index.md index 013ba1a44..9b8758847 100644 --- a/general/development/policies/codingstyle/index.md +++ b/general/development/policies/codingstyle/index.md @@ -2039,6 +2039,7 @@ This document was drawn from the following sources: - [JavaScript Coding Style](https://docs.moodle.org/dev/Javascript/Coding_Style) - [CSS Coding Style](./css.md) +- [TypeScript coding style](./typescript.md) - [SQL coding style](./sql.md) - [Coding](../../policies.md) - [PHP CodeSniffer](../../tools/phpcs.md) diff --git a/general/development/policies/codingstyle/typescript.md b/general/development/policies/codingstyle/typescript.md new file mode 100644 index 000000000..840643f98 --- /dev/null +++ b/general/development/policies/codingstyle/typescript.md @@ -0,0 +1,139 @@ +--- +title: TypeScript coding style +tags: + - Coding guidelines + - Policies + - Developer processes + - react +--- + + + +This page describes the coding style for the TypeScript and TSX sources under `js/esm/src` in Moodle core and plugins. + +Moodle follows [XO](https://github.com/xojs/xo)'s rules for TypeScript, with a small number of overrides listed below. This page does not repeat XO's rules: the linter is the reference, and it tells you which rule a line breaks and how to fix it. + +AMD and YUI JavaScript is not affected. It is still linted by ESLint, using the rules in `.eslintrc`. + +## Running the linter {/* #running-the-linter */} + +```bash +# Lint every TypeScript source +grunt xo + +# Apply the fixes XO can make itself +grunt xo --fix + +# Lint only some files +grunt xo --files=public/lib/js/esm/src/fetch.ts,public/lib/js/esm/src/url.ts +``` + +The build tasks lint first and stop if XO reports an error, so a bundle is never built from sources that do not pass. This applies to `grunt react`, `grunt js` and plain `grunt`. In [`grunt react:watch`](/docs/guides/javascript/react/buildtools#watch-mode-uses-esbuild-native-context), each change is linted before the rebuild, and the rebuild is skipped while the sources have errors. For installing and running Grunt, see [NodeJS and Grunt](../../tools/nodejs.md#grunt). + +Warnings are reported but do not stop the build. + +:::warning + +Run XO through `grunt xo`, not `npx xo`. Without the source globs that Grunt supplies, XO also lints every Markdown and HTML file in the tree. + +::: + +:::tip + +`--fix` rewrites code, not just whitespace. Review the diff before you commit it. + +::: + +## Rules {/* #rules */} + +The rules are: + +- XO's TypeScript rules +- the two classic rules from [eslint-plugin-react-hooks](https://www.npmjs.com/package/eslint-plugin-react-hooks): + - `react-hooks/rules-of-hooks`, as an error + - `react-hooks/exhaustive-deps`, as a warning +- the Moodle overrides below + +The configuration is in `xo.config.mjs` at the root of the Moodle repository. Each override has its reason next to it. + +:::note + +The newer React Compiler rules in eslint-plugin-react-hooks, such as `react-hooks/refs` and `react-hooks/set-state-in-effect`, are not enabled. `eslint-plugin-react` is not used either, because it does not run on the ESLint version that XO bundles. TypeScript already covers most of what it checks, such as prop types. + +::: + +## Moodle overrides {/* #moodle-overrides */} + +An override is only added where following XO would change something Moodle needs to keep, such as a released API name, a wire format, or the supported browsers. A rule being inconvenient is not a reason to override it. Propose a new override on the tracker. + +### Moodle conventions {/* #moodle-conventions */} + +Some overrides only apply the general [Coding style](./index.md), so they are not explained again here: + +- `@stylistic/max-len` is 132, the [maximum line length](./index.md#maximum-line-length). +- `jsdoc/require-asterisk-prefix` is `always`, matching the [docblock layout](./index.md#documentation-and-comments). +- `jsdoc/check-values` is off, because the [`@license`](./index.md#license) and [`@since`](./index.md#since) tags do not hold the values it expects. + +The others are specific to TypeScript: + +| Rule | Moodle setting | Why | +|---|---|---| +| `jsdoc/require-param`, `jsdoc/check-param-names` | `checkDestructured: false` | An options object needs its own `@param`, but its keys are described once on its type. | +| `unicorn/prefer-https` | off | The GPL header in core's TypeScript files, as in most of core, uses `http://` URLs. | +| `unicorn/filename-case` | camelCase, PascalCase or snake_case | A filename is the module's import name. Components use PascalCase, utility modules camelCase, and existing modules such as `react_autoinit` keep snake_case. | +| `unicorn/name-replacements` | `params`, `utils`, `func`, `args`, `props` and `repository` allowed | The longer names would be worse. `params` and `utils` are published names, `props` is React's own term, and `repository` is the name core uses for web service modules. | +| `unicorn/consistent-boolean-name` | off | Option names match the server-side parameters they are sent as, such as `loginrequired`. | +| `@typescript-eslint/naming-convention` | relaxed | Components may use PascalCase, as JSX requires. Object keys and type members may use any case, because they often name something external, such as an HTTP header or a snake_case web service field. | + +### Supported browsers {/* #supported-browsers */} + +esbuild does not downlevel or polyfill the bundles, so the code must already run in every browser listed under `browserslist` in `package.json`. These rules would otherwise suggest features that are too new: + +| Rule | Moodle setting | What it would suggest | +|---|---|---| +| `unicorn/no-array-sort` | off | `Array.prototype.toSorted()` (ES2023) | +| `unicorn/prefer-promise-with-resolvers` | off | `Promise.withResolvers()` (ES2024) | +| `unicorn/prefer-at` | off | `Array.prototype.at()` (ES2022) | +| `unicorn/logical-assignment-operators` | off | `??=`, `\|\|=` and `&&=` (ES2021) | +| `@typescript-eslint/prefer-nullish-coalescing` | `ignoreIfStatements: true` | `??=` in place of an `if` that assigns a default | +| `require-unicode-regexp` | `requireFlag: 'u'` | The regular expression `v` flag (ES2024) | + +The syntax ones matter most. An older browser cannot parse `??=` or the `v` flag at all, so the whole module fails to load, not just the line that uses them. + +### Null {/* #null */} + +XO bans the `null` type in favour of `undefined`. Moodle keeps `null`, because some values are `null` by contract: + +- the browser's `Storage.getItem()` returns `null` +- React's `useRef(null)` uses `null` +- a web service argument set to `null` is sent in the request, while `undefined` is left out + +`@typescript-eslint/no-restricted-types` is configured with XO's list minus `null`. + +## Disabling a rule inline {/* #disabling-a-rule-inline */} + +Sometimes code has a good reason to break a rule in one place. Disable the rule for that line only, and give the reason after `--`. XO reports a disable comment that has no reason, and one that no longer suppresses anything. + + + +```ts +/* eslint-disable-next-line @typescript-eslint/naming-convention -- Released API name from core/pending. */ +static async Promise( +``` + + + + + +```ts +/* eslint-disable @typescript-eslint/naming-convention */ +static async Promise( +``` + + + +Use a block (`eslint-disable` and `eslint-enable`) only when several lines in a row need the same exception, and give the reason on the opening comment. + +## See also {/* #see-also */} + +- [Build tools](/docs/guides/javascript/react/buildtools) diff --git a/general/development/tools/nodejs.md b/general/development/tools/nodejs.md index 1541f745d..e0dc12e7f 100644 --- a/general/development/tools/nodejs.md +++ b/general/development/tools/nodejs.md @@ -93,7 +93,8 @@ Typical commands: grunt amd # Alias for "ignorefiles", "eslint:amd", "rollup" grunt js # Alias for "amd", "yui" tasks. grunt css # Alias for "scss", "rawcss" tasks. -grunt react # Build all React components. +grunt react # Lint, then build all React components. +grunt xo # Lint TypeScript with XO. Add --fix to apply fixes. grunt shifter # Run Shifter grunt componentlibrary # Build the component library grunt eslint --show-lint-warnings # Show pedantic lint warnings for JS