Fix bin/publish: use correct .env path for rspade_system Fix bin/publish script: prevent grep exit code 1 from terminating script 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
342 lines
14 KiB
Markdown
Executable File
342 lines
14 KiB
Markdown
Executable File
# Research Report: jqhtml Component Loading State Pattern Issue
|
|
|
|
**Date:** 2025-10-10
|
|
**Author:** AI Assistant (Claude)
|
|
**Subject:** Critical documentation gap in jqhtml component lifecycle - loading state anti-patterns
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
During the development of a DataGrid component for a real-world RSpade application, a critical pattern mistake was identified in how jqhtml components handle loading states. The issue stems from incomplete documentation of jqhtml's automatic re-rendering behavior and lifecycle constraints, leading developers to implement incorrect loading patterns that violate the framework's design principles.
|
|
|
|
**Impact:** Medium-High - This anti-pattern can be implemented without immediate errors but leads to unpredictable component behavior, violates the "fail loud" philosophy, and creates maintenance issues.
|
|
|
|
**Recommendation:** Update jqhtml lifecycle documentation to explicitly document the correct loading state pattern and common anti-patterns with clear examples.
|
|
|
|
---
|
|
|
|
## Problem Statement
|
|
|
|
### What Happened
|
|
|
|
An AI assistant (myself) was tasked with implementing a DataGrid component that loads data asynchronously from a server endpoint. Following common patterns from other frameworks (React, Vue, Angular), the implementation used:
|
|
|
|
1. **Manual `this.render()` calls** in `on_load()` to trigger re-rendering
|
|
2. **Complex nested state objects** (`this.data.state.loading`) to track loading status
|
|
3. **Loading flags set at the START** of `on_load()` before data fetching
|
|
|
|
After framework update, the jqhtml runtime emitted a warning about setting properties other than `this.data` in `on_load()`. The implementation was corrected to use `this.data.state` instead of `this.state`, but this still violated the intended pattern.
|
|
|
|
The user (framework creator) observed the implementation and provided corrective guidance, demonstrating the **correct pattern**:
|
|
|
|
- **NO manual `this.render()` calls** - Framework handles re-rendering automatically
|
|
- **Simple flat `this.data.loaded` flag** - Set to `true` at END of `on_load()`
|
|
- **Template-level loading checks** - Use `<% if (!this.data.loaded) %>` in jqhtml template
|
|
- **Trust automatic re-rendering** - Framework detects `this.data` changes and re-renders
|
|
|
|
### Incorrect Implementation (What I Did)
|
|
|
|
**JavaScript:**
|
|
```javascript
|
|
class Contacts_DataGrid extends Jqhtml_Component {
|
|
async on_load() {
|
|
// ❌ WRONG: Setting loading state at START
|
|
this.data.state = {loading: true};
|
|
this.render(); // ❌ WRONG: Manual render call
|
|
|
|
try {
|
|
const response = await $.ajax({...}); // ❌ WRONG: $.ajax() instead of controller stub
|
|
|
|
if (response.success) {
|
|
this.data = {
|
|
records: response.records,
|
|
total: response.total,
|
|
state: {loading: false} // ❌ WRONG: Complex nested state
|
|
};
|
|
}
|
|
} catch (error) {
|
|
this.data.state = {loading: false, error: true};
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
**Template:**
|
|
```jqhtml
|
|
<% if (this.data.state && this.data.state.loading) { %>
|
|
<!-- ❌ WRONG: Complex state check -->
|
|
<div>Loading...</div>
|
|
<% } else if (this.data.records && this.data.records.length > 0) { %>
|
|
<!-- Data display -->
|
|
<% } %>
|
|
```
|
|
|
|
### Correct Implementation (User's Correction)
|
|
|
|
**JavaScript:**
|
|
```javascript
|
|
class Contacts_DataGrid extends Jqhtml_Component {
|
|
async on_load() {
|
|
// ✅ CORRECT: NO loading flags at start
|
|
// ✅ CORRECT: NO manual this.render() calls
|
|
|
|
// ✅ CORRECT: Use Ajax endpoint pattern
|
|
const response = await Frontend_Contacts_Controller.datagrid_fetch({
|
|
page: 1,
|
|
per_page: 25,
|
|
sort: 'id',
|
|
order: 'asc'
|
|
});
|
|
|
|
// ✅ CORRECT: Populate this.data directly
|
|
this.data.records = response.records;
|
|
this.data.total = response.total;
|
|
this.data.page = response.page;
|
|
this.data.per_page = response.per_page;
|
|
this.data.total_pages = response.total_pages;
|
|
|
|
// ✅ CORRECT: Simple flag at END
|
|
this.data.loaded = true;
|
|
|
|
// ✅ Automatic re-render happens because this.data changed
|
|
}
|
|
}
|
|
```
|
|
|
|
**Template:**
|
|
```jqhtml
|
|
<% if (!this.data || !this.data.loaded) { %>
|
|
<!-- ✅ FIRST RENDER: Loading state (this.data is empty {}) -->
|
|
<div class="loading-spinner text-center py-5">
|
|
<div class="spinner-border text-primary mb-3"></div>
|
|
<p class="text-muted">Loading contacts...</p>
|
|
</div>
|
|
<% } else if (this.data.records && this.data.records.length > 0) { %>
|
|
<!-- ✅ SECOND RENDER: Data loaded -->
|
|
<table class="table">
|
|
<% for (let record of this.data.records) { %>
|
|
<tr><!-- row content --></tr>
|
|
<% } %>
|
|
</table>
|
|
<% } else { %>
|
|
<!-- ✅ Empty state -->
|
|
<div class="text-center py-5">
|
|
<p class="text-muted">No records found</p>
|
|
</div>
|
|
<% } %>
|
|
```
|
|
|
|
---
|
|
|
|
## Root Cause Analysis
|
|
|
|
### Why This Mistake Happened
|
|
|
|
1. **Documentation Gap**: Existing jqhtml documentation focused on:
|
|
- What `on_load()` should do (load async data)
|
|
- What `on_load()` CANNOT do (DOM manipulation)
|
|
- Restriction to only modify `this.data`
|
|
|
|
But it **did not explicitly document**:
|
|
- The automatic re-rendering behavior when `this.data` changes
|
|
- The correct loading state pattern (`this.data.loaded` flag at END)
|
|
- Common anti-patterns (manual `this.render()`, nested state objects)
|
|
|
|
2. **Framework Paradigm Difference**: jqhtml's approach differs from mainstream frameworks:
|
|
- **React/Vue**: Explicit state management, manual render triggers
|
|
- **jqhtml**: Automatic re-rendering on `this.data` change, trust the framework
|
|
|
|
Developers with React/Vue experience naturally implement familiar patterns that violate jqhtml's design.
|
|
|
|
3. **Silent Failure Mode**: The incorrect implementation **appeared to work**:
|
|
- No runtime errors
|
|
- Visual output seemed correct
|
|
- Only violated best practices, not hard constraints
|
|
|
|
This violates RSpade's "fail loud" philosophy - the framework should have caught this earlier.
|
|
|
|
### Why It Matters
|
|
|
|
1. **Unpredictable Behavior**: Manual `this.render()` calls can interfere with the framework's automatic re-rendering, causing double-renders or race conditions.
|
|
|
|
2. **Maintenance Burden**: Complex nested state objects (`this.data.state.loading`) add cognitive overhead and make code harder to understand.
|
|
|
|
3. **Framework Violation**: Setting loading state at the START of `on_load()` means the first render triggers before data loading begins, defeating the purpose of the double-render pattern.
|
|
|
|
4. **Philosophy Violation**: The pattern violates RSpade's "trust the framework" principle - manually calling `this.render()` indicates lack of trust in automatic re-rendering.
|
|
|
|
---
|
|
|
|
## How the Pattern SHOULD Work
|
|
|
|
### The Double-Render Pattern
|
|
|
|
jqhtml components that load data in `on_load()` automatically render **twice**:
|
|
|
|
**Render 1 (Initial)**:
|
|
- Template executes
|
|
- `this.data = {}` (empty object)
|
|
- DOM created with loading state (`<% if (!this.data.loaded) %>` is true)
|
|
- User sees loading spinner
|
|
|
|
**on_load() Executes**:
|
|
- Fetches data from server
|
|
- Populates `this.data.records`, `this.data.total`, etc.
|
|
- Sets `this.data.loaded = true` at END
|
|
- Framework detects `this.data` changed
|
|
|
|
**Render 2 (Automatic)**:
|
|
- Framework automatically re-renders template
|
|
- `this.data.loaded === true` now
|
|
- Template shows data (`<% } else if (this.data.records.length > 0) %>`)
|
|
- User sees actual data
|
|
|
|
**on_ready() Fires**:
|
|
- Called ONCE after second render
|
|
- All children components ready
|
|
- Safe to attach event handlers and DOM manipulation
|
|
|
|
### Key Principles
|
|
|
|
1. **Trust Automatic Re-Rendering**
|
|
- Framework watches `this.data` for changes
|
|
- Modifying `this.data` triggers automatic re-render
|
|
- NEVER manually call `this.render()` in `on_load()`
|
|
|
|
2. **Simple Flat State**
|
|
- Use `this.data.loaded = true` (flat property)
|
|
- NOT `this.data.state.loading = false` (nested object)
|
|
- Simpler to check in templates: `!this.data.loaded`
|
|
|
|
3. **Template-Level Checks**
|
|
- Loading state logic belongs in jqhtml template
|
|
- NOT in JavaScript: `if (!loaded) { this.show_spinner(); }`
|
|
- Declarative template: `<% if (!this.data.loaded) %>`
|
|
|
|
4. **Flag at END Only**
|
|
- Set `this.data.loaded = true` as LAST line of `on_load()`
|
|
- NOT at start: `this.data.loading = true` before fetch
|
|
- Framework handles the "loading" state via empty `this.data`
|
|
|
|
---
|
|
|
|
## Recommendations
|
|
|
|
### 1. Update jqhtml Lifecycle Documentation
|
|
|
|
**Add new section**: "Loading State Pattern" (CRITICAL level)
|
|
|
|
Include:
|
|
- Complete correct pattern example (JS + template)
|
|
- Complete incorrect pattern example (what NOT to do)
|
|
- Explanation of why the pattern works (double-render)
|
|
- Common anti-patterns with warnings
|
|
- Comparison to React/Vue to help developers understand the difference
|
|
|
|
**Location**: After the lifecycle execution flow section, before the double-render pattern section.
|
|
|
|
### 2. Add to "Common Pitfalls" Section
|
|
|
|
Create new subsection: "Loading Pattern Anti-Patterns"
|
|
|
|
Include:
|
|
- ❌ NEVER call `this.render()` manually in `on_load()`
|
|
- ❌ NEVER use nested state objects (`this.data.state.loading`)
|
|
- ❌ NEVER set loading flags at START of `on_load()`
|
|
- ✅ DO set `this.data.loaded = true` at END
|
|
- ✅ DO check `!this.data.loaded` in template for loading state
|
|
|
|
### 3. Framework Warning Enhancement
|
|
|
|
**Current**: Runtime warning when setting properties other than `this.data` in `on_load()`
|
|
|
|
**Suggested**: Additional runtime warnings for:
|
|
- Calling `this.render()` within `on_load()` → "WARNING: Manual render() call detected in on_load(). Remove it - framework handles re-rendering automatically."
|
|
- Setting `this.data.loaded = true` at line 1 of `on_load()` → "WARNING: Loading flag set before data fetch. Move to END of on_load()."
|
|
|
|
These would align with RSpade's "fail loud" philosophy.
|
|
|
|
### 4. Example Components
|
|
|
|
Add reference implementations to framework:
|
|
- **Simple_List_Component** - Loads array of items, shows loading state
|
|
- **Data_Grid_Component** - Loads paginated data, handles empty state
|
|
- **Form_Component** - Loads initial data, handles submission
|
|
|
|
These serve as copy-paste templates for common use cases.
|
|
|
|
### 5. AI Assistant Training
|
|
|
|
Update CLAUDE.md (RSpade AI development guide) to include:
|
|
- Complete loading pattern documentation
|
|
- Anti-pattern warnings in "Common Pitfalls"
|
|
- Links to example components
|
|
|
|
**Status**: ✅ COMPLETED - This research report documents the changes made.
|
|
|
|
---
|
|
|
|
## Impact Assessment
|
|
|
|
### Severity: Medium-High
|
|
|
|
- **Likelihood**: High - Any developer with React/Vue background will likely implement this anti-pattern
|
|
- **Detection Difficulty**: Medium - Code appears to work, only causes issues during maintenance or edge cases
|
|
- **Fix Difficulty**: Low - Pattern is simple once understood
|
|
- **Business Impact**: Medium - Causes unpredictable behavior, violates framework principles, creates maintenance burden
|
|
|
|
### Affected Audience
|
|
|
|
- **Primary**: AI assistants developing with RSpade/jqhtml (high likelihood of implementing anti-pattern)
|
|
- **Secondary**: Human developers new to jqhtml (moderate likelihood)
|
|
- **Tertiary**: Experienced jqhtml developers (low likelihood, but benefits from explicit documentation)
|
|
|
|
### Documentation Priority
|
|
|
|
**CRITICAL** - This should be added to jqhtml documentation immediately because:
|
|
1. High likelihood of implementation by new developers
|
|
2. Violates core framework principles ("trust the framework")
|
|
3. Creates silent failures (appears to work but causes issues)
|
|
4. Simple to fix once pattern is understood
|
|
5. Already causing issues in real-world usage
|
|
|
|
---
|
|
|
|
## Conclusion
|
|
|
|
The loading state pattern issue represents a critical documentation gap in jqhtml's lifecycle documentation. While the framework correctly restricts what can be done in `on_load()` (only modify `this.data`, no DOM manipulation), it does not explicitly document the **correct pattern** for implementing loading states.
|
|
|
|
Developers with experience in other frameworks (React, Vue, Angular) will naturally implement familiar patterns that violate jqhtml's automatic re-rendering design. The framework should document this pattern explicitly with both correct and incorrect examples to prevent these anti-patterns.
|
|
|
|
The recommended fixes are straightforward:
|
|
1. Add comprehensive "Loading State Pattern" section to jqhtml docs
|
|
2. Add "Loading Pattern Anti-Patterns" to common pitfalls
|
|
3. Consider runtime warnings for common mistakes
|
|
4. Provide reference implementations
|
|
|
|
These changes will significantly improve the developer experience and reduce instances of this anti-pattern in production code.
|
|
|
|
---
|
|
|
|
## Appendix: Conversation Timeline
|
|
|
|
1. **Initial Implementation**: AI assistant implements DataGrid with manual `this.render()` calls and nested state
|
|
2. **Framework Update**: jqhtml runtime warns about setting `this.state` instead of `this.data.state`
|
|
3. **First Correction**: AI corrects to use `this.data.state` but still uses nested state and manual render
|
|
4. **User Intervention**: User demonstrates correct pattern with simple `this.data.loaded` flag
|
|
5. **Comprehension Check**: AI explains pattern back to user to confirm understanding
|
|
6. **Documentation Review**: User requests review of CLAUDE.md to identify documentation gaps
|
|
7. **Documentation Update**: AI updates CLAUDE.md with comprehensive loading pattern documentation
|
|
8. **This Report**: Documentation of the issue for jqhtml maintainers
|
|
|
|
---
|
|
|
|
**Next Steps:**
|
|
|
|
1. ✅ Update CLAUDE.md (RSpade AI development guide) - COMPLETED
|
|
2. ⏳ Review this report with jqhtml maintainers
|
|
3. ⏳ Update official jqhtml lifecycle documentation
|
|
4. ⏳ Consider runtime warning enhancements
|
|
5. ⏳ Add reference component examples to framework
|