Skip to content

Commit 66b4764

Browse files
vintavinta
authored
Merge pull request #257 from vinta/feature/survey-async-sync-methods
Feature/survey async sync methods
2 parents 285eafa + b5c2aed commit 66b4764

24 files changed

+1324
-877
lines changed

‎.claude/TODO.md‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,9 @@
2121

2222
### Paranoid Text Spacing Algorithm v7
2323

24-
### XPath to TreeWalker Migration with Idle Processing
25-
2624
- [x] Migrated from `XPath` to `TreeWalker` API
2725
- [x] `MutationObserver` with idle processing for dynamic content
2826
- [x] CSS visibility check for hidden elements (sr-only, visually-hidden)
29-
- **Result**: 5.5x performance improvement + non-blocking processing
3027

3128
## In Progress
3229

‎.claude/researches/async-sync-separation-analysis-poc/README.md‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Async/Sync Separation POC Files
2+
3+
This directory contains proof-of-concept implementations created during the async/sync separation research.
4+
5+
## Files
6+
7+
### TypeScript Implementations
8+
- `pangu-minimal.ts` - Core functionality in ~60 lines, showing the essential logic
9+
- `pangu-simplified.ts` - Full implementation with clear async/sync method separation
10+
- `pangu-ultra-simple.ts` - Even simpler version focusing on API clarity
11+
- `pangu-public-api.ts` - Complete public API design with all convenience methods
12+
- `task-scheduler-simplified.ts` - Simplified task queue without configuration complexity
13+
14+
### Documentation
15+
- `SIMPLIFICATION_COMPARISON.md` - Before/after code comparison
16+
- `SIMPLIFICATION_SUMMARY.md` - Summary of benefits and migration examples
17+
- `SIMPLIFICATION_FINAL.md` - Final analysis and recommendations
18+
19+
## Purpose
20+
21+
These files demonstrate how the codebase could be simplified by:
22+
1. Separating async and sync methods explicitly
23+
2. Removing configuration-based behavior switching
24+
3. Reducing from ~1000 lines to ~300 lines of core logic
25+
26+
## Status
27+
28+
**Not for production use** - These are research artifacts showing what's possible, not actual implementations. The main research conclusions are in the parent `async-sync-separation-analysis.md` file.
29+
30+
## Key Insights
31+
32+
- The core spacing logic is surprisingly simple (~60 lines)
33+
- Most complexity comes from configuration branching
34+
- Explicit method names (`spacingPage()` vs `spacingPageSync()`) are clearer than config flags
35+
- ~250 lines of code could be eliminated with this approach
36+
37+
See the main research document for the final recommendation.

‎.claude/researches/async-sync-separation-analysis-poc/SIMPLIFICATION_COMPARISON.md‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# Code Simplification Comparison
2+
3+
## Before: Complex Configuration-Based Branching
4+
5+
```typescript
6+
// User doesn't know if this is sync or async
7+
pangu.spacingPage();
8+
9+
// Complex internal logic with 4+ execution paths
10+
private spacingTextNodesInQueue(textNodes: Node[], onComplete?: () => void) {
11+
if (this.visibilityDetector.config.enabled) {
12+
if (this.taskScheduler.config.enabled) {
13+
this.taskScheduler.queue.add(() => {
14+
this.spacingTextNodes(textNodes);
15+
});
16+
if (onComplete) {
17+
this.taskScheduler.queue.setOnComplete(onComplete);
18+
}
19+
} else {
20+
this.spacingTextNodes(textNodes);
21+
onComplete?.();
22+
}
23+
return;
24+
}
25+
26+
const task = (chunkedTextNodes: Node[]) => this.spacingTextNodes(chunkedTextNodes);
27+
this.taskScheduler.processInChunks(textNodes, task, onComplete);
28+
}
29+
```
30+
31+
## After: Clear Async/Sync Separation
32+
33+
```typescript
34+
// Async version - always uses requestIdleCallback
35+
await pangu.spacingPage();
36+
37+
// Sync version - always immediate
38+
pangu.spacingPageSync();
39+
40+
// Simple async implementation
41+
async spacingNode(contextNode: Node): Promise<void> {
42+
const textNodes = DomWalker.collectTextNodes(contextNode, true);
43+
44+
if (this.visibilityDetector.config.enabled) {
45+
// Process all nodes in one batch to maintain context
46+
await requestIdleCallbackPromise();
47+
this.spacingTextNodes(textNodes);
48+
} else {
49+
// Process in chunks
50+
await processInChunksAsync(textNodes, (chunk) => this.spacingTextNodes(chunk));
51+
}
52+
}
53+
54+
// Simple sync implementation
55+
spacingNodeSync(contextNode: Node): void {
56+
const textNodes = DomWalker.collectTextNodes(contextNode, true);
57+
this.spacingTextNodes(textNodes);
58+
}
59+
```
60+
61+
## Benefits
62+
63+
1. **API Clarity**: Methods clearly indicate sync vs async behavior
64+
2. **Reduced Complexity**: No more nested if/else branches
65+
3. **Better Types**: TypeScript knows exactly what each method returns
66+
4. **Simpler Testing**: Test sync and async paths independently
67+
5. **No Configuration**: Remove `taskScheduler.config.enabled` entirely
68+
69+
## Usage Examples
70+
71+
### Async Usage (Non-blocking)
72+
```typescript
73+
// For initial page load
74+
await pangu.autoSpacingPage();
75+
76+
// For manual triggering with UI feedback
77+
button.onclick = async () => {
78+
button.disabled = true;
79+
try {
80+
await pangu.spacingPage();
81+
showSuccess();
82+
} catch (error) {
83+
showError(error);
84+
} finally {
85+
button.disabled = false;
86+
}
87+
};
88+
```
89+
90+
### Sync Usage (Immediate)
91+
```typescript
92+
// For small content updates
93+
pangu.spacingNodeSync(commentElement);
94+
95+
// For real-time typing
96+
input.oninput = () => {
97+
pangu.spacingNodeSync(input);
98+
};
99+
```
100+
101+
## Migration Path
102+
103+
1. **Remove TaskScheduler config**: No more `taskScheduler.config.enabled`
104+
2. **Replace callbacks with Promises**: Modern async/await patterns
105+
3. **Simplify MutationObserver**: Always use async for dynamic content
106+
4. **Clear method naming**: `Sync` suffix for synchronous methods
107+
108+
## Code Reduction
109+
110+
- **Removed**: Complex configuration logic, callback management
111+
- **Simplified**: Task queue to basic Promise-based queue
112+
- **Eliminated**: Multiple execution paths through same method
113+
- **Total lines saved**: ~100+ lines of branching logic

