Skip to content

Evaluate and eliminate remaining 3 effect blocks in BaseNodeViewer #653

Description

@malibio

Overview

Evaluate whether the 3 remaining $effect blocks in base-node-viewer.svelte can be eliminated or must remain as legitimate side effects.

Background

Issue #628 successfully reduced effects from 9 → 4 → 3 by:

  • Converting state to $derived (viewerPlaceholder, currentViewedNode)
  • Moving async loading from $effect to onMount (leverages {#key} component recreation)
  • Eliminating redundant watchers

Remaining Effects

1. Content Save Watcher (Line ~437) - $effect.pre

Purpose: Automatically saves node content to database when users edit

Why it might be legitimate:

  • Must run BEFORE DOM updates ($effect.pre)
  • Performs side effects (database writes)
  • Coordinates ancestor persistence to prevent FOREIGN KEY errors
  • Detects new vs existing nodes for immediate vs debounced saves

Potential elimination:

  • Move save logic to event handlers (on:input, on:blur)
  • Use store subscriptions instead of effect watching
  • Evaluate if this duplicates work already done by updateNodeContent()

2. Node Deletion Detection (Line ~532) - $effect

Purpose: Detects when nodes are deleted and cleans them up from database

Why it might be legitimate:

  • Compares previous vs current state to detect deletions
  • Performs side effects (API calls to delete nodes)
  • Coordinates with structural watcher to prevent CASCADE errors

Potential elimination:

  • Move to explicit event handlers for delete operations
  • Use store subscriptions for deletion detection
  • Evaluate if deletion should be explicit action vs implicit detection

3. Structural Change Tracker (Line ~626) - $effect.pre

Purpose: Tracks node hierarchy for cleanup and persistence coordination

Why it might be legitimate:

  • Must run BEFORE DOM updates ($effect.pre)
  • Maintains tracking Sets for deletion detection
  • Coordinates cleanup for removed nodes

Potential elimination:

  • Move structural tracking to the store layer
  • Use explicit hierarchy management instead of watching
  • Evaluate if this duplicates work done elsewhere

Investigation Questions

  1. Do these effects duplicate work?

    • Is content saving already handled by updateNodeContent()?
    • Is deletion already handled by explicit delete handlers?
    • Is structure tracking already done by the store?
  2. Can we use event-driven patterns instead?

    • Can content saves trigger on explicit events (blur, debounced input)?
    • Can deletions be explicit actions rather than detected changes?
    • Can structure changes be tracked via store subscriptions?
  3. Should this logic live in the component?

    • Could content coordination move to the store?
    • Could deletion detection move to a service?
    • Could structure tracking be centralized?

Acceptance Criteria

  • Each effect analyzed for necessity
  • Alternative patterns evaluated (events, subscriptions, derived state)
  • Decision documented for each effect (keep vs eliminate)
  • If eliminated: refactored with tests passing
  • If kept: documented why it's legitimately necessary
  • Architecture improved (effects in component vs store/service)

Success Metrics

Best case: 0 effects (all moved to better patterns)
Realistic: 1-2 effects (truly necessary side effect coordination)
Documentation: Clear explanation of why remaining effects can't be eliminated

Related

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions