|
| 1 | +# Code style |
| 2 | + |
| 3 | +`npm run lint` runs the biome linter, and `biome.json` fixes the formatting settings. This document covers the judgment calls that neither one can make. |
| 4 | + |
| 5 | +## Formatting |
| 6 | + |
| 7 | +Biome owns formatting. The JavaScript rules live in `biome.json`. |
| 8 | + |
| 9 | +Run the formatter on every file you add: |
| 10 | + |
| 11 | +```bash |
| 12 | +npx @biomejs/biome format --write <path> |
| 13 | +``` |
| 14 | + |
| 15 | +Format only files you created. Never pass a directory, and never format a file that already existed. The CLI formats a whole file at a time, and this repository is not formatted from end to end, so either one rewrites lines your change never touched and buries the real diff. |
| 16 | + |
| 17 | +In an existing file, write the lines you add by hand, to follow the rules outlined in `biome.json`. The settings are the house style, so follow them even when the lines around yours predate them. If you formatted such a file by accident, undo your changes and redo the edit. |
| 18 | + |
| 19 | +`npm run lint-fix` also writes. It formats `test/image/mocks` and applies the safe lint fixes across every included path, so run it only when you want both. |
| 20 | + |
| 21 | +## Modernize the lines you touch |
| 22 | + |
| 23 | +Use `const` and `let`, arrow functions, template literals, and `async`/`await` on every line you change. Do not convert the rest of the file. A pull request that modernizes a whole file hides the real change from the reviewer. |
| 24 | + |
| 25 | +Much of this code predates ES6. That is a reason to leave untouched lines alone, not a reason to write pre-ES6 code in the lines you add. |
| 26 | + |
| 27 | +## Extend what exists |
| 28 | + |
| 29 | +Update the existing function instead of adding a helper beside it. A new helper that overlaps an old one leaves the reader with two ways to do one thing, and the old one keeps its callers. |
| 30 | + |
| 31 | +Before you write a helper, search `src/lib/` for the behavior. `Lib` already holds the common cases, including `coerce`, `nestedProperty`, `isPlainObject`, and the date helpers. Color is the exception: it lives in `src/components/color`, not in `Lib`. |
| 32 | + |
| 33 | +The same rule applies to types. Reuse a type from `src/types/` instead of declaring a similar one. |
| 34 | + |
| 35 | +## Do not rename for taste |
| 36 | + |
| 37 | +Keep the diff focused on behavior. Rename an identifier only when the change makes the old name actively wrong. A rename spreads the diff across files and blocks `git blame`. |
| 38 | + |
| 39 | +Do not abbreviate words that the codebase spells out. Write `constructor`, not `ctor`. |
| 40 | + |
| 41 | +## Comments |
| 42 | + |
| 43 | +- Do not rewrite a comment when the replacement means the same thing. Leave the author's phrasing alone. |
| 44 | +- Delete a comment that restates the code. `// footer info` above `getFooter()` is noise. |
| 45 | +- Code must be self-documenting where possible |
| 46 | +- An inline comment gives the reason for the code, not a translation of it |
| 47 | +- A doc comment is a contract: what the unit does, what the caller supplies, what it returns, and how it fails. See [writing-style.md](writing-style.md). |
| 48 | +- Default to no comment. A comment must earn its place for a future maintainer reading the code cold. It must explain *why* something non-obvious is there, never how it was discovered. Naming the specific call site, flag, or test that motivated a defensive line is noise. |
| 49 | + |
| 50 | +## JSDoc |
| 51 | + |
| 52 | +Put the parameter list in one top-level block. Use `@param name - description`. Do not annotate each parameter inline. VS Code renders the first form and drops the second. |
| 53 | + |
| 54 | +```js |
| 55 | +/** |
| 56 | + * Coerce the axis range from user input. |
| 57 | + * |
| 58 | + * @param containerIn - the user-supplied axis container |
| 59 | + * @param containerOut - the full axis container to write into |
| 60 | + * @returns the coerced range, or undefined when the axis is autoranged |
| 61 | + */ |
| 62 | +``` |
| 63 | + |
| 64 | +## TypeScript |
| 65 | + |
| 66 | +The repository moves toward TypeScript. Prefer `.ts` for a new file. Do not run a bulk migration of existing `.js` files as part of another change. |
| 67 | + |
| 68 | +Put `import type` on its own line. The repository has no inline `type` imports. |
| 69 | + |
| 70 | +```ts |
| 71 | +import isNumeric from 'fast-isnumeric'; |
| 72 | +import { BADNUM } from '../constants/numerical'; |
| 73 | +import type { Datum } from '../types/lib/common'; |
| 74 | +``` |
| 75 | + |
| 76 | +Run `npm run typecheck` after any change under `src/types/`. |
| 77 | + |
| 78 | +## Markdown |
| 79 | + |
| 80 | +- Default to writing long sentences without line breaks. Only add line breaks for long lines if the surrounding text uses them. |
| 81 | + |
| 82 | +## Efficiency beats cleverness |
| 83 | + |
| 84 | +This library redraws a whole figure on every interaction, and a figure can carry a million points. So the code that runs per point pays for every abstraction. In `calc`, `plot`, `style`, `hoverPoints`, `selectPoints`, and any loop over a data array, write the plain, obvious, fast thing. |
| 85 | + |
| 86 | +- Use a plain `for` loop over a data array. A chain of `map`, `filter`, and `reduce` allocates an array per step and walks the data once per step. |
| 87 | +- Allocate nothing per point. Reuse an object, or write into a typed array. |
| 88 | +- Hoist the invariant work out of the loop: property lookups, `Lib.nestedProperty` calls, closures, and regular expressions |
| 89 | +- Walk the data once. A short expression that hides a second pass, or an O(n²) scan, costs more than ten plain lines that scan once. |
| 90 | +- Never reach for a clever construct to save a line in a hot path. The reviewer must see the cost of the code from the shape of the code. |
| 91 | + |
| 92 | +Outside the hot paths, clarity wins. The defaults path, the attribute files, and the plot API run once per figure, so write them for the reader. |
| 93 | + |
| 94 | +## plotly.js idioms |
| 95 | + |
| 96 | +- `Lib.coerce` with `dflt: null` deletes the property. An unset attribute reads as `undefined`, not `null`. Test with the loose `== null`. |
| 97 | +- Every attribute needs an `editType`. The flag decides which redraw path runs. A wrong `editType` produces a stale plot with no test failure. |
| 98 | +- Attribute objects must stay JSON-serializable. The schema generator reads them. |
| 99 | +- `supplyDefaults` must scale with the attribute count, not the data point count. Loop over data arrays in `calc` instead. |
| 100 | +- `@plotly/d3` is a fork of d3 v3. Do not reach for a d3 v7 API, and do not propose `@types/d3` v7 or a d3-v7-era submodule version. |
0 commit comments