‎.claude/researches/async-sync-separation-analysis-poc/SIMPLIFICATION_FINAL.md‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
# Final Simplification Summary
2+
3+
## The Core Problem
4+
5+
The current code has **complex configuration-based behavior** that makes it hard to understand and maintain:
6+
7+
```typescript
8+
// Current: Confusing branching logic
9+
if (this.visibilityDetector.config.enabled) {
10+
if (this.taskScheduler.config.enabled) {
11+
// Path 1: async with batch
12+
} else {
13+
// Path 2: sync with batch
14+
}
15+
} else {
16+
if (this.taskScheduler.config.enabled) {
17+
// Path 3: async with chunks
18+
} else {
19+
// Path 4: sync with chunks
20+
}
21+
}
22+
```
23+
24+
## The Solution: Explicit Async/Sync Methods
25+
26+
Instead of configuration flags, use clear method names:
27+
28+
```typescript
29+
// Proposed: Clear, explicit API
30+
await pangu.spacingPage(); // Always async (uses requestIdleCallback)
31+
pangu.spacingPageSync(); // Always sync (immediate processing)
32+
```
33+
34+
## Benefits of Simplification
35+
36+
### 1. **Remove Configuration Complexity**
37+
- Delete `taskScheduler.config.enabled` entirely
38+
- Behavior is determined by method name, not runtime config
39+
- No more guessing what `pangu.spacingPage()` does
40+
41+
### 2. **Simplify Implementation**
42+
```typescript
43+
// Before: Complex branching
44+
spacingNode(node: Node) {
45+
const textNodes = collectTextNodes(node);
46+
if (this.taskScheduler.config.enabled) {
47+
this.spacingTextNodesInQueue(textNodes);
48+
} else {
49+
this.spacingTextNodes(textNodes);
50+
}
51+
}
52+
53+
// After: Two clear methods
54+
async spacingNode(node: Node): Promise<void> {
55+
const textNodes = collectTextNodes(node);
56+
await this.processAsync(textNodes);
57+
}
58+
59+
spacingNodeSync(node: Node): void {
60+
const textNodes = collectTextNodes(node);
61+
this.processSync(textNodes);
62+
}
63+
```
64+
65+
### 3. **Better TypeScript Support**
66+
```typescript
67+
// TypeScript knows exactly what each method returns
68+
const result1 = await pangu.spacingPage(); // Promise<void>
69+
const result2 = pangu.spacingPageSync(); // void
70+
71+
// Error handling is clearer
72+
try {
73+
await pangu.spacingPage();
74+
} catch (error) {
75+
// Handle async errors properly
76+
}
77+
```
78+
79+
### 4. **Simpler Mental Model**
80+
81+
Current approach requires understanding:
82+
- What `taskScheduler.config.enabled` does
83+
- What `visibilityDetector.config.enabled` does
84+
- How they interact (4 different paths!)
85+
86+
New approach:
87+
- `spacingPage()` = async (non-blocking)
88+
- `spacingPageSync()` = sync (blocking)
89+
- That's it!
90+
91+
## Implementation Strategy
92+
93+
### Phase 1: Add New Methods
94+
```typescript
95+
export class BrowserPangu extends Pangu {
96+
// Async versions (use requestIdleCallback)
97+
async spacingPage(): Promise<void>
98+
async spacingNode(node: Node): Promise<void>
99+
async autoSpacingPage(config?: Config): Promise<void>
100+
101+
// Sync versions (immediate processing)
102+
spacingPageSync(): void
103+
spacingNodeSync(node: Node): void
104+
autoSpacingPageSync(config?: Config): void
105+
}
106+
```
107+
108+
### Phase 2: Simplify Internals
109+
- Remove `spacingTextNodesInQueue()` complexity
110+
- Remove `TaskScheduler` configuration
111+
- Use simple Promise-based idle processing
112+
113+
### Phase 3: Update Public API
114+
```typescript
115+
// All the convenience methods get async/sync versions
116+
async spacingElementById(id: string): Promise<void>
117+
spacingElementByIdSync(id: string): void
118+
119+
async spacingElementsByClassName(className: string): Promise<void>
120+
spacingElementsByClassNameSync(className: string): void
121+
```
122+
123+
## Real-World Usage
124+
125+
```typescript
126+
// Chrome Extension - Use async to avoid blocking UI
127+
button.addEventListener('click', async () => {
128+
button.disabled = true;
129+
try {
130+
await pangu.spacingPage();
131+
showSuccessMessage();
132+
} finally {
133+
button.disabled = false;
134+
}
135+
});
136+
137+
// Real-time editor - Use sync for immediate feedback
138+
editor.addEventListener('input', () => {
139+
pangu.spacingNodeSync(editor);
140+
});
141+
142+
// Initial page load - Use async
143+
window.addEventListener('load', async () => {
144+
await pangu.autoSpacingPage();
145+
});
146+
```
147+
148+
## Code Reduction Estimate
149+
150+
- **Remove**: ~200 lines of configuration logic
151+
- **Remove**: ~100 lines of branching conditions
152+
- **Remove**: Complex callback management
153+
- **Add**: ~50 lines for explicit async/sync methods
154+
- **Net reduction**: ~250 lines (>20% of codebase)
155+
156+
## Summary
157+
158+
By separating async and sync methods explicitly, we:
159+
1. Make the API self-documenting
160+
2. Remove complex configuration dependencies
161+
3. Simplify the implementation dramatically
162+
4. Improve type safety and error handling
163+
5. Make the library easier to understand and maintain
164+
165+
The key insight: **Configuration flags that change fundamental behavior should be replaced with explicit methods**.

0 commit comments

Comments
 (0)