Skip to content

Commit 89c2f36

Browse files
ishakutaishakuta
committed
Add feature flags system and ESLint code quality tools
Features: - 3-tier feature flags: query params, console API, settings UI - Geocoding wrapped as experimental feature (disabled by default) - ESLint with complexity tracking (max complexity: 10, max lines: 50) - Pre-commit hook runs ESLint on every commit - Only errors block commits, warnings allowed New files: - static/thoughts/js/utils/feature-flags.js - Feature flags core - static/thoughts/package.json - ESLint dependencies - static/thoughts/.eslintrc.json - ESLint configuration - scripts/lint.sh - Lint script - ESLINT.md - ESLint documentation - TESTING.md - Test plan (Playwright browser automation) Modified: - static/thoughts/index.html - Experimental features UI section - static/thoughts/js/app.js - Import feature-flags - static/thoughts/js/components/settings-modal.js - Render experimental toggles - static/thoughts/js/components/capture-area.js - Wrap geocoding in feature flag - static/thoughts/js/services/sync.js - Check feature flag before waiting for geocodes - static/thoughts/js/services/geocoding.js - Fix ESLint warnings - static/thoughts/sw.js - Cache feature-flags.js - .gitignore - Ignore node_modules and package-lock.json - THOUGHT_CAPTURE.md - Update roadmap Testing: - Visit /?ff=experimental to enable all experimental features - Use featureFlags.enable('geocoding') in console - ESLint runs: npm run lint (in static/thoughts/) - ESLint auto-runs on commit, blocks only on errors
1 parent c266a14 commit 89c2f36

16 files changed

Lines changed: 1027 additions & 15 deletions

File tree

‎.gitignore‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,8 @@ ehthumbs.db
1818

1919
# Development tools
2020
.claude/
21-
.vscode/
21+
.vscode/
22+
23+
# Node.js (for Thought Capture PWA linting)
24+
node_modules/
25+
package-lock.json

