diff --git a/CORE_REFACTOR_DEFERRED.md b/CORE_REFACTOR_DEFERRED.md index 3984c81a6..3a4aa5689 100644 --- a/CORE_REFACTOR_DEFERRED.md +++ b/CORE_REFACTOR_DEFERRED.md @@ -6,7 +6,7 @@ These items came out of the PR reviews for the multi-phase refactor (`core-refac ### Type the `core: any` back-reference in extracted managers -Every new manager declares `core: any;` — Phase 1 (`DiagnosticsManager`, `VisibilityTracker`, `StatePersistence`, `AutoSubmitManager`, `NavigationHandler`, `ResolverAdapter`) and Phase 2 (`RendererInstructionBuilder`, `ProcessQueue`, `ComponentLifecycle`, `ChildMatcher`, `DeletionEngine`, `ActionTriggerScheduler`). That defeats the point of converting to TypeScript — typos and accidental property reads through `core` go unchecked. +Every new manager declares `core: any;` — Phase 1 (`DiagnosticsManager`, `VisibilityTracker`, `StatePersistence`, `AutoSubmitManager`, `NavigationHandler`, `ResolverAdapter`), Phase 2 (`RendererInstructionBuilder`, `ProcessQueue`, `ComponentLifecycle`, `ChildMatcher`, `DeletionEngine`, `ActionTriggerScheduler`), and Phase 3 (`StateVariableDefinitionFactory`, `StateVariableInitializer`, `ComponentBuilder`, `CompositeExpander`). That defeats the point of converting to TypeScript — typos and accidental property reads through `core` go unchecked. Since `Core.js` is still JavaScript, defining a real `Core` interface is awkward. Two practical paths: @@ -21,6 +21,7 @@ Several managers hold no state of their own — only a `core` back-reference: - Phase 1: `NavigationHandler`, `ResolverAdapter` - Phase 2: `ComponentLifecycle`, `DeletionEngine` (and `ChildMatcher`, modulo its single recursion-guard array) +- Phase 3: `StateVariableDefinitionFactory`, `StateVariableInitializer`, `ComponentBuilder`, `CompositeExpander` — none keep their own state; they only read/write through `core` The pure-function shape used in `StateVariableNameResolver.ts` is more honest for these: @@ -67,9 +68,9 @@ A cleaner shape: make `calcStartEndIdx` a standalone helper (or a private method Three intentionally unawaited Promise calls remain in `Core.js`. They pre-date this PR (this refactor did not introduce them, and adding `.catch` handlers was out of scope for a behavior-preserving extraction), but they violate the AGENTS.md convention that fire-and-forget Promises must attach an explicit `.catch(...)` handler. They should be picked up in the next phase that touches Core directly. -- **`Core.js:410`** — `this.saveState();` inside the `generateDast` epilogue (`if (!this.receivedStateVariableChanges) { this.saveState(); }`). Returns a Promise; rejection is unhandled. -- **`Core.js:11160`** — `this.saveState(true, true);` inside `processNewStateVariableValues` after recording component submissions. Same shape as above. -- **`Core.js:449-452`** — `setTimeout(this.sendVisibilityChangedEvents.bind(this), this.visibilityInfo.saveDelay)` inside `onDocumentFirstVisible()`. The bound function returns `Promise | undefined`; setTimeout discards the return, so any rejection is unhandled. +- **`Core.js:444`** — `this.saveState();` inside the `generateDast` epilogue (`if (!this.receivedStateVariableChanges) { this.saveState(); }`). Returns a Promise; rejection is unhandled. +- **`Core.js:4387`** — `this.saveState(true, true);` inside `processNewStateVariableValues` after recording component submissions. Same shape as above. +- **`Core.js:483-486`** — `setTimeout(this.sendVisibilityChangedEvents.bind(this), this.visibilityInfo.saveDelay)` inside `onDocumentFirstVisible()`. The bound function returns `Promise | undefined`; setTimeout discards the return, so any rejection is unhandled. Suggested fix in each case: wrap with `.catch(reportTimerError("