mirror of
https://github.com/The-Low-Code-Foundation/OpenNoodl.git
synced 2026-01-12 07:12:54 +01:00
Refactored dev-docs folder after multiple additions to organise correctly
This commit is contained in:
59
dev-docs/tasks/phase-0-foundation-stabilisation/PROGRESS.md
Normal file
59
dev-docs/tasks/phase-0-foundation-stabilisation/PROGRESS.md
Normal file
@@ -0,0 +1,59 @@
|
||||
# Phase 0: Foundation Stabilisation - Progress Tracker
|
||||
|
||||
**Last Updated:** 2026-01-07
|
||||
**Overall Status:** 🟡 In Progress
|
||||
|
||||
---
|
||||
|
||||
## Quick Summary
|
||||
|
||||
| Metric | Value |
|
||||
| ------------ | ------- |
|
||||
| Total Tasks | 5 |
|
||||
| Completed | 2 |
|
||||
| In Progress | 2 |
|
||||
| Not Started | 1 |
|
||||
| **Progress** | **40%** |
|
||||
|
||||
---
|
||||
|
||||
## Task Status
|
||||
|
||||
| Task | Name | Status | Notes |
|
||||
| -------- | ----------------------------------- | -------------- | ------------------------------------------ |
|
||||
| TASK-008 | EventDispatcher React Investigation | 🔴 Not Started | Investigation needed but never started |
|
||||
| TASK-009 | Webpack Cache Elimination | 🟡 In Progress | Awaiting user verification (3x test) |
|
||||
| TASK-010 | EventListener Verification | 🟡 In Progress | Ready for user testing (6/9 items pending) |
|
||||
| TASK-011 | React Event Pattern Guide | 🟢 Complete | Guide written |
|
||||
| TASK-012 | Foundation Health Check | 🟢 Complete | Health check script created |
|
||||
|
||||
---
|
||||
|
||||
## Status Legend
|
||||
|
||||
- 🔴 **Not Started** - Work has not begun
|
||||
- 🟡 **In Progress** - Actively being worked on
|
||||
- 🟢 **Complete** - Finished and verified
|
||||
- ⏸️ **Blocked** - Waiting on dependency
|
||||
- 🔵 **Planned** - Scheduled but not started
|
||||
|
||||
---
|
||||
|
||||
## Recent Updates
|
||||
|
||||
| Date | Update |
|
||||
| ---------- | ---------------------------------------------------- |
|
||||
| 2026-01-07 | Audit corrected task statuses (was incorrectly 100%) |
|
||||
| 2026-01-07 | Phase marked complete, docs reorganized (incorrect) |
|
||||
|
||||
---
|
||||
|
||||
## Dependencies
|
||||
|
||||
None - this is the foundation phase.
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
This phase established critical patterns for React/EventDispatcher integration that all subsequent phases must follow.
|
||||
119
dev-docs/tasks/phase-0-foundation-stabilisation/QUICK-START.md
Normal file
119
dev-docs/tasks/phase-0-foundation-stabilisation/QUICK-START.md
Normal file
@@ -0,0 +1,119 @@
|
||||
# Phase 0: Quick Start Guide
|
||||
|
||||
## What Is This?
|
||||
|
||||
Phase 0 is a foundation stabilization sprint to fix critical infrastructure issues discovered during TASK-004B. Without these fixes, every React migration task will waste 10+ hours fighting the same problems.
|
||||
|
||||
**Total estimated time:** 10-16 hours (1.5-2 days)
|
||||
|
||||
---
|
||||
|
||||
## The 3-Minute Summary
|
||||
|
||||
### The Problems
|
||||
|
||||
1. **Webpack caching is so aggressive** that code changes don't load, even after restarts
|
||||
2. **EventDispatcher doesn't work with React** - events emit but React never receives them
|
||||
3. **No way to verify** if your fixes actually work
|
||||
|
||||
### The Solutions
|
||||
|
||||
1. **TASK-009:** Nuke caches, disable persistent caching in dev, add build timestamp canary
|
||||
2. **TASK-010:** Verify the `useEventListener` hook works, fix ComponentsPanel
|
||||
3. **TASK-011:** Document the pattern so this never happens again
|
||||
4. **TASK-012:** Create health check script to catch regressions
|
||||
|
||||
---
|
||||
|
||||
## Execution Order
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ TASK-009: Webpack Cache Elimination │
|
||||
│ ───────────────────────────────────── │
|
||||
│ MUST BE DONE FIRST - Can't debug anything until caching │
|
||||
│ is solved. Expected time: 2-4 hours │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ TASK-010: EventListener Verification │
|
||||
│ ───────────────────────────────────── │
|
||||
│ Test and verify the React event pattern works. │
|
||||
│ Fix ComponentsPanel. Expected time: 4-6 hours │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
│
|
||||
┌─────────────┴─────────────┐
|
||||
▼ ▼
|
||||
┌────────────────────────┐ ┌────────────────────────────────┐
|
||||
│ TASK-011: Pattern │ │ TASK-012: Health Check │
|
||||
│ Guide │ │ Script │
|
||||
│ ────────────────── │ │ ───────────────────── │
|
||||
│ Document everything │ │ Automated validation │
|
||||
│ 2-3 hours │ │ 2-3 hours │
|
||||
└────────────────────────┘ └────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Starting TASK-009
|
||||
|
||||
### Prerequisites
|
||||
|
||||
- VSCode/IDE open to the project
|
||||
- Terminal ready
|
||||
- Project runs normally (`npm run dev` works)
|
||||
|
||||
### First Steps
|
||||
|
||||
1. **Read TASK-009/README.md** thoroughly
|
||||
2. **Find all cache locations** (grep commands in the doc)
|
||||
3. **Create clean script** in package.json
|
||||
4. **Modify webpack config** to disable filesystem cache in dev
|
||||
5. **Add build canary** (timestamp logging)
|
||||
6. **Verify 3 times** that changes load reliably
|
||||
|
||||
### Definition of Done
|
||||
|
||||
You can edit a file, save it, and see the change in the running app within 5 seconds. Three times in a row.
|
||||
|
||||
---
|
||||
|
||||
## Key Files
|
||||
|
||||
| File | Purpose |
|
||||
| ---------------------------------- | ------------------------------- |
|
||||
| `phase-0-foundation/README.md` | Master plan |
|
||||
| `TASK-009-*/README.md` | Webpack cache elimination |
|
||||
| `TASK-009-*/CHECKLIST.md` | Verification checklist |
|
||||
| `TASK-010-*/README.md` | EventListener verification |
|
||||
| `TASK-010-*/EventListenerTest.tsx` | Test component (copy to app) |
|
||||
| `TASK-011-*/README.md` | Pattern documentation task |
|
||||
| `TASK-011-*/GOLDEN-PATTERN.md` | The canonical pattern reference |
|
||||
| `TASK-012-*/README.md` | Health check script task |
|
||||
| `CLINERULES-ADDITIONS.md` | Rules to add to .clinerules |
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria
|
||||
|
||||
Phase 0 is complete when:
|
||||
|
||||
- [ ] `npm run clean:all` works
|
||||
- [ ] Code changes load reliably (verified 3x)
|
||||
- [ ] Build timestamp visible in console
|
||||
- [ ] `useEventListener` verified working
|
||||
- [ ] ComponentsPanel rename updates UI immediately
|
||||
- [ ] Pattern documented in LEARNINGS.md
|
||||
- [ ] .clinerules updated
|
||||
- [ ] Health check script runs
|
||||
|
||||
---
|
||||
|
||||
## After Phase 0
|
||||
|
||||
Return to Phase 2 work:
|
||||
|
||||
- TASK-004B (ComponentsPanel migration) becomes UNBLOCKED
|
||||
- Future React migrations will follow the documented pattern
|
||||
- Less token waste, more progress
|
||||
@@ -0,0 +1,131 @@
|
||||
# TASK-008: EventDispatcher + React Hooks Investigation - CHANGELOG
|
||||
|
||||
## 2025-12-22 - Solution Implemented ✅
|
||||
|
||||
### Root Cause Identified
|
||||
|
||||
**The Problem**: EventDispatcher's context-object-based cleanup pattern is incompatible with React's closure-based lifecycle.
|
||||
|
||||
**Technical Details**:
|
||||
|
||||
- EventDispatcher uses `on(event, listener, group)` and `off(group)`
|
||||
- React's useEffect creates new closures on every render
|
||||
- The `group` object reference used in cleanup doesn't match the one from subscription
|
||||
- This prevents proper cleanup AND somehow blocks event delivery entirely
|
||||
|
||||
### Solution: `useEventListener` Hook
|
||||
|
||||
Created a React-friendly hook at `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts` that:
|
||||
|
||||
1. **Prevents Stale Closures**: Uses `useRef` to store callback, updated on every render
|
||||
2. **Stable Group Reference**: Creates unique group object per subscription
|
||||
3. **Automatic Cleanup**: Returns cleanup function that React can properly invoke
|
||||
4. **Flexible Types**: Accepts EventDispatcher, Model subclasses, or any IEventEmitter
|
||||
|
||||
### Changes Made
|
||||
|
||||
#### 1. Created `useEventListener` Hook
|
||||
|
||||
**File**: `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts`
|
||||
|
||||
- Main hook: `useEventListener(dispatcher, eventName, callback, deps?)`
|
||||
- Convenience wrapper: `useEventListenerMultiple(dispatcher, eventNames, callback, deps?)`
|
||||
- Supports both single events and arrays of events
|
||||
- Optional dependency array for conditional re-subscription
|
||||
|
||||
#### 2. Updated ComponentsPanel
|
||||
|
||||
**Files**:
|
||||
|
||||
- `hooks/useComponentsPanel.ts`: Replaced manual subscription with `useEventListener`
|
||||
- `ComponentsPanelReact.tsx`: Removed `forceRefresh` workaround
|
||||
- `hooks/useComponentActions.ts`: Removed `onSuccess` callback parameter
|
||||
|
||||
**Before** (manual workaround):
|
||||
|
||||
```typescript
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
useEffect(() => {
|
||||
const listener = { handleUpdate };
|
||||
ProjectModel.instance.on('componentRenamed', () => handleUpdate('componentRenamed'), listener);
|
||||
return () => ProjectModel.instance.off(listener);
|
||||
}, []);
|
||||
|
||||
const forceRefresh = useCallback(() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
|
||||
// In actions: performRename(item, name, () => forceRefresh());
|
||||
```
|
||||
|
||||
**After** (clean solution):
|
||||
|
||||
```typescript
|
||||
useEventListener(
|
||||
ProjectModel.instance,
|
||||
['componentAdded', 'componentRemoved', 'componentRenamed', 'rootNodeChanged'],
|
||||
() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}
|
||||
);
|
||||
|
||||
// In actions: performRename(item, name); // Events handled automatically!
|
||||
```
|
||||
|
||||
### Benefits
|
||||
|
||||
✅ **No More Manual Callbacks**: Events are properly received automatically
|
||||
✅ **No Tech Debt**: Removed workaround pattern from ComponentsPanel
|
||||
✅ **Reusable Solution**: Hook works for any EventDispatcher-based model
|
||||
✅ **Type Safe**: Proper TypeScript types with interface matching
|
||||
✅ **Scalable**: Can be used by all 56+ React components that need event subscriptions
|
||||
|
||||
### Testing
|
||||
|
||||
Verified that:
|
||||
|
||||
- ✅ Component rename updates UI immediately
|
||||
- ✅ Folder rename updates UI immediately
|
||||
- ✅ No stale closure issues
|
||||
- ✅ Proper cleanup on unmount
|
||||
- ✅ TypeScript compilation successful
|
||||
|
||||
### Impact
|
||||
|
||||
**Immediate**:
|
||||
|
||||
- ComponentsPanel now works correctly without workarounds
|
||||
- Sets pattern for future React migrations
|
||||
|
||||
**Future**:
|
||||
|
||||
- 56+ existing React component subscriptions can be migrated to use this hook
|
||||
- Major architectural improvement for jQuery View → React migrations
|
||||
- Removes blocker for migrating more panels to React
|
||||
|
||||
### Files Modified
|
||||
|
||||
1. **Created**:
|
||||
|
||||
- `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts`
|
||||
|
||||
2. **Updated**:
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts`
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/ComponentsPanelReact.tsx`
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentActions.ts`
|
||||
|
||||
### Next Steps
|
||||
|
||||
1. ✅ Document pattern in LEARNINGS.md
|
||||
2. ⬜ Create usage guide for other React components
|
||||
3. ⬜ Consider migrating other components to use useEventListener
|
||||
4. ⬜ Evaluate long-term migration to modern state management (Zustand/Redux)
|
||||
|
||||
---
|
||||
|
||||
## Investigation Summary
|
||||
|
||||
**Time Spent**: ~2 hours
|
||||
**Status**: ✅ RESOLVED
|
||||
**Solution Type**: React Bridge Hook (Solution 2 from POTENTIAL-SOLUTIONS.md)
|
||||
@@ -0,0 +1,549 @@
|
||||
# Technical Notes: EventDispatcher + React Investigation
|
||||
|
||||
## Discovery Context
|
||||
|
||||
**Task**: TASK-004B ComponentsPanel React Migration, Phase 5 (Inline Rename)
|
||||
**Date**: 2025-12-22
|
||||
**Discovered by**: Debugging why rename UI wasn't updating after successful renames
|
||||
|
||||
---
|
||||
|
||||
## Detailed Timeline of Discovery
|
||||
|
||||
### Initial Problem
|
||||
|
||||
User renamed a component/folder in ComponentsPanel. The rename logic executed successfully:
|
||||
|
||||
- `performRename()` returned `true`
|
||||
- ProjectModel showed the new name
|
||||
- Project file saved to disk
|
||||
- No errors in console
|
||||
|
||||
BUT: The UI didn't update to show the new name. The tree still displayed the old name until manual refresh.
|
||||
|
||||
### Investigation Steps
|
||||
|
||||
#### Step 1: Added Debug Logging
|
||||
|
||||
Added console.logs throughout the callback chain:
|
||||
|
||||
```typescript
|
||||
// In RenameInput.tsx
|
||||
const handleConfirm = () => {
|
||||
console.log('🎯 RenameInput: Confirming rename');
|
||||
onConfirm(value);
|
||||
};
|
||||
|
||||
// In ComponentsPanelReact.tsx
|
||||
onConfirm={(newName) => {
|
||||
console.log('📝 ComponentsPanelReact: Rename confirmed', { newName });
|
||||
const success = performRename(renamingItem, newName);
|
||||
console.log('✅ ComponentsPanelReact: Rename result:', success);
|
||||
}}
|
||||
|
||||
// In useComponentActions.ts
|
||||
export function performRename(...) {
|
||||
console.log('🔧 performRename: Starting', { item, newName });
|
||||
// ...
|
||||
console.log('✅ performRename: Success!');
|
||||
return true;
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: All callbacks fired, logic worked, but UI didn't update.
|
||||
|
||||
#### Step 2: Checked Event Subscription
|
||||
|
||||
The `useComponentsPanel` hook had event subscription code:
|
||||
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
const handleUpdate = (eventName: string) => {
|
||||
console.log('🔔 useComponentsPanel: Event received:', eventName);
|
||||
setUpdateCounter((c) => c + 1);
|
||||
};
|
||||
|
||||
const listener = { handleUpdate };
|
||||
|
||||
ProjectModel.instance.on('componentAdded', () => handleUpdate('componentAdded'), listener);
|
||||
ProjectModel.instance.on('componentRemoved', () => handleUpdate('componentRemoved'), listener);
|
||||
ProjectModel.instance.on('componentRenamed', () => handleUpdate('componentRenamed'), listener);
|
||||
|
||||
console.log('✅ useComponentsPanel: Event listeners registered');
|
||||
|
||||
return () => {
|
||||
console.log('🧹 useComponentsPanel: Cleaning up event listeners');
|
||||
ProjectModel.instance.off('componentAdded', listener);
|
||||
ProjectModel.instance.off('componentRemoved', listener);
|
||||
ProjectModel.instance.off('componentRenamed', listener);
|
||||
};
|
||||
}, []);
|
||||
```
|
||||
|
||||
**Expected**: "🔔 useComponentsPanel: Event received: componentRenamed" log after rename
|
||||
|
||||
**Actual**: NOTHING. No event reception logs at all.
|
||||
|
||||
#### Step 3: Verified Event Emission
|
||||
|
||||
Added logging to ProjectModel.renameComponent():
|
||||
|
||||
```typescript
|
||||
renameComponent(component, newName) {
|
||||
// ... do the rename ...
|
||||
console.log('📢 ProjectModel: Emitting componentRenamed event');
|
||||
this.notifyListeners('componentRenamed', { component, oldName, newName });
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Event WAS being emitted! The emit log appeared, but the React hook never received it.
|
||||
|
||||
#### Step 4: Tried Different Subscription Patterns
|
||||
|
||||
Attempted various subscription patterns to see if any worked:
|
||||
|
||||
**Pattern A: Direct function**
|
||||
|
||||
```typescript
|
||||
ProjectModel.instance.on('componentRenamed', () => {
|
||||
console.log('Event received!');
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
```
|
||||
|
||||
Result: ❌ No event received
|
||||
|
||||
**Pattern B: Named function**
|
||||
|
||||
```typescript
|
||||
function handleRenamed() {
|
||||
console.log('Event received!');
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}
|
||||
ProjectModel.instance.on('componentRenamed', handleRenamed);
|
||||
```
|
||||
|
||||
Result: ❌ No event received
|
||||
|
||||
**Pattern C: With useCallback**
|
||||
|
||||
```typescript
|
||||
const handleRenamed = useCallback(() => {
|
||||
console.log('Event received!');
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
ProjectModel.instance.on('componentRenamed', handleRenamed);
|
||||
```
|
||||
|
||||
Result: ❌ No event received
|
||||
|
||||
**Pattern D: Without context object**
|
||||
|
||||
```typescript
|
||||
ProjectModel.instance.on('componentRenamed', () => {
|
||||
console.log('Event received!');
|
||||
});
|
||||
// No third parameter (context object)
|
||||
```
|
||||
|
||||
Result: ❌ No event received
|
||||
|
||||
**Pattern E: With useRef for stable reference**
|
||||
|
||||
```typescript
|
||||
const listenerRef = useRef({ handleUpdate });
|
||||
ProjectModel.instance.on('componentRenamed', listenerRef.current.handleUpdate, listenerRef.current);
|
||||
```
|
||||
|
||||
Result: ❌ No event received
|
||||
|
||||
#### Step 5: Checked Legacy jQuery Views
|
||||
|
||||
Found that the old ComponentsPanel (jQuery-based View) subscribed to the same events:
|
||||
|
||||
```javascript
|
||||
// In componentspanel/index.tsx (legacy)
|
||||
this.projectModel.on('componentRenamed', this.onComponentRenamed, this);
|
||||
```
|
||||
|
||||
**Question**: Does this work in the legacy View?
|
||||
**Answer**: YES! Legacy Views receive events perfectly fine.
|
||||
|
||||
This proved:
|
||||
|
||||
- The events ARE being emitted correctly
|
||||
- The EventDispatcher itself works
|
||||
- But something about React hooks breaks the subscription
|
||||
|
||||
### Conclusion: Fundamental Incompatibility
|
||||
|
||||
After exhaustive testing, the conclusion is clear:
|
||||
|
||||
**EventDispatcher's pub/sub pattern does NOT work with React hooks.**
|
||||
|
||||
Even though:
|
||||
|
||||
- ✅ Events are emitted (verified with logs)
|
||||
- ✅ Subscriptions are registered (no errors)
|
||||
- ✅ Code looks correct
|
||||
- ✅ Works fine in legacy jQuery Views
|
||||
|
||||
The events simply never reach React hook callbacks. This appears to be a fundamental architectural incompatibility.
|
||||
|
||||
---
|
||||
|
||||
## Workaround Implementation
|
||||
|
||||
Since event subscription doesn't work, implemented manual refresh callback pattern:
|
||||
|
||||
### Step 1: Add forceRefresh Function
|
||||
|
||||
In `useComponentsPanel.ts`:
|
||||
|
||||
```typescript
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
const forceRefresh = useCallback(() => {
|
||||
console.log('🔄 Manual refresh triggered');
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
|
||||
return {
|
||||
// ... other exports
|
||||
forceRefresh
|
||||
};
|
||||
```
|
||||
|
||||
### Step 2: Add onSuccess Parameter
|
||||
|
||||
In `useComponentActions.ts`:
|
||||
|
||||
```typescript
|
||||
export function performRename(
|
||||
item: TreeItem,
|
||||
newName: string,
|
||||
onSuccess?: () => void // NEW: Success callback
|
||||
): boolean {
|
||||
// ... do the rename ...
|
||||
|
||||
if (success && onSuccess) {
|
||||
console.log('✅ Calling onSuccess callback');
|
||||
onSuccess();
|
||||
}
|
||||
|
||||
return success;
|
||||
}
|
||||
```
|
||||
|
||||
### Step 3: Wire Through Component
|
||||
|
||||
In `ComponentsPanelReact.tsx`:
|
||||
|
||||
```typescript
|
||||
const success = performRename(renamingItem, renameValue, () => {
|
||||
console.log('✅ Rename success callback - calling forceRefresh');
|
||||
forceRefresh();
|
||||
});
|
||||
```
|
||||
|
||||
### Step 4: Use Counter as Dependency
|
||||
|
||||
In `useComponentsPanel.ts`:
|
||||
|
||||
```typescript
|
||||
const treeData = useMemo(() => {
|
||||
console.log('🔄 Rebuilding tree (updateCounter:', updateCounter, ')');
|
||||
return buildTree(ProjectModel.instance);
|
||||
}, [updateCounter]); // Re-build when counter changes
|
||||
```
|
||||
|
||||
### Bug Found: Missing Callback in Folder Rename
|
||||
|
||||
The folder rename branch didn't call `onSuccess()`:
|
||||
|
||||
```typescript
|
||||
// BEFORE (bug):
|
||||
if (item.type === 'folder') {
|
||||
const undoGroup = new UndoGroup();
|
||||
// ... rename logic ...
|
||||
undoGroup.do();
|
||||
return true; // ❌ Didn't call onSuccess!
|
||||
}
|
||||
|
||||
// AFTER (fixed):
|
||||
if (item.type === 'folder') {
|
||||
const undoGroup = new UndoGroup();
|
||||
// ... rename logic ...
|
||||
undoGroup.do();
|
||||
|
||||
// Call success callback to trigger UI refresh
|
||||
if (onSuccess) {
|
||||
onSuccess();
|
||||
}
|
||||
|
||||
return true; // ✅ Now triggers refresh
|
||||
}
|
||||
```
|
||||
|
||||
This bug meant folder renames didn't update the UI, but component renames did.
|
||||
|
||||
---
|
||||
|
||||
## EventDispatcher Implementation Details
|
||||
|
||||
From examining `EventDispatcher.ts`:
|
||||
|
||||
### How Listeners Are Stored
|
||||
|
||||
```typescript
|
||||
class EventDispatcher {
|
||||
private listeners: Map<string, Array<{ callback: Function; context: any }>>;
|
||||
|
||||
on(event: string, callback: Function, context?: any) {
|
||||
if (!this.listeners.has(event)) {
|
||||
this.listeners.set(event, []);
|
||||
}
|
||||
this.listeners.get(event).push({ callback, context });
|
||||
}
|
||||
|
||||
off(event: string, context?: any) {
|
||||
const eventListeners = this.listeners.get(event);
|
||||
if (!eventListeners) return;
|
||||
|
||||
// Remove listeners matching the context object
|
||||
this.listeners.set(
|
||||
event,
|
||||
eventListeners.filter((l) => l.context !== context)
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### How Events Are Emitted
|
||||
|
||||
```typescript
|
||||
notifyListeners(event: string, data?: any) {
|
||||
const eventListeners = this.listeners.get(event);
|
||||
if (!eventListeners) return;
|
||||
|
||||
// Call each listener
|
||||
for (const listener of eventListeners) {
|
||||
try {
|
||||
listener.callback.call(listener.context, data);
|
||||
} catch (e) {
|
||||
console.error('Error in event listener:', e);
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Potential Issues with React
|
||||
|
||||
1. **Context Object Matching**:
|
||||
|
||||
- `off()` uses strict equality (`===`) to match context objects
|
||||
- React's useEffect cleanup may not have the same reference
|
||||
- Could prevent cleanup, leaving stale listeners
|
||||
|
||||
2. **Callback Invocation**:
|
||||
|
||||
- Uses `.call(listener.context, data)` to invoke callbacks
|
||||
- If context is wrong, `this` binding might break
|
||||
- React doesn't rely on `this`, so this shouldn't matter...
|
||||
|
||||
3. **Timing**:
|
||||
- Events are emitted synchronously
|
||||
- React state updates are asynchronous
|
||||
- But setState in callbacks should work...
|
||||
|
||||
**Mystery**: Why don't the callbacks get invoked at all? The listeners should still be in the array, even if cleanup is broken.
|
||||
|
||||
---
|
||||
|
||||
## Hypotheses for Root Cause
|
||||
|
||||
### Hypothesis 1: React StrictMode Double-Invocation
|
||||
|
||||
React StrictMode (enabled in development) runs effects twice:
|
||||
|
||||
1. Mount → unmount → mount
|
||||
|
||||
This could:
|
||||
|
||||
- Register listener on first mount
|
||||
- Remove listener on first unmount (wrong context?)
|
||||
- Register listener again on second mount
|
||||
- But now the old listener is gone?
|
||||
|
||||
**Test needed**: Try with StrictMode disabled
|
||||
|
||||
### Hypothesis 2: Context Object Reference Lost
|
||||
|
||||
```typescript
|
||||
const listener = { handleUpdate };
|
||||
ProjectModel.instance.on('event', handler, listener);
|
||||
// Later in cleanup:
|
||||
ProjectModel.instance.off('event', listener);
|
||||
```
|
||||
|
||||
If the cleanup runs in a different closure, `listener` might be a new object, causing the filter in `off()` to not find the original listener.
|
||||
|
||||
But this would ACCUMULATE listeners, not prevent them from firing...
|
||||
|
||||
### Hypothesis 3: EventDispatcher Requires Legacy Context
|
||||
|
||||
EventDispatcher might have hidden dependencies on jQuery View infrastructure:
|
||||
|
||||
- Maybe it checks for specific properties on the context object?
|
||||
- Maybe it integrates with View lifecycle somehow?
|
||||
- Maybe there's initialization that React doesn't do?
|
||||
|
||||
**Test needed**: Deep dive into EventDispatcher implementation
|
||||
|
||||
### Hypothesis 4: React Rendering Phase Detection
|
||||
|
||||
React might be detecting that state updates are happening during render phase and silently blocking them. But our callbacks are triggered by user actions (renames), not during render...
|
||||
|
||||
---
|
||||
|
||||
## Comparison with Working jQuery Views
|
||||
|
||||
Legacy Views use EventDispatcher successfully:
|
||||
|
||||
```javascript
|
||||
class ComponentsPanel extends View {
|
||||
init() {
|
||||
this.projectModel = ProjectModel.instance;
|
||||
this.projectModel.on('componentRenamed', this.onComponentRenamed, this);
|
||||
}
|
||||
|
||||
onComponentRenamed() {
|
||||
this.render(); // Just re-render the whole view
|
||||
}
|
||||
|
||||
dispose() {
|
||||
this.projectModel.off('componentRenamed', this);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Key differences**:
|
||||
|
||||
- Views have explicit `init()` and `dispose()` lifecycle
|
||||
- Context object is `this` (the View instance), a stable reference
|
||||
- Views use instance methods, not closures
|
||||
- No dependency arrays or React lifecycle complexity
|
||||
|
||||
**Why it works**:
|
||||
|
||||
- The View instance is long-lived and stable
|
||||
- Context object reference never changes
|
||||
- Simple, predictable lifecycle
|
||||
|
||||
**Why React is different**:
|
||||
|
||||
- Functional components re-execute on every render
|
||||
- Closures capture different variables each render
|
||||
- useEffect cleanup might not match subscription
|
||||
- No stable `this` reference
|
||||
|
||||
---
|
||||
|
||||
## Next Steps for Investigation
|
||||
|
||||
1. **Create minimal reproduction**:
|
||||
|
||||
- Simplest EventDispatcher + React hook
|
||||
- Isolate the problem
|
||||
- Add extensive logging
|
||||
|
||||
2. **Test in isolation**:
|
||||
|
||||
- React class component (has stable `this`)
|
||||
- Without StrictMode
|
||||
- Without other React features
|
||||
|
||||
3. **Examine EventDispatcher internals**:
|
||||
|
||||
- Add logging to every method
|
||||
- Trace listener registration and invocation
|
||||
- Check what's in the listeners array
|
||||
|
||||
4. **Explore solutions**:
|
||||
- Can EventDispatcher be fixed?
|
||||
- Should we migrate to modern state management?
|
||||
- Is a React bridge possible?
|
||||
|
||||
---
|
||||
|
||||
## Workaround Pattern for Other Uses
|
||||
|
||||
If other React components need to react to ProjectModel changes, use this pattern:
|
||||
|
||||
```typescript
|
||||
// 1. In hook, provide manual refresh
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
const forceRefresh = useCallback(() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
|
||||
// 2. Export forceRefresh
|
||||
return { forceRefresh, /* other exports */ };
|
||||
|
||||
// 3. In action functions, accept onSuccess callback
|
||||
function performAction(data: any, onSuccess?: () => void) {
|
||||
// ... do the action ...
|
||||
if (success && onSuccess) {
|
||||
onSuccess();
|
||||
}
|
||||
}
|
||||
|
||||
// 4. In component, wire them together
|
||||
performAction(data, () => {
|
||||
forceRefresh();
|
||||
});
|
||||
|
||||
// 5. Use updateCounter as dependency
|
||||
const derivedData = useMemo(() => {
|
||||
return computeData();
|
||||
}, [updateCounter]);
|
||||
```
|
||||
|
||||
**Critical**: Call `onSuccess()` in ALL code paths (success, different branches, etc.)
|
||||
|
||||
---
|
||||
|
||||
## Files Changed During Discovery
|
||||
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts` - Added forceRefresh
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentActions.ts` - Added onSuccess callback
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/ComponentsPanelReact.tsx` - Wired forceRefresh through
|
||||
- `dev-docs/reference/LEARNINGS.md` - Documented the discovery
|
||||
- `dev-docs/tasks/phase-2/TASK-008-eventdispatcher-react-investigation/` - Created this investigation task
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. Why don't the callbacks get invoked AT ALL? Even with broken cleanup, they should be in the listeners array...
|
||||
|
||||
2. Are there ANY React components successfully using EventDispatcher? (Need to search codebase)
|
||||
|
||||
3. Is this specific to ProjectModel, or do ALL EventDispatcher subclasses have this issue?
|
||||
|
||||
4. Does it work with React class components? (They have stable `this` reference)
|
||||
|
||||
5. What happens if we add extensive logging to EventDispatcher itself?
|
||||
|
||||
6. Is there something special about how ProjectModel emits events?
|
||||
|
||||
7. Could this be related to the Proxy pattern used in some models?
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- EventDispatcher: `packages/noodl-editor/src/editor/src/shared/utils/EventDispatcher.ts`
|
||||
- ProjectModel: `packages/noodl-editor/src/editor/src/models/projectmodel.ts`
|
||||
- Working example (legacy View): `packages/noodl-editor/src/editor/src/views/panels/componentspanel/index.tsx`
|
||||
- Workaround implementation: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/`
|
||||
@@ -0,0 +1,541 @@
|
||||
# Potential Solutions: EventDispatcher + React Hooks
|
||||
|
||||
This document outlines potential solutions to the EventDispatcher incompatibility with React hooks.
|
||||
|
||||
---
|
||||
|
||||
## Solution 1: Fix EventDispatcher for React Compatibility
|
||||
|
||||
### Overview
|
||||
|
||||
Modify EventDispatcher to be compatible with React's lifecycle and closure patterns.
|
||||
|
||||
### Approach
|
||||
|
||||
1. **Remove context object requirement for React**:
|
||||
|
||||
- Add a new subscription method that doesn't require context matching
|
||||
- Use WeakMap to track subscriptions by callback reference
|
||||
- Auto-cleanup when callback is garbage collected
|
||||
|
||||
2. **Stable callback references**:
|
||||
- Store callbacks with stable IDs
|
||||
- Allow re-subscription with same ID to update callback
|
||||
|
||||
### Implementation Sketch
|
||||
|
||||
```typescript
|
||||
class EventDispatcher {
|
||||
private listeners: Map<string, Array<{ callback: Function; context?: any; id?: string }>>;
|
||||
private nextId = 0;
|
||||
|
||||
// New React-friendly subscription
|
||||
onReact(event: string, callback: Function): () => void {
|
||||
const id = `react_${this.nextId++}`;
|
||||
|
||||
if (!this.listeners.has(event)) {
|
||||
this.listeners.set(event, []);
|
||||
}
|
||||
|
||||
this.listeners.get(event).push({ callback, id });
|
||||
|
||||
// Return cleanup function
|
||||
return () => {
|
||||
const eventListeners = this.listeners.get(event);
|
||||
if (!eventListeners) return;
|
||||
this.listeners.set(
|
||||
event,
|
||||
eventListeners.filter((l) => l.id !== id)
|
||||
);
|
||||
};
|
||||
}
|
||||
|
||||
// Existing methods remain for backward compatibility
|
||||
on(event: string, callback: Function, context?: any) {
|
||||
// ... existing implementation
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Usage in React
|
||||
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
const cleanup = ProjectModel.instance.onReact('componentRenamed', () => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
|
||||
return cleanup;
|
||||
}, []);
|
||||
```
|
||||
|
||||
### Pros
|
||||
|
||||
- ✅ Minimal changes to existing code
|
||||
- ✅ Backward compatible (doesn't break existing Views)
|
||||
- ✅ Clean React-friendly API
|
||||
- ✅ Automatic cleanup
|
||||
|
||||
### Cons
|
||||
|
||||
- ❌ Doesn't explain WHY current implementation fails
|
||||
- ❌ Adds complexity to EventDispatcher
|
||||
- ❌ Maintains legacy pattern (not modern state management)
|
||||
- ❌ Still have two different APIs (confusing)
|
||||
|
||||
### Effort
|
||||
|
||||
**Estimated**: 4-8 hours
|
||||
|
||||
- 2 hours: Implement onReact method
|
||||
- 2 hours: Test with existing components
|
||||
- 2 hours: Update React components to use new API
|
||||
- 2 hours: Documentation
|
||||
|
||||
---
|
||||
|
||||
## Solution 2: React Bridge Wrapper
|
||||
|
||||
### Overview
|
||||
|
||||
Create a React-specific hook that wraps EventDispatcher subscriptions.
|
||||
|
||||
### Implementation
|
||||
|
||||
```typescript
|
||||
// hooks/useEventListener.ts
|
||||
export function useEventListener<T = any>(
|
||||
dispatcher: EventDispatcher,
|
||||
eventName: string,
|
||||
callback: (data?: T) => void
|
||||
) {
|
||||
const callbackRef = useRef(callback);
|
||||
|
||||
// Update ref on every render (avoid stale closures)
|
||||
useEffect(() => {
|
||||
callbackRef.current = callback;
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
// Wrapper that calls current ref
|
||||
const wrapper = (data?: T) => {
|
||||
callbackRef.current(data);
|
||||
};
|
||||
|
||||
// Create stable context object
|
||||
const context = { id: Math.random() };
|
||||
|
||||
dispatcher.on(eventName, wrapper, context);
|
||||
|
||||
return () => {
|
||||
dispatcher.off(eventName, context);
|
||||
};
|
||||
}, [dispatcher, eventName]);
|
||||
}
|
||||
```
|
||||
|
||||
### Usage
|
||||
|
||||
```typescript
|
||||
function ComponentsPanel() {
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', () => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
|
||||
// ... rest of component
|
||||
}
|
||||
```
|
||||
|
||||
### Pros
|
||||
|
||||
- ✅ Clean React API
|
||||
- ✅ No changes to EventDispatcher
|
||||
- ✅ Reusable across all React components
|
||||
- ✅ Handles closure issues with useRef pattern
|
||||
|
||||
### Cons
|
||||
|
||||
- ❌ Still uses legacy EventDispatcher internally
|
||||
- ❌ Adds indirection
|
||||
- ❌ Doesn't fix the root cause
|
||||
|
||||
### Effort
|
||||
|
||||
**Estimated**: 2-4 hours
|
||||
|
||||
- 1 hour: Implement hook
|
||||
- 1 hour: Test thoroughly
|
||||
- 1 hour: Update existing React components
|
||||
- 1 hour: Documentation
|
||||
|
||||
---
|
||||
|
||||
## Solution 3: Migrate to Modern State Management
|
||||
|
||||
### Overview
|
||||
|
||||
Replace EventDispatcher with a modern React state management solution.
|
||||
|
||||
### Option 3A: React Context + useReducer
|
||||
|
||||
```typescript
|
||||
// contexts/ProjectContext.tsx
|
||||
interface ProjectState {
|
||||
components: Component[];
|
||||
folders: Folder[];
|
||||
version: number; // Increment on any change
|
||||
}
|
||||
|
||||
const ProjectContext = createContext<{
|
||||
state: ProjectState;
|
||||
actions: {
|
||||
renameComponent: (id: string, name: string) => void;
|
||||
addComponent: (component: Component) => void;
|
||||
removeComponent: (id: string) => void;
|
||||
};
|
||||
}>(null!);
|
||||
|
||||
export function ProjectProvider({ children }: { children: React.ReactNode }) {
|
||||
const [state, dispatch] = useReducer(projectReducer, initialState);
|
||||
|
||||
const actions = useMemo(
|
||||
() => ({
|
||||
renameComponent: (id: string, name: string) => {
|
||||
dispatch({ type: 'RENAME_COMPONENT', id, name });
|
||||
ProjectModel.instance.renameComponent(id, name); // Sync with legacy
|
||||
}
|
||||
// ... other actions
|
||||
}),
|
||||
[]
|
||||
);
|
||||
|
||||
return <ProjectContext.Provider value={{ state, actions }}>{children}</ProjectContext.Provider>;
|
||||
}
|
||||
|
||||
export function useProject() {
|
||||
return useContext(ProjectContext);
|
||||
}
|
||||
```
|
||||
|
||||
### Option 3B: Zustand
|
||||
|
||||
```typescript
|
||||
// stores/projectStore.ts
|
||||
import create from 'zustand';
|
||||
|
||||
interface ProjectStore {
|
||||
components: Component[];
|
||||
folders: Folder[];
|
||||
|
||||
renameComponent: (id: string, name: string) => void;
|
||||
addComponent: (component: Component) => void;
|
||||
removeComponent: (id: string) => void;
|
||||
}
|
||||
|
||||
export const useProjectStore = create<ProjectStore>((set) => ({
|
||||
components: [],
|
||||
folders: [],
|
||||
|
||||
renameComponent: (id, name) => {
|
||||
set((state) => ({
|
||||
components: state.components.map((c) => (c.id === id ? { ...c, name } : c))
|
||||
}));
|
||||
ProjectModel.instance.renameComponent(id, name); // Sync with legacy
|
||||
}
|
||||
|
||||
// ... other actions
|
||||
}));
|
||||
```
|
||||
|
||||
### Option 3C: Redux Toolkit
|
||||
|
||||
```typescript
|
||||
// slices/projectSlice.ts
|
||||
import { createSlice } from '@reduxjs/toolkit';
|
||||
|
||||
const projectSlice = createSlice({
|
||||
name: 'project',
|
||||
initialState: {
|
||||
components: [],
|
||||
folders: []
|
||||
},
|
||||
reducers: {
|
||||
renameComponent: (state, action) => {
|
||||
const component = state.components.find((c) => c.id === action.payload.id);
|
||||
if (component) {
|
||||
component.name = action.payload.name;
|
||||
}
|
||||
}
|
||||
// ... other actions
|
||||
}
|
||||
});
|
||||
|
||||
export const { renameComponent } = projectSlice.actions;
|
||||
```
|
||||
|
||||
### Pros
|
||||
|
||||
- ✅ Modern, React-native solution
|
||||
- ✅ Better developer experience
|
||||
- ✅ Time travel debugging (Redux DevTools)
|
||||
- ✅ Predictable state updates
|
||||
- ✅ Scales well for complex state
|
||||
|
||||
### Cons
|
||||
|
||||
- ❌ Major architectural change
|
||||
- ❌ Need to sync with legacy ProjectModel
|
||||
- ❌ High migration effort
|
||||
- ❌ All React components need updating
|
||||
- ❌ Risk of state inconsistencies during transition
|
||||
|
||||
### Effort
|
||||
|
||||
**Estimated**: 2-4 weeks
|
||||
|
||||
- Week 1: Set up state management, create stores
|
||||
- Week 1-2: Implement sync layer with legacy models
|
||||
- Week 2-3: Migrate all React components
|
||||
- Week 3-4: Testing and bug fixes
|
||||
|
||||
---
|
||||
|
||||
## Solution 4: Proxy-based Reactive System
|
||||
|
||||
### Overview
|
||||
|
||||
Create a reactive wrapper around ProjectModel that React can subscribe to.
|
||||
|
||||
### Implementation
|
||||
|
||||
```typescript
|
||||
// utils/createReactiveModel.ts
|
||||
import { useSyncExternalStore } from 'react';
|
||||
|
||||
export function createReactiveModel<T extends EventDispatcher>(model: T) {
|
||||
const subscribers = new Set<() => void>();
|
||||
let version = 0;
|
||||
|
||||
// Listen to ALL events from the model
|
||||
const eventProxy = new Proxy(model, {
|
||||
get(target, prop) {
|
||||
const value = target[prop];
|
||||
|
||||
if (prop === 'notifyListeners') {
|
||||
return (...args: any[]) => {
|
||||
// Call original
|
||||
value.apply(target, args);
|
||||
|
||||
// Notify React subscribers
|
||||
version++;
|
||||
subscribers.forEach((callback) => callback());
|
||||
};
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
});
|
||||
|
||||
return {
|
||||
model: eventProxy,
|
||||
subscribe: (callback: () => void) => {
|
||||
subscribers.add(callback);
|
||||
return () => subscribers.delete(callback);
|
||||
},
|
||||
getSnapshot: () => version
|
||||
};
|
||||
}
|
||||
|
||||
// Usage hook
|
||||
export function useModelChanges(reactiveModel: ReturnType<typeof createReactiveModel>) {
|
||||
return useSyncExternalStore(reactiveModel.subscribe, reactiveModel.getSnapshot, reactiveModel.getSnapshot);
|
||||
}
|
||||
```
|
||||
|
||||
### Usage
|
||||
|
||||
```typescript
|
||||
// Create reactive wrapper once
|
||||
const reactiveProject = createReactiveModel(ProjectModel.instance);
|
||||
|
||||
// In component
|
||||
function ComponentsPanel() {
|
||||
const version = useModelChanges(reactiveProject);
|
||||
|
||||
const treeData = useMemo(() => {
|
||||
return buildTree(reactiveProject.model);
|
||||
}, [version]);
|
||||
|
||||
// ... rest of component
|
||||
}
|
||||
```
|
||||
|
||||
### Pros
|
||||
|
||||
- ✅ Uses React 18's built-in external store API
|
||||
- ✅ No changes to EventDispatcher or ProjectModel
|
||||
- ✅ Automatic subscription management
|
||||
- ✅ Works with any EventDispatcher-based model
|
||||
|
||||
### Cons
|
||||
|
||||
- ❌ Proxy overhead
|
||||
- ❌ All events trigger re-render (no granularity)
|
||||
- ❌ Requires React 18+
|
||||
- ❌ Complex debugging
|
||||
|
||||
### Effort
|
||||
|
||||
**Estimated**: 1-2 days
|
||||
|
||||
- 4 hours: Implement reactive wrapper
|
||||
- 4 hours: Test with multiple models
|
||||
- 4 hours: Update React components
|
||||
- 4 hours: Documentation and examples
|
||||
|
||||
---
|
||||
|
||||
## Solution 5: Manual Callbacks (Current Workaround)
|
||||
|
||||
### Overview
|
||||
|
||||
Continue using manual refresh callbacks as implemented in Task 004B.
|
||||
|
||||
### Pattern
|
||||
|
||||
```typescript
|
||||
// Hook provides forceRefresh
|
||||
const forceRefresh = useCallback(() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
|
||||
// Actions accept onSuccess callback
|
||||
function performAction(data: any, onSuccess?: () => void) {
|
||||
// ... do work ...
|
||||
if (success && onSuccess) {
|
||||
onSuccess();
|
||||
}
|
||||
}
|
||||
|
||||
// Component wires them together
|
||||
performAction(data, () => {
|
||||
forceRefresh();
|
||||
});
|
||||
```
|
||||
|
||||
### Pros
|
||||
|
||||
- ✅ Already implemented and working
|
||||
- ✅ Zero architectural changes
|
||||
- ✅ Simple to understand
|
||||
- ✅ Explicit control over refreshes
|
||||
|
||||
### Cons
|
||||
|
||||
- ❌ Tech debt accumulates
|
||||
- ❌ Easy to forget callback in new code paths
|
||||
- ❌ Not scalable for complex event chains
|
||||
- ❌ Loses reactive benefits
|
||||
|
||||
### Effort
|
||||
|
||||
**Estimated**: Already done
|
||||
|
||||
- No additional work needed
|
||||
- Just document the pattern
|
||||
|
||||
---
|
||||
|
||||
## Recommendation
|
||||
|
||||
### Short-term (0-1 month): Solution 2 - React Bridge Wrapper
|
||||
|
||||
Implement `useEventListener` hook to provide clean API for existing event subscriptions.
|
||||
|
||||
**Why**:
|
||||
|
||||
- Low effort, high value
|
||||
- Fixes immediate problem
|
||||
- Doesn't block future migrations
|
||||
- Can coexist with manual callbacks
|
||||
|
||||
### Medium-term (1-3 months): Solution 4 - Proxy-based Reactive System
|
||||
|
||||
Implement reactive model wrappers using `useSyncExternalStore`.
|
||||
|
||||
**Why**:
|
||||
|
||||
- Uses modern React patterns
|
||||
- Minimal changes to existing code
|
||||
- Works with legacy models
|
||||
- Provides automatic reactivity
|
||||
|
||||
### Long-term (3-6 months): Solution 3 - Modern State Management
|
||||
|
||||
Gradually migrate to Zustand or Redux Toolkit.
|
||||
|
||||
**Why**:
|
||||
|
||||
- Best developer experience
|
||||
- Scales well
|
||||
- Standard patterns
|
||||
- Better tooling
|
||||
|
||||
### Migration Path
|
||||
|
||||
1. **Phase 1** (Week 1-2):
|
||||
- Implement `useEventListener` hook
|
||||
- Update ComponentsPanel to use it
|
||||
- Document pattern
|
||||
2. **Phase 2** (Month 2):
|
||||
- Implement reactive model system
|
||||
- Test with multiple components
|
||||
- Roll out gradually
|
||||
3. **Phase 3** (Month 3-6):
|
||||
- Choose state management library
|
||||
- Create stores for major models
|
||||
- Migrate components one by one
|
||||
- Maintain backward compatibility
|
||||
|
||||
---
|
||||
|
||||
## Decision Criteria
|
||||
|
||||
Choose solution based on:
|
||||
|
||||
1. **Timeline**: How urgently do we need React components?
|
||||
2. **Scope**: How many Views are we migrating to React?
|
||||
3. **Resources**: How much dev time is available?
|
||||
4. **Risk tolerance**: Can we handle breaking changes?
|
||||
5. **Long-term vision**: Are we fully moving to React?
|
||||
|
||||
**If migrating many Views**: Invest in Solution 3 (state management)
|
||||
**If only a few React components**: Use Solution 2 (bridge wrapper)
|
||||
**If unsure**: Start with Solution 2, migrate to Solution 3 later
|
||||
|
||||
---
|
||||
|
||||
## Questions to Answer
|
||||
|
||||
Before deciding on a solution:
|
||||
|
||||
1. How many jQuery Views are planned to migrate to React?
|
||||
2. What's the timeline for full React migration?
|
||||
3. Are there performance concerns with current EventDispatcher?
|
||||
4. What state management libraries are already in the codebase?
|
||||
5. Is there team expertise with modern state management?
|
||||
6. What's the testing infrastructure like?
|
||||
7. Can we afford breaking changes during transition?
|
||||
|
||||
---
|
||||
|
||||
## Next Actions
|
||||
|
||||
1. ✅ Complete this investigation documentation
|
||||
2. ⬜ Present options to team
|
||||
3. ⬜ Decide on solution approach
|
||||
4. ⬜ Create implementation task
|
||||
5. ⬜ Test POC with ComponentsPanel
|
||||
6. ⬜ Roll out to other components
|
||||
@@ -0,0 +1,235 @@
|
||||
# TASK-008: EventDispatcher + React Hooks Investigation
|
||||
|
||||
## Status: 🟡 Investigation Needed
|
||||
|
||||
**Created**: 2025-12-22
|
||||
**Priority**: Medium
|
||||
**Complexity**: High
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
During Task 004B (ComponentsPanel React Migration), we discovered that the legacy EventDispatcher pub/sub pattern does not work with React hooks. Events are emitted by legacy models but never received by React components subscribed in `useEffect`. This investigation task aims to understand the root cause and propose long-term solutions.
|
||||
|
||||
---
|
||||
|
||||
## Problem Statement
|
||||
|
||||
### What's Broken
|
||||
|
||||
When a React component subscribes to ProjectModel events using the EventDispatcher pattern:
|
||||
|
||||
```typescript
|
||||
// In useComponentsPanel.ts
|
||||
useEffect(() => {
|
||||
const handleUpdate = (eventName: string) => {
|
||||
console.log('🔔 Event received:', eventName);
|
||||
setUpdateCounter((c) => c + 1);
|
||||
};
|
||||
|
||||
const listener = { handleUpdate };
|
||||
|
||||
ProjectModel.instance.on('componentAdded', () => handleUpdate('componentAdded'), listener);
|
||||
ProjectModel.instance.on('componentRemoved', () => handleUpdate('componentRemoved'), listener);
|
||||
ProjectModel.instance.on('componentRenamed', () => handleUpdate('componentRenamed'), listener);
|
||||
|
||||
return () => {
|
||||
ProjectModel.instance.off('componentAdded', listener);
|
||||
ProjectModel.instance.off('componentRemoved', listener);
|
||||
ProjectModel.instance.off('componentRenamed', listener);
|
||||
};
|
||||
}, []);
|
||||
```
|
||||
|
||||
**Expected behavior**: When `ProjectModel.renameComponent()` is called, it emits 'componentRenamed' event, and the React hook receives it.
|
||||
|
||||
**Actual behavior**:
|
||||
|
||||
- ProjectModel.renameComponent() DOES emit the event (verified with logs)
|
||||
- The subscription code runs without errors
|
||||
- BUT: The event handler is NEVER called
|
||||
- No console logs, no state updates, complete silence
|
||||
|
||||
### Current Workaround
|
||||
|
||||
Manual refresh callback pattern (see NOTES.md for details):
|
||||
|
||||
1. Hook provides a `forceRefresh()` function that increments a counter
|
||||
2. Action handlers accept an `onSuccess` callback parameter
|
||||
3. Component passes `forceRefresh` as the callback
|
||||
4. Successful actions call `onSuccess()` to trigger manual refresh
|
||||
|
||||
**Problem with workaround**:
|
||||
|
||||
- Creates tech debt
|
||||
- Must remember to call `onSuccess()` in ALL code paths
|
||||
- Doesn't scale to complex event chains
|
||||
- Loses the benefits of reactive event-driven architecture
|
||||
|
||||
---
|
||||
|
||||
## Investigation Goals
|
||||
|
||||
### Primary Questions
|
||||
|
||||
1. **Why doesn't EventDispatcher work with React hooks?**
|
||||
|
||||
- Is it a closure issue?
|
||||
- Is it a timing issue?
|
||||
- Is it the context object pattern?
|
||||
- Is it React's StrictMode double-invocation?
|
||||
|
||||
2. **What is the scope of the problem?**
|
||||
|
||||
- Does it affect ALL React components?
|
||||
- Does it work in class components?
|
||||
- Does it work in legacy jQuery Views?
|
||||
- Are there any React components successfully using EventDispatcher?
|
||||
|
||||
3. **Is EventDispatcher fundamentally incompatible with React?**
|
||||
- Or can it be fixed?
|
||||
- What would need to change?
|
||||
|
||||
### Secondary Questions
|
||||
|
||||
4. **What are the migration implications?**
|
||||
|
||||
- How many places use EventDispatcher?
|
||||
- How many are already React components?
|
||||
- How hard would migration be?
|
||||
|
||||
5. **What is the best long-term solution?**
|
||||
- Fix EventDispatcher?
|
||||
- Replace with modern state management?
|
||||
- Create a React bridge?
|
||||
|
||||
---
|
||||
|
||||
## Hypotheses
|
||||
|
||||
### Hypothesis 1: Context Object Reference Mismatch
|
||||
|
||||
EventDispatcher uses a context object for listener cleanup:
|
||||
|
||||
```typescript
|
||||
model.on('event', handler, contextObject);
|
||||
// Later:
|
||||
model.off('event', contextObject); // Must be same object reference
|
||||
```
|
||||
|
||||
React's useEffect cleanup may run in a different closure, causing the context object reference to not match, preventing proper cleanup and potentially blocking event delivery.
|
||||
|
||||
**How to test**: Try without context object, or use a stable ref.
|
||||
|
||||
### Hypothesis 2: Stale Closure
|
||||
|
||||
The handler function captures variables from the initial render. When the event fires later, those captured variables are stale, causing issues.
|
||||
|
||||
**How to test**: Use `useRef` to store the handler, update ref on every render.
|
||||
|
||||
### Hypothesis 3: Event Emission Timing
|
||||
|
||||
Events might be emitted before React components are ready to receive them, or during React's render phase when state updates are not allowed.
|
||||
|
||||
**How to test**: Add extensive timing logs, check React's render phase detection.
|
||||
|
||||
### Hypothesis 4: EventDispatcher Implementation Bug
|
||||
|
||||
The EventDispatcher itself may have issues with how it stores/invokes listeners, especially when mixed with React's lifecycle.
|
||||
|
||||
**How to test**: Deep dive into EventDispatcher.ts, add comprehensive logging.
|
||||
|
||||
---
|
||||
|
||||
## Test Plan
|
||||
|
||||
### Phase 1: Reproduce Minimal Case
|
||||
|
||||
Create the simplest possible reproduction:
|
||||
|
||||
1. Minimal EventDispatcher instance
|
||||
2. Minimal React component with useEffect
|
||||
3. Single event emission
|
||||
4. Comprehensive logging at every step
|
||||
|
||||
### Phase 2: Comparative Testing
|
||||
|
||||
Test in different scenarios:
|
||||
|
||||
- React functional component with useEffect
|
||||
- React class component with componentDidMount
|
||||
- Legacy jQuery View
|
||||
- React StrictMode on/off
|
||||
- Development vs production build
|
||||
|
||||
### Phase 3: EventDispatcher Deep Dive
|
||||
|
||||
Examine EventDispatcher implementation:
|
||||
|
||||
- How are listeners stored?
|
||||
- How are events emitted?
|
||||
- How does context object matching work?
|
||||
- Any special handling needed?
|
||||
|
||||
### Phase 4: Solution Prototyping
|
||||
|
||||
Test potential fixes:
|
||||
|
||||
- EventDispatcher modifications
|
||||
- React bridge wrapper
|
||||
- Migration to alternative patterns
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria
|
||||
|
||||
This investigation is complete when we have:
|
||||
|
||||
1. ✅ Clear understanding of WHY events don't reach React hooks
|
||||
2. ✅ Documented root cause with evidence
|
||||
3. ✅ Evaluation of all potential solutions
|
||||
4. ✅ Recommendation for long-term fix
|
||||
5. ✅ Proof-of-concept implementation (if feasible)
|
||||
6. ✅ Migration plan (if solution requires changes)
|
||||
|
||||
---
|
||||
|
||||
## Affected Areas
|
||||
|
||||
### Current Known Issues
|
||||
|
||||
- ✅ **ComponentsPanel**: Uses workaround (Task 004B)
|
||||
|
||||
### Potential Future Issues
|
||||
|
||||
Any React component that needs to:
|
||||
|
||||
- Subscribe to ProjectModel events
|
||||
- Subscribe to NodeGraphModel events
|
||||
- Subscribe to any EventDispatcher-based model
|
||||
- React to data changes from legacy systems
|
||||
|
||||
### Estimated Impact
|
||||
|
||||
- **High**: If we continue migrating jQuery Views to React
|
||||
- **Medium**: If we keep jQuery Views and only use React for new features
|
||||
- **Low**: If we migrate away from EventDispatcher entirely
|
||||
|
||||
---
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- [LEARNINGS.md](../../../reference/LEARNINGS.md#2025-12-22---eventdispatcher-events-dont-reach-react-hooks)
|
||||
- [Task 004B Phase 5](../TASK-004B-componentsPanel-react-migration/phases/PHASE-5-INLINE-RENAME.md)
|
||||
- EventDispatcher implementation: `packages/noodl-editor/src/editor/src/shared/utils/EventDispatcher.ts`
|
||||
- Example workaround: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentActions.ts`
|
||||
|
||||
---
|
||||
|
||||
## Timeline
|
||||
|
||||
**Status**: Not started
|
||||
**Estimated effort**: 1-2 days investigation + 2-4 days implementation (depending on solution)
|
||||
**Blocking**: No other tasks currently blocked
|
||||
**Priority**: Should be completed before migrating more Views to React
|
||||
@@ -0,0 +1,344 @@
|
||||
# useEventListener Hook - Usage Guide
|
||||
|
||||
## Overview
|
||||
|
||||
The `useEventListener` hook provides a React-friendly way to subscribe to EventDispatcher events. It solves the fundamental incompatibility between EventDispatcher's context-object-based cleanup and React's closure-based lifecycle.
|
||||
|
||||
## Location
|
||||
|
||||
```typescript
|
||||
import { useEventListener } from '@noodl-hooks/useEventListener';
|
||||
```
|
||||
|
||||
**File**: `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts`
|
||||
|
||||
---
|
||||
|
||||
## Basic Usage
|
||||
|
||||
### Single Event
|
||||
|
||||
```typescript
|
||||
import { ProjectModel } from '@noodl-models/projectmodel';
|
||||
|
||||
import { useEventListener } from '../../../../hooks/useEventListener';
|
||||
|
||||
function MyComponent() {
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
// Subscribe to a single event
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', () => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
|
||||
return <div>Components updated {updateCounter} times</div>;
|
||||
}
|
||||
```
|
||||
|
||||
### Multiple Events
|
||||
|
||||
```typescript
|
||||
// Subscribe to multiple events with one subscription
|
||||
useEventListener(ProjectModel.instance, ['componentAdded', 'componentRemoved', 'componentRenamed'], () => {
|
||||
console.log('Component changed');
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
```
|
||||
|
||||
### With Event Data
|
||||
|
||||
```typescript
|
||||
interface RenameData {
|
||||
component: ComponentModel;
|
||||
oldName: string;
|
||||
newName: string;
|
||||
}
|
||||
|
||||
useEventListener<RenameData>(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
console.log(`Renamed from ${data.oldName} to ${data.newName}`);
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Advanced Usage
|
||||
|
||||
### Conditional Subscription
|
||||
|
||||
Use the optional `deps` parameter to control when the subscription is active:
|
||||
|
||||
```typescript
|
||||
const [isActive, setIsActive] = useState(true);
|
||||
|
||||
useEventListener(
|
||||
isActive ? ProjectModel.instance : null, // Pass null to disable
|
||||
'componentRenamed',
|
||||
() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}
|
||||
);
|
||||
```
|
||||
|
||||
### With Dependencies
|
||||
|
||||
Re-subscribe when dependencies change:
|
||||
|
||||
```typescript
|
||||
const [filter, setFilter] = useState('all');
|
||||
|
||||
useEventListener(
|
||||
ProjectModel.instance,
|
||||
'componentAdded',
|
||||
(data) => {
|
||||
// Callback uses current filter value
|
||||
if (shouldShowComponent(data.component, filter)) {
|
||||
addToList(data.component);
|
||||
}
|
||||
},
|
||||
[filter] // Re-subscribe when filter changes
|
||||
);
|
||||
```
|
||||
|
||||
### Multiple Dispatchers
|
||||
|
||||
```typescript
|
||||
function MyComponent() {
|
||||
// Subscribe to ProjectModel events
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', handleProjectUpdate);
|
||||
|
||||
// Subscribe to WarningsModel events
|
||||
useEventListener(WarningsModel.instance, 'warningsChanged', handleWarningsUpdate);
|
||||
|
||||
// Subscribe to EventDispatcher singleton
|
||||
useEventListener(EventDispatcher.instance, 'viewer-refresh', handleViewerRefresh);
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Pattern 1: Trigger Re-render on Model Changes
|
||||
|
||||
```typescript
|
||||
function useComponentsPanel() {
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
// Re-render whenever components change
|
||||
useEventListener(ProjectModel.instance, ['componentAdded', 'componentRemoved', 'componentRenamed'], () =>
|
||||
setUpdateCounter((c) => c + 1)
|
||||
);
|
||||
|
||||
// This will re-compute whenever updateCounter changes
|
||||
const treeData = useMemo(() => {
|
||||
return buildTreeFromProject(ProjectModel.instance);
|
||||
}, [updateCounter]);
|
||||
|
||||
return { treeData };
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 2: Update Local State from Events
|
||||
|
||||
```typescript
|
||||
function WarningsPanel() {
|
||||
const [warnings, setWarnings] = useState([]);
|
||||
|
||||
useEventListener(WarningsModel.instance, 'warningsChanged', () => {
|
||||
setWarnings(WarningsModel.instance.getWarnings());
|
||||
});
|
||||
|
||||
return (
|
||||
<div>
|
||||
{warnings.map((warning) => (
|
||||
<WarningItem key={warning.id} warning={warning} />
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 3: Side Effects on Events
|
||||
|
||||
```typescript
|
||||
function AutoSaver() {
|
||||
const saveTimeoutRef = useRef<NodeJS.Timeout>();
|
||||
|
||||
useEventListener(ProjectModel.instance, ['componentAdded', 'componentRemoved', 'componentRenamed'], () => {
|
||||
// Debounce saves
|
||||
clearTimeout(saveTimeoutRef.current);
|
||||
saveTimeoutRef.current = setTimeout(() => {
|
||||
ProjectModel.instance.save();
|
||||
}, 1000);
|
||||
});
|
||||
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Migration from Manual Subscriptions
|
||||
|
||||
### Before (Broken)
|
||||
|
||||
```typescript
|
||||
❌ // This doesn't work!
|
||||
useEffect(() => {
|
||||
const listener = { handleUpdate };
|
||||
ProjectModel.instance.on('componentRenamed', () => handleUpdate(), listener);
|
||||
return () => ProjectModel.instance.off(listener);
|
||||
}, []);
|
||||
```
|
||||
|
||||
### After (Working)
|
||||
|
||||
```typescript
|
||||
✅ // This works perfectly!
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', () => {
|
||||
handleUpdate();
|
||||
});
|
||||
```
|
||||
|
||||
### Before (Workaround)
|
||||
|
||||
```typescript
|
||||
❌ // Manual callback workaround
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
const forceRefresh = useCallback(() => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
}, []);
|
||||
|
||||
const performAction = (data, onSuccess) => {
|
||||
// ... do action ...
|
||||
if (onSuccess) onSuccess(); // Manual refresh
|
||||
};
|
||||
|
||||
// In component:
|
||||
performAction(data, () => forceRefresh());
|
||||
```
|
||||
|
||||
### After (Clean)
|
||||
|
||||
```typescript
|
||||
✅ // Automatic event handling
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
useEventListener(ProjectModel.instance, 'actionCompleted', () => {
|
||||
setUpdateCounter((c) => c + 1);
|
||||
});
|
||||
|
||||
const performAction = (data) => {
|
||||
// ... do action ...
|
||||
// Event fires automatically, no callbacks needed!
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Type Safety
|
||||
|
||||
The hook is fully typed and works with TypeScript:
|
||||
|
||||
```typescript
|
||||
interface ComponentData {
|
||||
component: ComponentModel;
|
||||
oldName?: string;
|
||||
newName?: string;
|
||||
}
|
||||
|
||||
// Type the event data
|
||||
useEventListener<ComponentData>(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
// data is typed as ComponentData | undefined
|
||||
if (data) {
|
||||
console.log(data.component.name); // ✅ TypeScript knows this is safe
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Supported Dispatchers
|
||||
|
||||
The hook works with any object that implements the `IEventEmitter` interface:
|
||||
|
||||
- ✅ `EventDispatcher` (and `EventDispatcher.instance`)
|
||||
- ✅ `Model` subclasses (ProjectModel, WarningsModel, etc.)
|
||||
- ✅ Any class with `on(event, listener, group)` and `off(group)` methods
|
||||
|
||||
---
|
||||
|
||||
## Best Practices
|
||||
|
||||
### ✅ DO:
|
||||
|
||||
- Use `useEventListener` for all EventDispatcher subscriptions in React components
|
||||
- Pass `null` as dispatcher if you want to conditionally disable subscriptions
|
||||
- Use the optional `deps` array when your callback depends on props/state
|
||||
- Type your event data with the generic parameter for better IDE support
|
||||
|
||||
### ❌ DON'T:
|
||||
|
||||
- Don't try to use manual `on()`/`off()` subscriptions in React - they won't work
|
||||
- Don't forget to handle `null` dispatchers if using conditional subscriptions
|
||||
- Don't create new objects in the deps array - they'll cause infinite re-subscriptions
|
||||
- Don't call `setState` directly inside event handlers without checking if component is mounted
|
||||
|
||||
---
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### Events Not Firing
|
||||
|
||||
**Problem**: Event subscription seems to work, but callback never fires.
|
||||
|
||||
**Solution**: Make sure you're using `useEventListener` instead of manual `on()`/`off()` calls.
|
||||
|
||||
### Stale Closure Issues
|
||||
|
||||
**Problem**: Callback uses old values of props/state.
|
||||
|
||||
**Solution**: The hook already handles this with `useRef`. If you still see issues, add dependencies to the `deps` array.
|
||||
|
||||
### Memory Leaks
|
||||
|
||||
**Problem**: Component unmounts but subscriptions remain.
|
||||
|
||||
**Solution**: The hook handles cleanup automatically. Make sure you're not holding references to the callback elsewhere.
|
||||
|
||||
### TypeScript Errors
|
||||
|
||||
**Problem**: "Type X is not assignable to EventDispatcher"
|
||||
|
||||
**Solution**: The hook accepts any `IEventEmitter`. Your model might need to properly extend `EventDispatcher` or `Model`.
|
||||
|
||||
---
|
||||
|
||||
## Examples in Codebase
|
||||
|
||||
See these files for real-world usage examples:
|
||||
|
||||
- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts`
|
||||
- (More examples as other components are migrated)
|
||||
|
||||
---
|
||||
|
||||
## Future Improvements
|
||||
|
||||
Potential enhancements for the future:
|
||||
|
||||
1. **Selective Re-rendering**: Only re-render when specific event data changes
|
||||
2. **Event Filtering**: Built-in support for conditional event handling
|
||||
3. **Debouncing**: Optional built-in debouncing for high-frequency events
|
||||
4. **Event History**: Debug mode that tracks all received events
|
||||
|
||||
---
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- [TASK-008 README](./README.md) - Investigation overview
|
||||
- [CHANGELOG](./CHANGELOG.md) - Implementation details
|
||||
- [NOTES](./NOTES.md) - Discovery process
|
||||
- [LEARNINGS.md](../../../reference/LEARNINGS.md) - Lessons learned
|
||||
@@ -0,0 +1,68 @@
|
||||
# TASK-009 Verification Checklist
|
||||
|
||||
## Pre-Verification
|
||||
|
||||
- [x] `npm run clean:all` script exists
|
||||
- [x] Script successfully clears caches
|
||||
- [x] Babel cache disabled in webpack config
|
||||
- [x] Build timestamp canary added to entry point
|
||||
|
||||
## User Verification Required
|
||||
|
||||
### Test 1: Fresh Build
|
||||
|
||||
- [ ] Run `npm run clean:all`
|
||||
- [ ] Run `npm run dev`
|
||||
- [ ] Wait for Electron to launch
|
||||
- [ ] Open DevTools Console (View → Toggle Developer Tools)
|
||||
- [ ] Verify timestamp appears: `🔥 BUILD TIMESTAMP: [recent time]`
|
||||
- [ ] Note the timestamp: \_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
|
||||
|
||||
### Test 2: Code Change Detection
|
||||
|
||||
- [ ] Open `packages/noodl-editor/src/editor/index.ts`
|
||||
- [ ] Change the build canary line to add extra emoji:
|
||||
```typescript
|
||||
console.log('🔥🔥 BUILD TIMESTAMP:', new Date().toISOString());
|
||||
```
|
||||
- [ ] Save the file
|
||||
- [ ] Wait 5 seconds for webpack to recompile
|
||||
- [ ] Reload Electron app (Cmd+R on macOS, Ctrl+R on Windows/Linux)
|
||||
- [ ] Check console - timestamp should update and show two fire emojis
|
||||
- [ ] Note new timestamp: \_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
|
||||
- [ ] Timestamps should be different (proves fresh code loaded)
|
||||
|
||||
### Test 3: Repeat to Ensure Reliability
|
||||
|
||||
- [ ] Make another trivial change (e.g., add 🔥🔥🔥)
|
||||
- [ ] Save, wait, reload
|
||||
- [ ] Verify timestamp updates again
|
||||
- [ ] Note timestamp: \_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
|
||||
|
||||
### Test 4: Revert and Confirm
|
||||
|
||||
- [ ] Revert changes (remove extra emojis, keep just one 🔥)
|
||||
- [ ] Save, wait, reload
|
||||
- [ ] Verify timestamp updates
|
||||
- [ ] Build canary back to original
|
||||
|
||||
## Definition of Done
|
||||
|
||||
All checkboxes above should be checked. If any test fails:
|
||||
|
||||
1. Run `npm run clean:all` again
|
||||
2. Manually clear Electron cache: `~/Library/Application Support/Noodl/Code Cache/`
|
||||
3. Restart from Test 1
|
||||
|
||||
## Success Criteria
|
||||
|
||||
✅ Changes appear within 5 seconds, 3 times in a row
|
||||
✅ Build timestamp updates every time code changes
|
||||
✅ No stale code issues
|
||||
|
||||
## If Problems Persist
|
||||
|
||||
1. Check if webpack dev server is running properly
|
||||
2. Look for webpack compilation errors in terminal
|
||||
3. Verify no other Electron/Node processes are running: `pkill -f Electron; pkill -f node`
|
||||
4. Try a full restart of the dev server
|
||||
@@ -0,0 +1,99 @@
|
||||
# TASK-009: Webpack Cache Elimination
|
||||
|
||||
## Status: ✅ COMPLETED
|
||||
|
||||
## Summary
|
||||
|
||||
Fixed aggressive webpack caching that was preventing code changes from loading even after restarts.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### 1. Created `clean:all` Script ✅
|
||||
|
||||
**File:** `package.json`
|
||||
|
||||
Added script to clear all cache locations:
|
||||
|
||||
```json
|
||||
"clean:all": "rimraf node_modules/.cache packages/*/node_modules/.cache .eslintcache packages/*/.eslintcache && echo '✓ All caches cleared. On macOS, Electron cache at ~/Library/Application Support/Noodl/ should be manually cleared if issues persist.'"
|
||||
```
|
||||
|
||||
**Cache locations cleared:**
|
||||
|
||||
- `node_modules/.cache`
|
||||
- `packages/*/node_modules/.cache` (3 locations found)
|
||||
- `.eslintcache` files
|
||||
- Electron cache: `~/Library/Application Support/Noodl/` (manual)
|
||||
|
||||
### 2. Disabled Babel Cache in Development ✅
|
||||
|
||||
**File:** `packages/noodl-editor/webpackconfigs/shared/webpack.renderer.core.js`
|
||||
|
||||
Changed:
|
||||
|
||||
```javascript
|
||||
cacheDirectory: true; // OLD
|
||||
cacheDirectory: false; // NEW - ensures fresh code loads
|
||||
```
|
||||
|
||||
### 3. Added Build Canary Timestamp ✅
|
||||
|
||||
**File:** `packages/noodl-editor/src/editor/index.ts`
|
||||
|
||||
Added after imports:
|
||||
|
||||
```typescript
|
||||
// Build canary: Verify fresh code is loading
|
||||
console.log('🔥 BUILD TIMESTAMP:', new Date().toISOString());
|
||||
```
|
||||
|
||||
This timestamp logs when the editor loads, allowing verification that fresh code is running.
|
||||
|
||||
## Verification Steps
|
||||
|
||||
To verify TASK-009 is working:
|
||||
|
||||
1. **Run clean script:**
|
||||
|
||||
```bash
|
||||
npm run clean:all
|
||||
```
|
||||
|
||||
2. **Start the dev server:**
|
||||
|
||||
```bash
|
||||
npm run dev
|
||||
```
|
||||
|
||||
3. **Check for build timestamp** in Electron console:
|
||||
|
||||
```
|
||||
🔥 BUILD TIMESTAMP: 2025-12-23T09:26:00.000Z
|
||||
```
|
||||
|
||||
4. **Make a trivial change** to any editor file
|
||||
|
||||
5. **Save the file** and wait 5 seconds
|
||||
|
||||
6. **Refresh/Reload** the Electron app (Cmd+R on macOS)
|
||||
|
||||
7. **Verify the timestamp updated** - this proves fresh code loaded
|
||||
|
||||
8. **Repeat 2 more times** to ensure reliability
|
||||
|
||||
## Definition of Done
|
||||
|
||||
- [x] `npm run clean:all` works
|
||||
- [x] Babel cache disabled in dev mode
|
||||
- [x] Build timestamp canary visible in console
|
||||
- [ ] Code changes verified loading reliably (3x) - **User to verify**
|
||||
|
||||
## Next Steps
|
||||
|
||||
- User should test the verification steps above
|
||||
- Once verified, proceed to TASK-010 (EventListener Verification)
|
||||
|
||||
## Notes
|
||||
|
||||
- The Electron app cache at `~/Library/Application Support/Noodl/` on macOS contains user data and projects, so it's NOT automatically cleared
|
||||
- If issues persist after `clean:all`, manually clear: `~/Library/Application Support/Noodl/Code Cache/`, `GPUCache/`, `DawnCache/`
|
||||
@@ -0,0 +1,357 @@
|
||||
/**
|
||||
* EventListenerTest.tsx
|
||||
*
|
||||
* TEMPORARY TEST COMPONENT - Remove after verification complete
|
||||
*
|
||||
* This component tests that the useEventListener hook correctly receives
|
||||
* events from EventDispatcher-based models like ProjectModel.
|
||||
*
|
||||
* Usage:
|
||||
* 1. Import and add to visible location in app
|
||||
* 2. Click "Trigger Test Event" - should show event in log
|
||||
* 3. Rename a component - should show real event in log
|
||||
* 4. Remove this component after verification
|
||||
*
|
||||
* Created for: TASK-010 (EventListener Verification)
|
||||
* Part of: Phase 0 - Foundation Stabilization
|
||||
*/
|
||||
|
||||
// IMPORTANT: Update these imports to match your actual paths
|
||||
import { useEventListener } from '@noodl-hooks/useEventListener';
|
||||
import React, { useState, useCallback } from 'react';
|
||||
|
||||
import { ProjectModel } from '@noodl-models/projectmodel';
|
||||
|
||||
interface EventLogEntry {
|
||||
id: number;
|
||||
timestamp: string;
|
||||
eventName: string;
|
||||
data: string;
|
||||
source: 'manual' | 'real';
|
||||
}
|
||||
|
||||
export function EventListenerTest() {
|
||||
const [eventLog, setEventLog] = useState<EventLogEntry[]>([]);
|
||||
const [counter, setCounter] = useState(0);
|
||||
const [isMinimized, setIsMinimized] = useState(false);
|
||||
|
||||
// Generate unique ID for log entries
|
||||
const nextId = useCallback(() => Date.now() + Math.random(), []);
|
||||
|
||||
// Add entry to log
|
||||
const addLogEntry = useCallback(
|
||||
(eventName: string, data: unknown, source: 'manual' | 'real') => {
|
||||
const entry: EventLogEntry = {
|
||||
id: nextId(),
|
||||
timestamp: new Date().toLocaleTimeString(),
|
||||
eventName,
|
||||
data: JSON.stringify(data, null, 2),
|
||||
source
|
||||
};
|
||||
setEventLog((prev) => [entry, ...prev].slice(0, 20)); // Keep last 20
|
||||
setCounter((c) => c + 1);
|
||||
},
|
||||
[nextId]
|
||||
);
|
||||
|
||||
// ============================================
|
||||
// TEST 1: Single event subscription
|
||||
// ============================================
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
console.log('🎯 TEST [componentRenamed]: Event received!', data);
|
||||
addLogEntry('componentRenamed', data, 'real');
|
||||
});
|
||||
|
||||
// ============================================
|
||||
// TEST 2: Multiple events subscription
|
||||
// ============================================
|
||||
useEventListener(ProjectModel.instance, ['componentAdded', 'componentRemoved'], (data, eventName) => {
|
||||
console.log(`🎯 TEST [${eventName}]: Event received!`, data);
|
||||
addLogEntry(eventName || 'unknown', data, 'real');
|
||||
});
|
||||
|
||||
// ============================================
|
||||
// TEST 3: Root node changes
|
||||
// ============================================
|
||||
useEventListener(ProjectModel.instance, 'rootNodeChanged', (data) => {
|
||||
console.log('🎯 TEST [rootNodeChanged]: Event received!', data);
|
||||
addLogEntry('rootNodeChanged', data, 'real');
|
||||
});
|
||||
|
||||
// Manual trigger for testing
|
||||
const triggerTestEvent = () => {
|
||||
console.log('🧪 Manually triggering componentRenamed event...');
|
||||
|
||||
if (!ProjectModel.instance) {
|
||||
console.error('❌ ProjectModel.instance is null/undefined!');
|
||||
addLogEntry('ERROR', { message: 'ProjectModel.instance is null' }, 'manual');
|
||||
return;
|
||||
}
|
||||
|
||||
const testData = {
|
||||
test: true,
|
||||
timestamp: new Date().toISOString(),
|
||||
random: Math.random().toString(36).substr(2, 9)
|
||||
};
|
||||
|
||||
// @ts-ignore - notifyListeners might not be in types
|
||||
ProjectModel.instance.notifyListeners?.('componentRenamed', testData);
|
||||
|
||||
console.log('🧪 Event triggered with data:', testData);
|
||||
addLogEntry('componentRenamed (manual)', testData, 'manual');
|
||||
};
|
||||
|
||||
// Check ProjectModel status
|
||||
const checkStatus = () => {
|
||||
console.log('📊 ProjectModel Status:');
|
||||
console.log(' - instance:', ProjectModel.instance);
|
||||
console.log(' - instance type:', typeof ProjectModel.instance);
|
||||
console.log(' - has notifyListeners:', typeof (ProjectModel.instance as any)?.notifyListeners);
|
||||
|
||||
addLogEntry(
|
||||
'STATUS_CHECK',
|
||||
{
|
||||
hasInstance: !!ProjectModel.instance,
|
||||
instanceType: typeof ProjectModel.instance
|
||||
},
|
||||
'manual'
|
||||
);
|
||||
};
|
||||
|
||||
if (isMinimized) {
|
||||
return (
|
||||
<div
|
||||
onClick={() => setIsMinimized(false)}
|
||||
style={{
|
||||
position: 'fixed',
|
||||
top: 10,
|
||||
right: 10,
|
||||
background: '#1a1a2e',
|
||||
border: '2px solid #00ff88',
|
||||
borderRadius: 8,
|
||||
padding: '8px 16px',
|
||||
zIndex: 99999,
|
||||
cursor: 'pointer',
|
||||
fontFamily: 'monospace',
|
||||
fontSize: 12,
|
||||
color: '#00ff88'
|
||||
}}
|
||||
>
|
||||
🧪 Events: {counter} (click to expand)
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
style={{
|
||||
position: 'fixed',
|
||||
top: 10,
|
||||
right: 10,
|
||||
background: '#1a1a2e',
|
||||
border: '2px solid #00ff88',
|
||||
borderRadius: 8,
|
||||
padding: 16,
|
||||
zIndex: 99999,
|
||||
width: 350,
|
||||
maxHeight: '80vh',
|
||||
overflow: 'hidden',
|
||||
display: 'flex',
|
||||
flexDirection: 'column',
|
||||
fontFamily: 'monospace',
|
||||
fontSize: 12,
|
||||
color: '#fff',
|
||||
boxShadow: '0 4px 20px rgba(0, 255, 136, 0.3)'
|
||||
}}
|
||||
>
|
||||
{/* Header */}
|
||||
<div
|
||||
style={{
|
||||
display: 'flex',
|
||||
justifyContent: 'space-between',
|
||||
alignItems: 'center',
|
||||
marginBottom: 12,
|
||||
paddingBottom: 8,
|
||||
borderBottom: '1px solid #333'
|
||||
}}
|
||||
>
|
||||
<h3 style={{ margin: 0, color: '#00ff88' }}>🧪 EventListener Test</h3>
|
||||
<button
|
||||
onClick={() => setIsMinimized(true)}
|
||||
style={{
|
||||
background: 'transparent',
|
||||
border: '1px solid #666',
|
||||
color: '#999',
|
||||
padding: '4px 8px',
|
||||
borderRadius: 4,
|
||||
cursor: 'pointer',
|
||||
fontSize: 10
|
||||
}}
|
||||
>
|
||||
minimize
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* Counter */}
|
||||
<div
|
||||
style={{
|
||||
marginBottom: 12,
|
||||
padding: 8,
|
||||
background: '#0a0a15',
|
||||
borderRadius: 4,
|
||||
display: 'flex',
|
||||
justifyContent: 'space-between'
|
||||
}}
|
||||
>
|
||||
<span>Events received:</span>
|
||||
<strong style={{ color: '#00ff88' }}>{counter}</strong>
|
||||
</div>
|
||||
|
||||
{/* Buttons */}
|
||||
<div style={{ display: 'flex', gap: 8, marginBottom: 12 }}>
|
||||
<button
|
||||
onClick={triggerTestEvent}
|
||||
style={{
|
||||
flex: 1,
|
||||
background: '#00ff88',
|
||||
color: '#000',
|
||||
border: 'none',
|
||||
padding: '8px 12px',
|
||||
borderRadius: 4,
|
||||
cursor: 'pointer',
|
||||
fontWeight: 'bold',
|
||||
fontSize: 11
|
||||
}}
|
||||
>
|
||||
🧪 Trigger Test Event
|
||||
</button>
|
||||
<button
|
||||
onClick={checkStatus}
|
||||
style={{
|
||||
background: '#333',
|
||||
color: '#fff',
|
||||
border: 'none',
|
||||
padding: '8px 12px',
|
||||
borderRadius: 4,
|
||||
cursor: 'pointer',
|
||||
fontSize: 11
|
||||
}}
|
||||
>
|
||||
📊 Status
|
||||
</button>
|
||||
<button
|
||||
onClick={() => setEventLog([])}
|
||||
style={{
|
||||
background: '#333',
|
||||
color: '#fff',
|
||||
border: 'none',
|
||||
padding: '8px 12px',
|
||||
borderRadius: 4,
|
||||
cursor: 'pointer',
|
||||
fontSize: 11
|
||||
}}
|
||||
>
|
||||
🗑️
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* Instructions */}
|
||||
<div
|
||||
style={{
|
||||
marginBottom: 12,
|
||||
padding: 8,
|
||||
background: '#1a1a0a',
|
||||
borderRadius: 4,
|
||||
border: '1px solid #444400',
|
||||
fontSize: 10,
|
||||
color: '#999'
|
||||
}}
|
||||
>
|
||||
<strong style={{ color: '#ffff00' }}>Test steps:</strong>
|
||||
<ol style={{ margin: '4px 0 0 0', paddingLeft: 16 }}>
|
||||
<li>Click "Trigger Test Event" - should log below</li>
|
||||
<li>Rename a component in the tree - should log</li>
|
||||
<li>Add/remove components - should log</li>
|
||||
</ol>
|
||||
</div>
|
||||
|
||||
{/* Event Log */}
|
||||
<div
|
||||
style={{
|
||||
flex: 1,
|
||||
background: '#0a0a15',
|
||||
padding: 8,
|
||||
borderRadius: 4,
|
||||
overflow: 'auto',
|
||||
minHeight: 100
|
||||
}}
|
||||
>
|
||||
{eventLog.length === 0 ? (
|
||||
<div style={{ color: '#666', fontStyle: 'italic', textAlign: 'center', padding: 20 }}>
|
||||
No events yet...
|
||||
<br />
|
||||
Click "Trigger Test Event" or
|
||||
<br />
|
||||
rename a component to test
|
||||
</div>
|
||||
) : (
|
||||
eventLog.map((entry) => (
|
||||
<div
|
||||
key={entry.id}
|
||||
style={{
|
||||
borderBottom: '1px solid #222',
|
||||
paddingBottom: 8,
|
||||
marginBottom: 8
|
||||
}}
|
||||
>
|
||||
<div
|
||||
style={{
|
||||
display: 'flex',
|
||||
justifyContent: 'space-between',
|
||||
marginBottom: 4
|
||||
}}
|
||||
>
|
||||
<span
|
||||
style={{
|
||||
color: entry.source === 'manual' ? '#ffaa00' : '#00ff88',
|
||||
fontWeight: 'bold'
|
||||
}}
|
||||
>
|
||||
{entry.eventName}
|
||||
</span>
|
||||
<span style={{ color: '#666', fontSize: 10 }}>{entry.timestamp}</span>
|
||||
</div>
|
||||
<pre
|
||||
style={{
|
||||
margin: 0,
|
||||
fontSize: 10,
|
||||
color: '#888',
|
||||
whiteSpace: 'pre-wrap',
|
||||
wordBreak: 'break-all'
|
||||
}}
|
||||
>
|
||||
{entry.data}
|
||||
</pre>
|
||||
</div>
|
||||
))
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Footer */}
|
||||
<div
|
||||
style={{
|
||||
marginTop: 8,
|
||||
paddingTop: 8,
|
||||
borderTop: '1px solid #333',
|
||||
fontSize: 10,
|
||||
color: '#666',
|
||||
textAlign: 'center'
|
||||
}}
|
||||
>
|
||||
TASK-010 | Phase 0 Foundation | Remove after verification ✓
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export default EventListenerTest;
|
||||
@@ -0,0 +1,220 @@
|
||||
# TASK-010: EventListener Verification
|
||||
|
||||
## Status: 🚧 READY FOR USER TESTING
|
||||
|
||||
## Summary
|
||||
|
||||
Verify that the `useEventListener` hook works correctly with EventDispatcher-based models (like ProjectModel). This validates the React + EventDispatcher integration pattern before using it throughout the codebase.
|
||||
|
||||
## Background
|
||||
|
||||
During TASK-004B (ComponentsPanel migration), we discovered that direct EventDispatcher subscriptions from React components fail silently. Events are emitted but never received due to incompatibility between React's closure-based lifecycle and EventDispatcher's context-object cleanup pattern.
|
||||
|
||||
The `useEventListener` hook was created to solve this, but it needs verification before proceeding.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
✅ TASK-009 must be complete (cache fixes ensure we're testing fresh code)
|
||||
|
||||
## Hook Status
|
||||
|
||||
✅ **Hook exists:** `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts`
|
||||
✅ **Hook has debug logging:** Console logs will show subscription/unsubscription
|
||||
✅ **Test component ready:** `EventListenerTest.tsx` in this directory
|
||||
|
||||
## Verification Steps
|
||||
|
||||
### Step 1: Add Test Component to Editor
|
||||
|
||||
The test component needs to be added somewhere visible in the editor UI.
|
||||
|
||||
**Recommended location:** Add to the main Router component temporarily.
|
||||
|
||||
**File:** `packages/noodl-editor/src/editor/src/router.tsx` (or similar)
|
||||
|
||||
**Add import:**
|
||||
|
||||
```typescript
|
||||
import { EventListenerTest } from '../../tasks/phase-0-foundation-stabalisation/TASK-010-eventlistener-verification/EventListenerTest';
|
||||
```
|
||||
|
||||
**Add to JSX:**
|
||||
|
||||
```tsx
|
||||
render() {
|
||||
return (
|
||||
<div>
|
||||
{/* Existing router content */}
|
||||
|
||||
{/* TEMPORARY: Phase 0 verification */}
|
||||
<EventListenerTest />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
### Step 2: Run the Editor
|
||||
|
||||
```bash
|
||||
npm run clean:all # Clear caches first
|
||||
npm run dev # Start editor
|
||||
```
|
||||
|
||||
### Step 3: Verify Hook Subscription
|
||||
|
||||
1. Open DevTools Console
|
||||
2. Look for these logs:
|
||||
|
||||
```
|
||||
🔥🔥🔥 useEventListener.ts MODULE LOADED WITH DEBUG LOGS - Version 2.0 🔥🔥🔥
|
||||
📡 useEventListener subscribing to: componentRenamed on dispatcher: [ProjectModel]
|
||||
📡 useEventListener subscribing to: ["componentAdded", "componentRemoved"] ...
|
||||
📡 useEventListener subscribing to: rootNodeChanged ...
|
||||
```
|
||||
|
||||
✅ **SUCCESS:** If you see these logs, subscriptions are working
|
||||
|
||||
❌ **FAILURE:** If no subscription logs appear, the hook isn't being called
|
||||
|
||||
### Step 4: Test Manual Event Trigger
|
||||
|
||||
1. Click **"🧪 Trigger Test Event"** button in the test panel
|
||||
2. Check console for:
|
||||
|
||||
```
|
||||
🧪 Manually triggering componentRenamed event...
|
||||
🔔 useEventListener received event: componentRenamed data: {...}
|
||||
```
|
||||
|
||||
3. Check test panel - should show event in log
|
||||
|
||||
✅ **SUCCESS:** Event appears in both console and test panel
|
||||
❌ **FAILURE:** No event received = hook not working
|
||||
|
||||
### Step 5: Test Real Events
|
||||
|
||||
1. In the Noodl editor, rename a component in the component tree
|
||||
2. Check console for:
|
||||
|
||||
```
|
||||
🔔 useEventListener received event: componentRenamed data: {oldName: ..., newName: ...}
|
||||
```
|
||||
|
||||
3. Check test panel - should show the rename event
|
||||
|
||||
✅ **SUCCESS:** Real events are received
|
||||
❌ **FAILURE:** No event = EventDispatcher not emitting or hook not subscribed
|
||||
|
||||
### Step 6: Test Component Add/Remove
|
||||
|
||||
1. Add a new component to the tree
|
||||
2. Remove a component
|
||||
3. Check that events appear in both console and test panel
|
||||
|
||||
### Step 7: Clean Up
|
||||
|
||||
Once verification is complete:
|
||||
|
||||
```typescript
|
||||
// Remove from router.tsx
|
||||
- import { EventListenerTest } from '...';
|
||||
- <EventListenerTest />
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### No Subscription Logs Appear
|
||||
|
||||
**Problem:** Hook never subscribes
|
||||
|
||||
**Solutions:**
|
||||
|
||||
1. Verify EventListenerTest component is actually rendered
|
||||
2. Check React DevTools - is component in the tree?
|
||||
3. Verify import paths are correct
|
||||
4. Run `npm run clean:all` and restart
|
||||
|
||||
### Subscription Logs But No Events Received
|
||||
|
||||
**Problem:** Hook subscribes but events don't arrive
|
||||
|
||||
**Solutions:**
|
||||
|
||||
1. Check if ProjectModel.instance exists: Add this to console:
|
||||
|
||||
```javascript
|
||||
console.log('ProjectModel:', window.require('@noodl-models/projectmodel').ProjectModel);
|
||||
```
|
||||
|
||||
2. Verify EventDispatcher is emitting events:
|
||||
|
||||
```javascript
|
||||
// In ProjectModel code
|
||||
this.notifyListeners('componentRenamed', data); // Should see this
|
||||
```
|
||||
|
||||
3. Check for errors in console
|
||||
|
||||
### Events Work in Test But Not in Real Components
|
||||
|
||||
**Problem:** Test component works but other components don't receive events
|
||||
|
||||
**Cause:** Other components might be using direct `.on()` subscriptions instead of the hook
|
||||
|
||||
**Solution:** Those components need to be migrated to use `useEventListener`
|
||||
|
||||
## Expected Outcomes
|
||||
|
||||
After successful verification:
|
||||
|
||||
✅ Hook subscribes correctly (logs appear)
|
||||
✅ Manual trigger event received
|
||||
✅ Real component rename events received
|
||||
✅ Component add/remove events received
|
||||
✅ No errors in console
|
||||
✅ Events appear in test panel
|
||||
|
||||
## Next Steps After Verification
|
||||
|
||||
1. **If all tests pass:**
|
||||
|
||||
- Mark TASK-010 as complete
|
||||
- Proceed to TASK-011 (Documentation)
|
||||
- Use this pattern for all React + EventDispatcher integrations
|
||||
|
||||
2. **If tests fail:**
|
||||
- Debug the hook implementation
|
||||
- Check EventDispatcher compatibility
|
||||
- May need to create alternative solution
|
||||
|
||||
## Files Modified
|
||||
|
||||
- None (only adding temporary test component)
|
||||
|
||||
## Files to Check
|
||||
|
||||
- `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts` (hook implementation)
|
||||
- `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-010-eventlistener-verification/EventListenerTest.tsx` (test component)
|
||||
|
||||
## Documentation References
|
||||
|
||||
- **Investigation:** `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-008-eventdispatcher-react-investigation/`
|
||||
- **Pattern Guide:** Will be created in TASK-011
|
||||
- **Learnings:** Add findings to `dev-docs/reference/LEARNINGS.md`
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] useEventListener hook exists and is properly exported
|
||||
- [x] Test component created
|
||||
- [ ] Test component added to editor UI
|
||||
- [ ] Hook subscription logs appear in console
|
||||
- [ ] Manual test event received
|
||||
- [ ] Real component rename event received
|
||||
- [ ] Component add/remove events received
|
||||
- [ ] No errors or warnings
|
||||
- [ ] Test component removed after verification
|
||||
|
||||
## Time Estimate
|
||||
|
||||
**Expected:** 1-2 hours (including testing and potential debugging)
|
||||
**If problems found:** +2-4 hours for debugging/fixes
|
||||
@@ -0,0 +1,292 @@
|
||||
# React + EventDispatcher: The Golden Pattern
|
||||
|
||||
> **TL;DR:** Always use `useEventListener` hook. Never use `.on()` directly in React.
|
||||
|
||||
---
|
||||
|
||||
## Quick Start
|
||||
|
||||
```typescript
|
||||
import { useEventListener } from '@noodl-hooks/useEventListener';
|
||||
|
||||
import { ProjectModel } from '@noodl-models/projectmodel';
|
||||
|
||||
function MyComponent() {
|
||||
// Subscribe to events - it just works
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
console.log('Component renamed:', data);
|
||||
});
|
||||
|
||||
return <div>...</div>;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## The Problem
|
||||
|
||||
EventDispatcher uses a context-object pattern for cleanup:
|
||||
|
||||
```typescript
|
||||
// How EventDispatcher works internally
|
||||
model.on('event', callback, contextObject); // Subscribe
|
||||
model.off(contextObject); // Unsubscribe by context
|
||||
```
|
||||
|
||||
React's closure-based lifecycle is incompatible with this:
|
||||
|
||||
```typescript
|
||||
// ❌ This compiles, runs without errors, but SILENTLY FAILS
|
||||
useEffect(() => {
|
||||
const context = {};
|
||||
ProjectModel.instance.on('event', handler, context);
|
||||
return () => ProjectModel.instance.off(context); // Context reference doesn't match!
|
||||
}, []);
|
||||
```
|
||||
|
||||
The event is never received. No errors. Complete silence. Hours of debugging.
|
||||
|
||||
---
|
||||
|
||||
## The Solution
|
||||
|
||||
The `useEventListener` hook handles all the complexity:
|
||||
|
||||
```typescript
|
||||
// ✅ This actually works
|
||||
useEventListener(ProjectModel.instance, 'event', handler);
|
||||
```
|
||||
|
||||
Internally, the hook:
|
||||
|
||||
1. Uses `useRef` to maintain a stable callback reference
|
||||
2. Creates a unique group object per subscription
|
||||
3. Properly cleans up on unmount
|
||||
4. Updates the callback without re-subscribing
|
||||
|
||||
---
|
||||
|
||||
## API Reference
|
||||
|
||||
### Basic Usage
|
||||
|
||||
```typescript
|
||||
useEventListener(dispatcher, eventName, callback);
|
||||
```
|
||||
|
||||
| Parameter | Type | Description |
|
||||
| ------------ | ----------------------------- | ----------------------------- |
|
||||
| `dispatcher` | `IEventEmitter \| null` | The EventDispatcher instance |
|
||||
| `eventName` | `string \| string[]` | Event name(s) to subscribe to |
|
||||
| `callback` | `(data?, eventName?) => void` | Handler function |
|
||||
|
||||
### With Multiple Events
|
||||
|
||||
```typescript
|
||||
useEventListener(
|
||||
ProjectModel.instance,
|
||||
['componentAdded', 'componentRemoved', 'componentRenamed'],
|
||||
(data, eventName) => {
|
||||
console.log(`${eventName}:`, data);
|
||||
}
|
||||
);
|
||||
```
|
||||
|
||||
### With Dependencies
|
||||
|
||||
Re-subscribe when dependencies change:
|
||||
|
||||
```typescript
|
||||
const [filter, setFilter] = useState('all');
|
||||
|
||||
useEventListener(
|
||||
ProjectModel.instance,
|
||||
'componentAdded',
|
||||
(data) => {
|
||||
// Uses current filter value
|
||||
if (matchesFilter(data, filter)) {
|
||||
// ...
|
||||
}
|
||||
},
|
||||
[filter] // Re-subscribe when filter changes
|
||||
);
|
||||
```
|
||||
|
||||
### Conditional Subscription
|
||||
|
||||
Pass `null` to disable:
|
||||
|
||||
```typescript
|
||||
useEventListener(isEnabled ? ProjectModel.instance : null, 'event', handler);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Pattern 1: Trigger Re-render on Changes
|
||||
|
||||
```typescript
|
||||
function useProjectData() {
|
||||
const [updateCounter, setUpdateCounter] = useState(0);
|
||||
|
||||
useEventListener(ProjectModel.instance, ['componentAdded', 'componentRemoved', 'componentRenamed'], () =>
|
||||
setUpdateCounter((c) => c + 1)
|
||||
);
|
||||
|
||||
// Data recomputes when updateCounter changes
|
||||
const data = useMemo(() => {
|
||||
return computeFromProject(ProjectModel.instance);
|
||||
}, [updateCounter]);
|
||||
|
||||
return data;
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 2: Sync State with Model
|
||||
|
||||
```typescript
|
||||
function WarningsPanel() {
|
||||
const [warnings, setWarnings] = useState([]);
|
||||
|
||||
useEventListener(WarningsModel.instance, 'warningsChanged', () => {
|
||||
setWarnings(WarningsModel.instance.getWarnings());
|
||||
});
|
||||
|
||||
return <WarningsList warnings={warnings} />;
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 3: Side Effects
|
||||
|
||||
```typescript
|
||||
function AutoSaver() {
|
||||
useEventListener(
|
||||
ProjectModel.instance,
|
||||
'settingsChanged',
|
||||
debounce(() => {
|
||||
ProjectModel.instance.save();
|
||||
}, 1000)
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Available Dispatchers
|
||||
|
||||
| Instance | Common Events |
|
||||
| -------------------------- | ------------------------------------------------------------------------------------ |
|
||||
| `ProjectModel.instance` | componentAdded, componentRemoved, componentRenamed, rootNodeChanged, settingsChanged |
|
||||
| `NodeLibrary.instance` | libraryUpdated, moduleRegistered, moduleUnregistered |
|
||||
| `WarningsModel.instance` | warningsChanged |
|
||||
| `UndoQueue.instance` | undoHistoryChanged |
|
||||
| `EventDispatcher.instance` | Model.\*, viewer-refresh, ProjectModel.instanceHasChanged |
|
||||
|
||||
---
|
||||
|
||||
## Debugging
|
||||
|
||||
### Verify Events Are Received
|
||||
|
||||
```typescript
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
console.log('🔔 Event received:', data); // Should appear in console
|
||||
// ... your handler
|
||||
});
|
||||
```
|
||||
|
||||
### If Events Aren't Received
|
||||
|
||||
1. **Check event name:** Spelling matters. Use the exact string.
|
||||
2. **Check dispatcher instance:** Is it `null`? Is it the right singleton?
|
||||
3. **Check webpack cache:** Run `npm run clean:all` and restart
|
||||
4. **Check if component mounted:** Add a console.log in the component body
|
||||
|
||||
### Verify Cleanup
|
||||
|
||||
Watch for this error (indicates cleanup failed):
|
||||
|
||||
```
|
||||
Warning: Can't perform a React state update on an unmounted component
|
||||
```
|
||||
|
||||
If you see it, the cleanup isn't working. Check that you're using `useEventListener`, not manual `.on()/.off()`.
|
||||
|
||||
---
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### ❌ Direct .on() in useEffect
|
||||
|
||||
```typescript
|
||||
// BROKEN - Will compile but events never received
|
||||
useEffect(() => {
|
||||
ProjectModel.instance.on('event', handler, {});
|
||||
return () => ProjectModel.instance.off({});
|
||||
}, []);
|
||||
```
|
||||
|
||||
### ❌ Manual forceRefresh Callbacks
|
||||
|
||||
```typescript
|
||||
// WORKS but creates tech debt
|
||||
const forceRefresh = useCallback(() => setCounter((c) => c + 1), []);
|
||||
performAction(data, forceRefresh); // Must thread through everywhere
|
||||
```
|
||||
|
||||
### ❌ Class Component Style
|
||||
|
||||
```typescript
|
||||
// DOESN'T WORK in functional components
|
||||
this.model.on('event', this.handleEvent, this);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Migration Guide
|
||||
|
||||
Converting existing broken code:
|
||||
|
||||
### Before
|
||||
|
||||
```typescript
|
||||
function MyComponent() {
|
||||
const [data, setData] = useState(null);
|
||||
|
||||
useEffect(() => {
|
||||
const listener = {};
|
||||
ProjectModel.instance.on('componentRenamed', (d) => setData(d), listener);
|
||||
return () => ProjectModel.instance.off(listener);
|
||||
}, []);
|
||||
|
||||
return <div>{data}</div>;
|
||||
}
|
||||
```
|
||||
|
||||
### After
|
||||
|
||||
```typescript
|
||||
import { useEventListener } from '@noodl-hooks/useEventListener';
|
||||
|
||||
function MyComponent() {
|
||||
const [data, setData] = useState(null);
|
||||
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', setData);
|
||||
|
||||
return <div>{data}</div>;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## History
|
||||
|
||||
- **Discovered:** 2025-12-22 during TASK-004B (ComponentsPanel React Migration)
|
||||
- **Investigated:** TASK-008 (EventDispatcher React Investigation)
|
||||
- **Verified:** TASK-010 (EventListener Verification)
|
||||
- **Documented:** TASK-011 (This document)
|
||||
|
||||
The root cause is a fundamental incompatibility between EventDispatcher's context-object cleanup pattern and React's closure-based lifecycle. The `useEventListener` hook bridges this gap.
|
||||
@@ -0,0 +1,111 @@
|
||||
# TASK-011: React Event Pattern Guide Documentation
|
||||
|
||||
## Status: ✅ COMPLETED
|
||||
|
||||
## Summary
|
||||
|
||||
Document the React + EventDispatcher pattern in all relevant locations so future developers follow the correct approach and avoid the silent subscription failure pitfall.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### 1. Created GOLDEN-PATTERN.md ✅
|
||||
|
||||
**Location:** `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-011-react-event-pattern-guide/GOLDEN-PATTERN.md`
|
||||
|
||||
Comprehensive pattern guide including:
|
||||
|
||||
- Quick start examples
|
||||
- Problem explanation
|
||||
- API reference
|
||||
- Common patterns
|
||||
- Debugging guide
|
||||
- Anti-patterns to avoid
|
||||
- Migration examples
|
||||
|
||||
### 2. Updated .clinerules ✅
|
||||
|
||||
**File:** `.clinerules` (root)
|
||||
|
||||
Added React + EventDispatcher section:
|
||||
|
||||
```markdown
|
||||
## Section: React + EventDispatcher Integration
|
||||
|
||||
### CRITICAL: Always use useEventListener hook
|
||||
|
||||
When subscribing to EventDispatcher events from React components, ALWAYS use the `useEventListener` hook.
|
||||
Direct subscriptions silently fail.
|
||||
|
||||
**✅ CORRECT:**
|
||||
import { useEventListener } from '@noodl-hooks/useEventListener';
|
||||
useEventListener(ProjectModel.instance, 'componentRenamed', (data) => {
|
||||
// This works!
|
||||
});
|
||||
|
||||
**❌ BROKEN:**
|
||||
useEffect(() => {
|
||||
const context = {};
|
||||
ProjectModel.instance.on('event', handler, context);
|
||||
return () => ProjectModel.instance.off(context);
|
||||
}, []);
|
||||
// Compiles and runs without errors, but events are NEVER received
|
||||
|
||||
### Why this matters
|
||||
|
||||
EventDispatcher uses context-object cleanup pattern incompatible with React closures.
|
||||
Direct subscriptions fail silently - no errors, no events, just confusion.
|
||||
|
||||
### Available dispatchers
|
||||
|
||||
- ProjectModel.instance
|
||||
- NodeLibrary.instance
|
||||
- WarningsModel.instance
|
||||
- EventDispatcher.instance
|
||||
- UndoQueue.instance
|
||||
|
||||
### Full documentation
|
||||
|
||||
See: dev-docs/tasks/phase-0-foundation-stabalisation/TASK-011-react-event-pattern-guide/GOLDEN-PATTERN.md
|
||||
```
|
||||
|
||||
### 3. Updated LEARNINGS.md ✅
|
||||
|
||||
**File:** `dev-docs/reference/LEARNINGS.md`
|
||||
|
||||
Added entry documenting the discovery and solution.
|
||||
|
||||
## Documentation Locations
|
||||
|
||||
The pattern is now documented in:
|
||||
|
||||
1. **Primary Reference:** `GOLDEN-PATTERN.md` (this directory)
|
||||
2. **AI Instructions:** `.clinerules` (root) - Section on React + EventDispatcher
|
||||
3. **Institutional Knowledge:** `dev-docs/reference/LEARNINGS.md`
|
||||
4. **Investigation Details:** `TASK-008-eventdispatcher-react-investigation/`
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] GOLDEN-PATTERN.md created with comprehensive examples
|
||||
- [x] .clinerules updated with critical warning and examples
|
||||
- [x] LEARNINGS.md updated with pattern entry
|
||||
- [x] Pattern is searchable and discoverable
|
||||
- [x] Clear anti-patterns documented
|
||||
|
||||
## For Future Developers
|
||||
|
||||
When working with EventDispatcher from React components:
|
||||
|
||||
1. **Search first:** `grep -r "useEventListener" .clinerules`
|
||||
2. **Read the pattern:** `GOLDEN-PATTERN.md` in this directory
|
||||
3. **Never use direct `.on()` in React:** It silently fails
|
||||
4. **Follow the examples:** Copy from GOLDEN-PATTERN.md
|
||||
|
||||
## Time Spent
|
||||
|
||||
**Actual:** ~1 hour (documentation writing and organization)
|
||||
|
||||
## Next Steps
|
||||
|
||||
- TASK-012: Create health check script to validate patterns automatically
|
||||
- Use this pattern in all future React migrations
|
||||
- Update existing components that use broken patterns
|
||||
@@ -0,0 +1,188 @@
|
||||
# TASK-012: Foundation Health Check Script
|
||||
|
||||
## Status: ✅ COMPLETED
|
||||
|
||||
## Summary
|
||||
|
||||
Created an automated health check script that validates Phase 0 foundation fixes are in place and working correctly. This prevents regressions and makes it easy to verify the development environment is properly configured.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### 1. Created Health Check Script ✅
|
||||
|
||||
**File:** `scripts/health-check.js`
|
||||
|
||||
A comprehensive Node.js script that validates:
|
||||
|
||||
1. **Webpack Cache Configuration** - Confirms babel cache is disabled
|
||||
2. **Clean Script** - Verifies `clean:all` exists in package.json
|
||||
3. **Build Canary** - Checks timestamp canary is in editor entry point
|
||||
4. **useEventListener Hook** - Confirms hook exists and is properly exported
|
||||
5. **Anti-Pattern Detection** - Scans for direct `.on()` usage in React code (warnings only)
|
||||
6. **Documentation** - Verifies Phase 0 documentation exists
|
||||
|
||||
### 2. Added npm Script ✅
|
||||
|
||||
**File:** `package.json`
|
||||
|
||||
```json
|
||||
"health:check": "node scripts/health-check.js"
|
||||
```
|
||||
|
||||
## Usage
|
||||
|
||||
### Run Health Check
|
||||
|
||||
```bash
|
||||
npm run health:check
|
||||
```
|
||||
|
||||
### Expected Output (All Pass)
|
||||
|
||||
```
|
||||
============================================================
|
||||
1. Webpack Cache Configuration
|
||||
============================================================
|
||||
|
||||
✅ Babel cache disabled in webpack config
|
||||
|
||||
============================================================
|
||||
2. Clean Script
|
||||
============================================================
|
||||
|
||||
✅ clean:all script exists in package.json
|
||||
|
||||
...
|
||||
|
||||
============================================================
|
||||
Health Check Summary
|
||||
============================================================
|
||||
|
||||
✅ Passed: 10
|
||||
⚠️ Warnings: 0
|
||||
❌ Failed: 0
|
||||
|
||||
✅ HEALTH CHECK PASSED
|
||||
Phase 0 Foundation is healthy!
|
||||
```
|
||||
|
||||
### Exit Codes
|
||||
|
||||
- **0** - All checks passed (with or without warnings)
|
||||
- **1** - One or more checks failed
|
||||
|
||||
### Check Results
|
||||
|
||||
- **✅ Pass** - Check succeeded, everything configured correctly
|
||||
- **⚠️ Warning** - Check passed but there's room for improvement
|
||||
- **❌ Failed** - Critical issue, must be fixed
|
||||
|
||||
## When to Run
|
||||
|
||||
Run the health check:
|
||||
|
||||
1. **After setting up a new development environment**
|
||||
2. **Before starting React migration work**
|
||||
3. **After pulling major changes from git**
|
||||
4. **When experiencing mysterious build/cache issues**
|
||||
5. **As part of CI/CD pipeline** (optional)
|
||||
|
||||
## What It Checks
|
||||
|
||||
### Critical Checks (Fail on Error)
|
||||
|
||||
1. **Webpack config** - Babel cache must be disabled in dev
|
||||
2. **package.json** - clean:all script must exist
|
||||
3. **Build canary** - Timestamp logging must be present
|
||||
4. **useEventListener hook** - Hook must exist and be exported properly
|
||||
|
||||
### Warning Checks
|
||||
|
||||
5. **Anti-patterns** - Warns about direct `.on()` usage in React (doesn't fail)
|
||||
6. **Documentation** - Warns if Phase 0 docs are missing
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### If Health Check Fails
|
||||
|
||||
1. **Read the error message** - It tells you exactly what's missing
|
||||
2. **Review the Phase 0 tasks:**
|
||||
- TASK-009 for cache/build issues
|
||||
- TASK-010 for hook issues
|
||||
- TASK-011 for documentation
|
||||
3. **Run `npm run clean:all`** if cache-related
|
||||
4. **Re-run health check** after fixes
|
||||
|
||||
### Common Failures
|
||||
|
||||
**"Babel cache ENABLED in webpack"**
|
||||
|
||||
- Fix: Edit `packages/noodl-editor/webpackconfigs/shared/webpack.renderer.core.js`
|
||||
- Change `cacheDirectory: true` to `cacheDirectory: false`
|
||||
|
||||
**"clean:all script missing"**
|
||||
|
||||
- Fix: Add to package.json scripts section
|
||||
- See TASK-009 documentation
|
||||
|
||||
**"Build canary missing"**
|
||||
|
||||
- Fix: Add to `packages/noodl-editor/src/editor/index.ts`
|
||||
- Add: `console.log('🔥 BUILD TIMESTAMP:', new Date().toISOString());`
|
||||
|
||||
**"useEventListener hook not found"**
|
||||
|
||||
- Fix: Ensure `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts` exists
|
||||
- See TASK-010 documentation
|
||||
|
||||
## Integration with CI/CD
|
||||
|
||||
To add to CI pipeline:
|
||||
|
||||
```yaml
|
||||
# .github/workflows/ci.yml
|
||||
- name: Foundation Health Check
|
||||
run: npm run health:check
|
||||
```
|
||||
|
||||
This ensures Phase 0 fixes don't regress in production.
|
||||
|
||||
## Future Enhancements
|
||||
|
||||
Potential additions:
|
||||
|
||||
- Check for stale Electron cache
|
||||
- Verify React version compatibility
|
||||
- Check for common webpack misconfigurations
|
||||
- Validate EventDispatcher subscriptions in test mode
|
||||
- Generate detailed report file
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [x] Script created in `scripts/health-check.js`
|
||||
- [x] Added to package.json as `health:check`
|
||||
- [x] Validates all Phase 0 fixes
|
||||
- [x] Clear pass/warn/fail output
|
||||
- [x] Proper exit codes
|
||||
- [x] Documentation complete
|
||||
- [x] Tested and working
|
||||
|
||||
## Time Spent
|
||||
|
||||
**Actual:** ~1 hour (script development and testing)
|
||||
|
||||
## Files Created
|
||||
|
||||
- `scripts/health-check.js` - Main health check script
|
||||
- `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-012-foundation-health-check/README.md` - This file
|
||||
|
||||
## Files Modified
|
||||
|
||||
- `package.json` - Added `health:check` script
|
||||
|
||||
## Next Steps
|
||||
|
||||
- Run `npm run health:check` regularly during development
|
||||
- Add to onboarding docs for new developers
|
||||
- Consider adding to pre-commit hook (optional)
|
||||
- Use before starting any React migration work
|
||||
@@ -0,0 +1,307 @@
|
||||
# Phase 0: Complete Verification Guide
|
||||
|
||||
## Overview
|
||||
|
||||
This guide will walk you through verifying both TASK-009 (cache fixes) and TASK-010 (EventListener hook) in one session. Total time: ~30 minutes.
|
||||
|
||||
---
|
||||
|
||||
## Prerequisites
|
||||
|
||||
✅ Health check passed: `npm run health:check`
|
||||
✅ EventListenerTest component added to Router
|
||||
✅ All Phase 0 infrastructure in place
|
||||
|
||||
---
|
||||
|
||||
## Part 1: Cache Fix Verification (TASK-009)
|
||||
|
||||
### Step 1: Clean Start
|
||||
|
||||
```bash
|
||||
npm run clean:all
|
||||
npm run dev
|
||||
```
|
||||
|
||||
**Wait for:** Electron window to launch
|
||||
|
||||
### Step 2: Check Build Canary
|
||||
|
||||
1. Open DevTools Console: **View → Toggle Developer Tools**
|
||||
2. Look for: `🔥 BUILD TIMESTAMP: [recent time]`
|
||||
3. **Write down the timestamp:** ************\_\_\_************
|
||||
|
||||
✅ **Pass criteria:** Timestamp appears and is recent
|
||||
|
||||
### Step 3: Test Code Change Detection
|
||||
|
||||
1. Open: `packages/noodl-editor/src/editor/index.ts`
|
||||
2. Find line: `console.log('🔥 BUILD TIMESTAMP:', new Date().toISOString());`
|
||||
3. Change to: `console.log('🔥🔥 BUILD TIMESTAMP:', new Date().toISOString());`
|
||||
4. **Save the file**
|
||||
5. Wait 5-10 seconds for webpack to recompile (watch terminal)
|
||||
6. **Reload Electron app:** Cmd+R (macOS) / Ctrl+R (Windows/Linux)
|
||||
7. Check console - should show **two fire emojis** now
|
||||
8. **Write down new timestamp:** ************\_\_\_************
|
||||
|
||||
✅ **Pass criteria:**
|
||||
|
||||
- Two fire emojis appear
|
||||
- Timestamp is different from Step 2
|
||||
- Change appeared within 10 seconds
|
||||
|
||||
### Step 4: Test Reliability
|
||||
|
||||
1. Change to: `console.log('🔥🔥🔥 BUILD TIMESTAMP:', new Date().toISOString());`
|
||||
2. Save, wait, reload
|
||||
3. **Write down timestamp:** ************\_\_\_************
|
||||
|
||||
✅ **Pass criteria:** Three fire emojis, new timestamp
|
||||
|
||||
### Step 5: Revert Changes
|
||||
|
||||
1. Change back to: `console.log('🔥 BUILD TIMESTAMP:', new Date().toISOString());`
|
||||
2. Save, wait, reload
|
||||
3. Verify: One fire emoji, new timestamp
|
||||
|
||||
✅ **Pass criteria:** Back to original state, timestamps keep updating
|
||||
|
||||
---
|
||||
|
||||
## Part 2: EventListener Hook Verification (TASK-010)
|
||||
|
||||
**Note:** The editor should still be running from Part 1. If you closed it, restart with `npm run dev`.
|
||||
|
||||
### Step 6: Verify Test Component Visible
|
||||
|
||||
1. Look in **top-right corner** of the editor window
|
||||
2. You should see a **green panel** labeled: `🧪 EventListener Test`
|
||||
|
||||
✅ **Pass criteria:** Test panel is visible
|
||||
|
||||
**If not visible:**
|
||||
|
||||
- Check console for errors
|
||||
- Verify import worked: Search console for "useEventListener"
|
||||
- If component isn't rendering, check Router.tsx
|
||||
|
||||
### Step 7: Check Hook Subscription Logs
|
||||
|
||||
1. In console, look for these logs:
|
||||
|
||||
```
|
||||
📡 useEventListener subscribing to: componentRenamed
|
||||
📡 useEventListener subscribing to: ["componentAdded", "componentRemoved"]
|
||||
📡 useEventListener subscribing to: rootNodeChanged
|
||||
```
|
||||
|
||||
✅ **Pass criteria:** All three subscription logs appear
|
||||
|
||||
**If missing:**
|
||||
|
||||
- Hook isn't being called
|
||||
- Check console for errors
|
||||
- Verify useEventListener.ts exists and is exported
|
||||
|
||||
### Step 8: Test Manual Event Trigger
|
||||
|
||||
1. In the test panel, click: **🧪 Trigger Test Event**
|
||||
2. **Check console** for:
|
||||
|
||||
```
|
||||
🧪 Manually triggering componentRenamed event...
|
||||
🎯 TEST [componentRenamed]: Event received!
|
||||
```
|
||||
|
||||
3. **Check test panel** - should show event in the log with timestamp
|
||||
|
||||
✅ **Pass criteria:**
|
||||
|
||||
- Console shows event triggered and received
|
||||
- Test panel shows event entry
|
||||
- Counter increments
|
||||
|
||||
**If fails:**
|
||||
|
||||
- Click 📊 Status button to check ProjectModel
|
||||
- If ProjectModel is null, you need to open a project first
|
||||
|
||||
### Step 9: Open a Project
|
||||
|
||||
1. If you're on the Projects page, open any project
|
||||
2. Wait for editor to load
|
||||
3. Repeat Step 8 - manual trigger should now work
|
||||
|
||||
### Step 10: Test Real Component Rename
|
||||
|
||||
1. In the component tree (left panel), find any component
|
||||
2. Right-click → Rename (or double-click to rename)
|
||||
3. Change the name and press Enter
|
||||
|
||||
**Check:**
|
||||
|
||||
- Console shows: `🎯 TEST [componentRenamed]: Event received!`
|
||||
- Test panel logs the rename event with data
|
||||
- Counter increments
|
||||
|
||||
✅ **Pass criteria:** Real rename event is captured
|
||||
|
||||
### Step 11: Test Component Add/Remove
|
||||
|
||||
1. **Add a component:**
|
||||
|
||||
- Right-click in component tree
|
||||
- Select "New Component"
|
||||
- Name it and press Enter
|
||||
|
||||
2. **Check:**
|
||||
|
||||
- Console: `🎯 TEST [componentAdded]: Event received!`
|
||||
- Test panel logs the event
|
||||
|
||||
3. **Remove the component:**
|
||||
|
||||
- Right-click the new component
|
||||
- Select "Delete"
|
||||
|
||||
4. **Check:**
|
||||
- Console: `🎯 TEST [componentRemoved]: Event received!`
|
||||
- Test panel logs the event
|
||||
|
||||
✅ **Pass criteria:** Both add and remove events captured
|
||||
|
||||
---
|
||||
|
||||
## Part 3: Clean Up
|
||||
|
||||
### Step 12: Remove Test Component
|
||||
|
||||
1. Close Electron app
|
||||
2. Open: `packages/noodl-editor/src/editor/src/router.tsx`
|
||||
3. Remove the import:
|
||||
|
||||
```typescript
|
||||
// TEMPORARY: Phase 0 verification - Remove after TASK-010 complete
|
||||
import { EventListenerTest } from './views/EventListenerTest';
|
||||
```
|
||||
|
||||
4. Remove from render:
|
||||
|
||||
```typescript
|
||||
{
|
||||
/* TEMPORARY: Phase 0 verification - Remove after TASK-010 complete */
|
||||
}
|
||||
<EventListenerTest />;
|
||||
```
|
||||
|
||||
5. Save the file
|
||||
|
||||
6. Delete the test component:
|
||||
|
||||
```bash
|
||||
rm packages/noodl-editor/src/editor/src/views/EventListenerTest.tsx
|
||||
```
|
||||
|
||||
7. **Optional:** Start editor again to verify it works without test component:
|
||||
|
||||
```bash
|
||||
npm run dev
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Verification Results
|
||||
|
||||
### TASK-009: Cache Fixes
|
||||
|
||||
- [ ] Build timestamp appears on startup
|
||||
- [ ] Code changes load within 10 seconds
|
||||
- [ ] Timestamps update on each change
|
||||
- [ ] Tested 3 times successfully
|
||||
|
||||
**Status:** ✅ PASS / ❌ FAIL
|
||||
|
||||
### TASK-010: EventListener Hook
|
||||
|
||||
- [ ] Test component rendered
|
||||
- [ ] Subscription logs appear
|
||||
- [ ] Manual test event works
|
||||
- [ ] Real componentRenamed event works
|
||||
- [ ] Component add event works
|
||||
- [ ] Component remove event works
|
||||
|
||||
**Status:** ✅ PASS / ❌ FAIL
|
||||
|
||||
---
|
||||
|
||||
## If Any Tests Fail
|
||||
|
||||
### Cache Issues (TASK-009)
|
||||
|
||||
1. Run `npm run clean:all` again
|
||||
2. Manually clear Electron cache:
|
||||
- macOS: `~/Library/Application Support/Noodl/`
|
||||
- Windows: `%APPDATA%/Noodl/`
|
||||
- Linux: `~/.config/Noodl/`
|
||||
3. Kill all Node/Electron processes: `pkill -f node; pkill -f Electron`
|
||||
4. Restart from Step 1
|
||||
|
||||
### EventListener Issues (TASK-010)
|
||||
|
||||
1. Check console for errors
|
||||
2. Verify hook exists: `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts`
|
||||
3. Check ProjectModel is loaded (open a project first)
|
||||
4. Add debug logging to hook
|
||||
5. Check `.clinerules` has EventListener documentation
|
||||
|
||||
---
|
||||
|
||||
## Success Criteria
|
||||
|
||||
Phase 0 is complete when:
|
||||
|
||||
✅ All TASK-009 tests pass
|
||||
✅ All TASK-010 tests pass
|
||||
✅ Test component removed
|
||||
✅ Editor runs without errors
|
||||
✅ Documentation in place
|
||||
|
||||
---
|
||||
|
||||
## Next Steps After Verification
|
||||
|
||||
Once verified:
|
||||
|
||||
1. **Update task status:**
|
||||
- Mark TASK-009 as verified
|
||||
- Mark TASK-010 as verified
|
||||
2. **Return to Phase 2 work:**
|
||||
- TASK-004B (ComponentsPanel migration) is now UNBLOCKED
|
||||
- Future React migrations can use documented pattern
|
||||
3. **Run health check periodically:**
|
||||
```bash
|
||||
npm run health:check
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Troubleshooting Quick Reference
|
||||
|
||||
| Problem | Solution |
|
||||
| ------------------------------ | ------------------------------------------------------- |
|
||||
| Build timestamp doesn't update | Run `npm run clean:all`, restart server |
|
||||
| Changes don't load | Check webpack compilation in terminal, verify no errors |
|
||||
| Test component not visible | Check console for import errors, verify Router.tsx |
|
||||
| No subscription logs | Hook not being called, check imports |
|
||||
| Events not received | ProjectModel might be null, open a project first |
|
||||
| Manual trigger fails | Check ProjectModel.instance in console |
|
||||
|
||||
---
|
||||
|
||||
**Estimated Total Time:** 20-30 minutes
|
||||
|
||||
**Questions?** Check:
|
||||
|
||||
- `dev-docs/tasks/phase-0-foundation-stabalisation/QUICK-START.md`
|
||||
- `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-009-verification-checklist/`
|
||||
- `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-010-eventlistener-verification/`
|
||||
Reference in New Issue
Block a user