‎ESLINT.md‎

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
# ESLint Setup - Thought Capture PWA
2+
3+
## Overview
4+
5+
ESLint is configured to run on every commit and enforce code quality standards focused on maintainability, complexity, and common errors.
6+
7+
## Running ESLint
8+
9+
### Manual Execution
10+
11+
```bash
12+
# From project root
13+
./scripts/lint.sh
14+
15+
# Or from static/thoughts/
16+
cd static/thoughts
17+
npm run lint # Check for issues
18+
npm run lint:fix # Auto-fix issues where possible
19+
```
20+
21+
### Automatic Execution
22+
23+
ESLint runs automatically on **every commit** that touches `static/thoughts/` files:
24+
25+
```bash
26+
git add .
27+
git commit -m "Your message"
28+
# ESLint runs automatically after version update
29+
```
30+
31+
**Behavior:**
32+
- ✅ **Warnings:** Allowed, commit proceeds
33+
- ❌ **Errors:** Block commit, must be fixed
34+
35+
No environment variable needed - it always runs.
36+
37+
## Configuration
38+
39+
### Files
40+
41+
- **`static/thoughts/package.json`** - npm dependencies
42+
- **`static/thoughts/.eslintrc.json`** - ESLint rules
43+
- **`scripts/lint.sh`** - Lint script (auto-installs deps if needed)
44+
- **`.git/hooks/pre-commit`** - Git hook that runs ESLint
45+
46+
### Rules Enforced
47+
48+
#### Complexity & Maintainability
49+
- **Cyclomatic complexity:** ≤ 10 (warns if exceeded)
50+
- **Max function length:** ≤ 50 lines (warns if exceeded)
51+
- **Max nesting depth:** ≤ 4 levels
52+
- **Max parameters:** ≤ 4 per function
53+
- **Max nested callbacks:** ≤ 3 levels
54+
55+
#### Code Quality
56+
- `no-var` - Must use `let` or `const` (error)
57+
- `prefer-const` - Use `const` when variable isn't reassigned (warning)
58+
- `no-unused-vars` - No unused variables (warning, except prefixed with `_`)
59+
- `eqeqeq` - Use `===` instead of `==` (warning)
60+
- `no-eval` - No `eval()` usage (error)
61+
- `no-implied-eval` - No `setTimeout(string)` (error)
62+
63+
#### Code Style
64+
- `semi` - Require semicolons (error)
65+
- `quotes` - Prefer single quotes (warning)
66+
- `indent` - 4 spaces (warning)
67+
68+
## Suppressing Warnings
69+
70+
Sometimes a function legitimately needs higher complexity. Use inline comments with justification:
71+
72+
```javascript
73+
// eslint-disable-next-line complexity -- Geocoding logic requires multiple conditional branches
74+
async function reverseGeocode(lat, lon) {
75+
// ... complex but necessary logic
76+
}
77+
```
78+
79+
**Guidelines:**
80+
- Always include `--` followed by a reason
81+
- Only suppress when refactoring would reduce readability
82+
- Keep suppressions to a minimum
83+
84+
## Current Status
85+
86+
✅ **All checks passing** (0 errors, 0 warnings)
87+
88+
Suppressions currently in place:
89+
1. `syncFromGitHub()` - Multi-step sync logic (60 lines)
90+
2. `syncToGitHub()` - Validation + grouping logic (complexity 12)
91+
3. `reverseGeocode()` - API response parsing (complexity 13)
92+
93+
## Fixing Common Issues
94+
95+
### Unused variable
96+
```javascript
97+
// ❌ Bad
98+
function example(foo, bar) {
99+
return foo; // bar is unused
100+
}
101+
102+
// ✅ Good (prefix with _)
103+
function example(foo, _bar) {
104+
return foo;
105+
}
106+
107+
// ✅ Good (remove it)
108+
function example(foo) {
109+
return foo;
110+
}
111+
```
112+
113+
### Should use const
114+
```javascript
115+
// ❌ Bad
116+
let cache = [];
117+
cache.push(item);
118+
119+
// ✅ Good
120+
const cache = [];
121+
cache.push(item); // const allows mutation, just not reassignment
122+
```
123+
124+
### Complex function
125+
```javascript
126+
// ❌ Bad (complexity 15)
127+
function processData(data) {
128+
if (a) {
129+
if (b) {
130+
if (c) {
131+
// ... many nested conditions
132+
}
133+
}
134+
}
135+
}
136+
137+
// ✅ Good (split into smaller functions)
138+
function processData(data) {
139+
if (!isValid(data)) return null;
140+
return transform(data);
141+
}
142+
143+
function isValid(data) { /* ... */ }
144+
function transform(data) { /* ... */ }
145+
```
146+
147+
## IDE Integration
148+
149+
### VS Code
150+
151+
Install ESLint extension:
152+
```bash
153+
code --install-extension dbaeumer.vscode-eslint
154+
```
155+
156+
Add to `.vscode/settings.json`:
157+
```json
158+
{
159+
"eslint.workingDirectories": ["static/thoughts"],
160+
"editor.codeActionsOnSave": {
161+
"source.fixAll.eslint": true
162+
}
163+
}
164+
```
165+
166+
### WebStorm / IntelliJ
167+
168+
ESLint is auto-detected. Enable under:
169+
- Settings → Languages & Frameworks → JavaScript → Code Quality Tools → ESLint
170+
- Check "Automatic ESLint configuration"
171+
172+
## Troubleshooting
173+
174+
### ESLint not running on commit?
175+
```bash
176+
# Check hook is executable
177+
ls -la .git/hooks/pre-commit
178+
# Should show -rwxr-xr-x
179+
180+
# If not executable:
181+
chmod +x .git/hooks/pre-commit
182+
```
183+
184+
### Dependencies not installed?
185+
```bash
186+
cd static/thoughts
187+
npm install
188+
```
189+
190+
### Want to skip ESLint for one commit?
191+
```bash
192+
# Not recommended, but if absolutely necessary:
193+
git commit --no-verify -m "Emergency fix"
194+
```
195+
196+
### ESLint version warnings?
197+
These are expected - we're using ESLint 8.x which is stable. ESLint 9.x requires major config changes. We can upgrade later if needed.
198+
199+
## Maintenance
200+
201+
### Update ESLint
202+
```bash
203+
cd static/thoughts
204+
npm update eslint eslint-plugin-complexity
205+
```
206+
207+
### Adjust complexity thresholds
208+
Edit `static/thoughts/.eslintrc.json`:
209+
```json
210+
{
211+
"rules": {
212+
"complexity": ["warn", 15], // Increase from 10 to 15
213+
"max-lines-per-function": ["warn", { "max": 75 }] // Increase from 50 to 75
214+
}
215+
}
216+
```
217+
218+
### Add new rules
219+
See [ESLint Rules](https://eslint.org/docs/latest/rules/) for all available rules.
220+
221+
## Why These Rules?
222+
223+
- **Complexity limits:** Reduce cognitive load, improve testability
224+
- **Function length limits:** Encourage single responsibility principle
225+
- **No `var`:** Prevent scope confusion (block-scoped `let`/`const` are clearer)
226+
- **No `eval`:** Security risk, performance issue
227+
- **Prefer `const`:** Signals intent, prevents accidental reassignment
228+
- **Semicolons:** Avoid ASI (Automatic Semicolon Insertion) edge cases
229+
230+
## Philosophy
231+
232+
ESLint is a **guide**, not a tyrant:
233+
- Warnings suggest improvements but don't block work
234+
- Errors prevent common bugs and security issues
235+
- Suppressions are allowed with justification
236+
- Rules can be adjusted if they don't fit our codebase
237+
238+
The goal is **maintainable, readable code**, not perfect adherence to arbitrary limits.

0 commit comments

Comments
 (0)