|
| 1 | +# Coding Style Guide |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +This document outlines the coding style and conventions used in the Content Scope Scripts project. |
| 6 | + |
| 7 | +## Code Formatting |
| 8 | + |
| 9 | +### Prettier |
| 10 | + |
| 11 | +We use [Prettier](https://prettier.io/) for automatic code formatting. This ensures consistent code style across the entire codebase. |
| 12 | + |
| 13 | +- **Configuration**: See [`.prettierrc`](.prettierrc) for current formatting settings |
| 14 | +- **IDE Integration**: Enable Prettier in your IDE/editor for automatic formatting on save |
| 15 | +- **CI/CD**: Code formatting is checked in our continuous integration pipeline |
| 16 | + |
| 17 | +### Running Prettier |
| 18 | + |
| 19 | +```bash |
| 20 | +# Format all files |
| 21 | +npm run lint-fix |
| 22 | + |
| 23 | +# Check formatting (without making changes) |
| 24 | +npm run lint |
| 25 | +``` |
| 26 | + |
| 27 | +## TypeScript via JSDoc |
| 28 | + |
| 29 | +We use JSDoc comments to provide TypeScript-like type safety without requiring a TypeScript compilation step. |
| 30 | + |
| 31 | +### JSDoc Resources |
| 32 | + |
| 33 | +- https://devhints.io/jsdoc |
| 34 | +- https://docs.joshuatz.com/cheatsheets/js/jsdoc/ |
| 35 | + |
| 36 | +### Basic JSDoc Usage |
| 37 | + |
| 38 | +```javascript |
| 39 | +/** |
| 40 | + * @param {string} videoId - The video identifier |
| 41 | + * @param {() => void} handler - Callback function to invoke |
| 42 | + * @returns {boolean} Whether the operation was successful |
| 43 | + */ |
| 44 | +function processVideo(videoId, handler) { |
| 45 | + // implementation |
| 46 | +} |
| 47 | +``` |
| 48 | + |
| 49 | +### Type Annotations |
| 50 | + |
| 51 | +```javascript |
| 52 | +// Variable type annotation |
| 53 | +/** @type {HTMLElement|null} */ |
| 54 | +const element = document.getElementById('my-element'); |
| 55 | + |
| 56 | +// Function parameter and return types |
| 57 | +/** |
| 58 | + * @param {HTMLIFrameElement} iframe |
| 59 | + * @returns {(() => void)|null} |
| 60 | + */ |
| 61 | +function setupIframe(iframe) { |
| 62 | + // implementation |
| 63 | +} |
| 64 | +``` |
| 65 | + |
| 66 | +### Interface Definitions |
| 67 | + |
| 68 | +```javascript |
| 69 | +/** |
| 70 | + * @typedef {Object} VideoParams |
| 71 | + * @property {string} id - Video ID |
| 72 | + * @property {string} title - Video title |
| 73 | + * @property {number} duration - Duration in seconds |
| 74 | + */ |
| 75 | + |
| 76 | +/** |
| 77 | + * @typedef {import("./iframe").IframeFeature} IframeFeature |
| 78 | + */ |
| 79 | +``` |
| 80 | + |
| 81 | +## Safety and Defensive Programming |
| 82 | + |
| 83 | +### Type Guards Over Type Casting |
| 84 | + |
| 85 | +When working with DOM elements or external environments (like iframes), prefer runtime type checks over type casting: |
| 86 | + |
| 87 | +```javascript |
| 88 | +// ❌ Avoid type casting when safety is uncertain |
| 89 | +/** @type {Element} */ |
| 90 | +const element = someUnknownValue; |
| 91 | + |
| 92 | +// ✅ Use instanceof checks for runtime safety |
| 93 | +if (!(target instanceof Element)) return; |
| 94 | +const element = target; // TypeScript now knows this is an Element |
| 95 | +``` |
| 96 | + |
| 97 | +### Null/Undefined Checks |
| 98 | + |
| 99 | +Always check for null/undefined when accessing properties that might not exist: |
| 100 | + |
| 101 | +```javascript |
| 102 | +// ❌ Unsafe |
| 103 | +const doc = iframe.contentDocument; |
| 104 | +doc.addEventListener('click', handler); |
| 105 | + |
| 106 | +// ✅ Safe |
| 107 | +const doc = iframe.contentDocument; |
| 108 | +if (!doc) { |
| 109 | + console.log('could not access contentDocument'); |
| 110 | + return; |
| 111 | +} |
| 112 | +doc.addEventListener('click', handler); |
| 113 | +``` |
| 114 | + |
| 115 | +## Best Practices |
| 116 | + |
| 117 | +1. **Use meaningful variable names** that describe their purpose |
| 118 | +2. **Add JSDoc comments** for all public functions and complex logic |
| 119 | +3. **Prefer explicit type checks** over type assertions in uncertain environments |
| 120 | +4. **Handle edge cases** gracefully with proper error handling |
| 121 | +5. **Keep functions small and focused** on a single responsibility |
| 122 | +6. **Design richer return types** to avoid using exceptions as control flow |
| 123 | +7. **Favor implements over extends** to avoid class inheritance (see Interface Implementation section) |
| 124 | +8. **Remove 'index' files** if they only serve to enable re-exports - prefer explicit imports/exports |
| 125 | +9. **Prefer function declarations** over arrow functions for module-level functions |
| 126 | + |
| 127 | +### Return Types and Error Handling |
| 128 | + |
| 129 | +Instead of using exceptions for control flow, design richer return types: |
| 130 | + |
| 131 | +```javascript |
| 132 | +// ❌ Using exceptions for control flow |
| 133 | +function parseVideoId(url) { |
| 134 | + if (!url) throw new Error('URL is required'); |
| 135 | + if (!isValidUrl(url)) throw new Error('Invalid URL'); |
| 136 | + return extractId(url); |
| 137 | +} |
| 138 | + |
| 139 | +// ✅ Using richer return types |
| 140 | +/** |
| 141 | + * @typedef {Object} ParseResult |
| 142 | + * @property {boolean} success |
| 143 | + * @property {string} [videoId] - Present when success is true |
| 144 | + * @property {string} [error] - Present when success is false |
| 145 | + */ |
| 146 | + |
| 147 | +/** |
| 148 | + * @param {string} url |
| 149 | + * @returns {ParseResult} |
| 150 | + */ |
| 151 | +function parseVideoId(url) { |
| 152 | + if (!url) return { success: false, error: 'URL is required' }; |
| 153 | + if (!isValidUrl(url)) return { success: false, error: 'Invalid URL' }; |
| 154 | + return { success: true, videoId: extractId(url) }; |
| 155 | +} |
| 156 | +``` |
| 157 | + |
| 158 | +### Interface Implementation |
| 159 | + |
| 160 | +Favor `implements` over `extends` to avoid class inheritance. While this is awkward in JSDoc, it promotes composition over inheritance: |
| 161 | + |
| 162 | +```javascript |
| 163 | +/** |
| 164 | + * @typedef {Object} IframeFeature |
| 165 | + * @property {function(HTMLIFrameElement): void} iframeDidLoad |
| 166 | + */ |
| 167 | + |
| 168 | +/** |
| 169 | + * @implements {IframeFeature} |
| 170 | + */ |
| 171 | +export class ReplaceWatchLinks { |
| 172 | + /** |
| 173 | + * @param {HTMLIFrameElement} iframe |
| 174 | + */ |
| 175 | + iframeDidLoad(iframe) { |
| 176 | + // implementation |
| 177 | + } |
| 178 | +} |
| 179 | +``` |
| 180 | + |
| 181 | +### Import/Export Patterns |
| 182 | + |
| 183 | +Avoid index files that only serve re-exports. Be explicit about imports: |
| 184 | + |
| 185 | +```javascript |
| 186 | +// ❌ Avoid index.js files with only re-exports |
| 187 | +// index.js |
| 188 | +export { FeatureA } from './feature-a.js'; |
| 189 | +export { FeatureB } from './feature-b.js'; |
| 190 | + |
| 191 | +// ✅ Import directly from source files |
| 192 | +import { FeatureA } from './features/feature-a.js'; |
| 193 | +import { FeatureB } from './features/feature-b.js'; |
| 194 | +``` |
| 195 | + |
| 196 | +### Function Declarations |
| 197 | + |
| 198 | +Prefer function declarations over arrow functions for module-level functions: |
| 199 | + |
| 200 | +```javascript |
| 201 | +// ❌ Arrow functions at module level |
| 202 | +const processVideo = (videoId) => { |
| 203 | + // implementation |
| 204 | +}; |
| 205 | + |
| 206 | +const validateUrl = (url) => { |
| 207 | + // implementation |
| 208 | +}; |
| 209 | + |
| 210 | +// ✅ Function declarations at module level |
| 211 | +function processVideo(videoId) { |
| 212 | + // implementation |
| 213 | +} |
| 214 | + |
| 215 | +function validateUrl(url) { |
| 216 | + // implementation |
| 217 | +} |
| 218 | +``` |
| 219 | + |
| 220 | +**Why function declarations are preferred:** |
| 221 | +- Hoisted, so order doesn't matter |
| 222 | +- Cleaner syntax for longer functions |
| 223 | +- Better stack traces in debugging |
| 224 | +- More conventional for module-level exports |
| 225 | + |
| 226 | +## IDE Configuration |
| 227 | + |
| 228 | +### VS Code |
| 229 | + |
| 230 | +Recommended extensions: |
| 231 | +- Prettier - Code formatter |
| 232 | +- TypeScript and JavaScript Language Features (built-in) |
| 233 | +- ESLint |
| 234 | + |
| 235 | + |
| 236 | +## Linting |
| 237 | + |
| 238 | +We use ESLint for code quality checks. See [`eslint.config.js`](eslint.config.js) for the current linting configuration. |
| 239 | + |
| 240 | +Run linting with: |
| 241 | + |
| 242 | +```bash |
| 243 | +npm run lint |
| 244 | +``` |
| 245 | + |
| 246 | +Follow the linting rules and fix any issues before committing code. |
0 commit comments