|
| 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`: single quotes, a four-space indent, no trailing commas, and a 120 column line width. |
| 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 the settings above. That means single quotes, a four-space indent, no trailing comma, and no line past 120 columns. 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