refactor(worker-javascript): extract Phase 4 helpers from Core.js (final)#1040
Open
dqnykamp wants to merge 10 commits intoDoenet:mainfrom
Open
refactor(worker-javascript): extract Phase 4 helpers from Core.js (final)#1040dqnykamp wants to merge 10 commits intoDoenet:mainfrom
dqnykamp wants to merge 10 commits intoDoenet:mainfrom
Conversation
Begin breaking up the 13,837-line Core class by lifting seven self- contained, low-coupling helpers into TypeScript modules. The pattern matches the existing composed siblings (Dependencies.js, ParameterStack): each module is constructed with a `core` back-reference, and Core retains a thin delegating wrapper for every method/property that was previously on the class so external callers (CoreWorker, tests, components, and `coreFunctions`-bound references) continue to work unchanged. Modules extracted: - DiagnosticsManager.ts — diagnostics queue + source-location walk - StateVariableNameResolver.ts — pure-function name resolution utilities - VisibilityTracker.ts — visibility state and save/suspend timers - StatePersistence.ts — save to localStorage / database - AutoSubmitManager.ts — debounced answer-submit queue - NavigationHandler.ts — handleNavigatingToComponent, navigateToTarget - ResolverAdapter.ts — adapter to the external Rust name resolver No behavior change. Core.js drops from 13,837 to 12,909 lines. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Continues the Core.js breakup with six moderate-effort modules. Same pattern as Phase 1 (composed sibling holding a `core` back-reference, thin delegating wrapper on Core for every public method/property). No behavior change. Modules extracted: - RendererInstructionBuilder.ts — owns componentsToRender, componentsWithChangedChildrenToRender, rendererState; the dast instruction stream sent to the renderer - ProcessQueue.ts — owns processQueue, processing, stopProcessingRequests; async request queue and entry-point scheduling (executeProcesses, requestAction, requestUpdate, requestRecordEvent) - ComponentLifecycle.ts — stateless: registration, ancestors, defining-child splicing, propagation to shadows - ChildMatcher.ts — child-group matching, adapter substitution, rendered-child filtering (recursion guard only) - DeletionEngine.ts — stateless two-phase component deletion - ActionTriggerScheduler.ts — owns stateVariableChangeTriggers, actionsChangedToActions, originsOfActionsChangedToActions; trigger polling and chained-action graph Core.js drops from 12,909 to 11,253 lines (this PR), 13,837 → 11,253 since the refactor began (~18.7%). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The deep guts of Core. Same composition pattern as Phases 1 and 2: each module is constructed with a `core` back-reference, and Core retains thin delegating wrappers for every public method/property. No behavior change. Modules extracted (in dependency order): - StateVariableDefinitionFactory.ts — synchronous shape-building: attribute / adapter / reference-shadow definitions, plus the shadow-conversion modifiers - StateVariableInitializer.ts — runtime initialization: lazy-resolving getters, dependency wiring, array-entry materialization, prop-index resolution - ComponentBuilder.ts — recursive component instantiation from serialized DAST plus the post-creation error-component flush - CompositeExpander.ts — composite expansion + replacement swap into active children + active/inactive marking; mutually recursive with ComponentBuilder via Core's delegators Subtle fix: `core.publicCaseInsensitiveAliasSubstitutions.bind(this)` calls inside CompositeExpander needed `bind(this.core)` — the wrapper on Core uses `this.componentInfoObjects`, so the bind target must be Core, not the manager. Core.js drops from 11,253 to 6,063 lines (this PR), 13,837 → 6,063 since the refactor began (~56.2%). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…ndings
Two regressions from the Phase 3 extraction, surfaced in CI:
1. StateVariableInitializer.ts:425: \`let core = this;\` captured the
manager instead of Core, so the five \`core.addDiagnostic({...})\`
calls inside the inner array-callbacks (set up by
\`initializeArrayStateVariable\` for setArrayValue / getArrayValue /
etc.) failed at runtime with "core.addDiagnostic is not a function".
Fix: \`let core = this.core;\`. Caught by functionoperators.test.ts.
2. StateVariableDefinitionFactory.ts:1299: inside
\`stateDef.returnArrayDependenciesByKey = function () {...}\`, \`this\`
resolves to the stateDef at call time (not the manager), and
\`stateDef.arrayVarNameFromArrayKey\` is the method to call. The
blanket \`this.\` → \`this.core.\` transform in the extraction script
incorrectly added \`core.\` here. Fix: revert to
\`this.arrayVarNameFromArrayKey(key)\`. Caught by spreadsheet,
curve, curve.bezier, odesystem, and rectangle tests.
Audited the rest of the extracted modules for the same pattern
(\`function () {...}\` callbacks attached to stateVarObj/stateDef with
\`this.core.X\` references inside): no further occurrences.
ComponentBuilder.ts and CompositeExpander.ts have no inner-callback
patterns at all.
Verified: all 91 tests across the failing CI files plus 238 broader
regression tests pass.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The final phase. Same composition pattern as the prior phases. With
the Phase 3 binding-bug lessons in hand, each cluster was pre-audited
for `let core = this` captures and `function () {...}` callbacks
attached to stateVarObj/stateDef before the bulk transformation; all
five Phase 4 clusters were clean. No behavior change.
Modules extracted (in dependency order):
- StateVariableEvaluator.ts — `getStateVariableValue` resolution +
`getStateVariableDefinitionArguments` + `recordActualChangeInStateVariable`
- StalenessPropagator.ts — mark-stale walk through the dependency
graph plus on-demand `createFromArrayEntry`
- EssentialValueWriter.ts — `processNewStateVariableValues` +
`requestComponentChanges` + `executeUpdateStateVariables` +
`replacementChangesFromCompositesToUpdate` + the diff-record
calculators that feed `StatePersistence`
- CompositeReplacementUpdater.ts — `updateCompositeReplacements` and
the supporting shadow / withhold / error-replacement plumbing
- UpdateExecutor.ts — `performAction` and `performUpdate`, the
orchestrators dequeued by `ProcessQueue`
Side-effect: top-level helper `calculateAllComponentsShadowing` was
used by both Core and the new CompositeReplacementUpdater. Moved it
into CompositeReplacementUpdater.ts (sole remaining caller).
Core.js drops from 6,063 to 1,411 lines (this PR), 13,837 → 1,411
since the refactor began (~89.8%). The plan's end-state target was
~1,400 lines for Core, holding the constructor, the `generateDast`
orchestrator, the public-API delegators, and the hot-state fields.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stacked on #1039 (which stacks on #1038, which stacks on #1036). This PR's diff includes Phases 1, 2, and 3 until those land; review Phase 4 by reading the final commits or use the commits tab. Once the prior PRs merge, this PR will auto-rebase and the diff will show only Phase 4.
Summary
The final phase of breaking up `packages/doenetml-worker-javascript/src/Core.js`. Five remaining engines plus the `performAction`/`performUpdate` orchestrators. Same composition pattern. No behavior change.
Net effect over all four phases: `Core.js` drops from 13,837 → 1,411 lines (-89.8%). That hits the plan's end-state target almost to the line — what remains is the constructor, the `generateDast` orchestrator, the public-API delegators, and the hot-state fields.
Modules extracted in Phase 4
Pre-extraction discipline
After the Phase 3 binding bugs (caught only in CI), each Phase 4 cluster was pre-audited for:
All five Phase 4 clusters were clean — no patterns to work around.
One follow-up surfaced
The top-level helper `calculateAllComponentsShadowing` was used by both Core and the new `CompositeReplacementUpdater`. Moved it into `CompositeReplacementUpdater.ts` (the sole remaining caller).
Test plan
Final tally across the four-PR refactor
🤖 Generated with Claude Code