Skip to content

Docking: prevent the OnLiveLayoutChanged round-trip footgun (analyzer and/or API change) #439

@codemonkeychris

Description

@codemonkeychris

Background

The #1 mistake when wiring DockManager is round-tripping the host's live layout back into the Layout prop:

// ❌ anti-pattern — double-owns the shape
var (layout, setLayout) = UseState<DockNode?>(BuildLayout());
new DockManager {
    Layout = layout,
    OnLiveLayoutChanged = next => setLayout(next),
};

The host already owns the user's drag-modified shape internally (spec 045 §2.30) and resolves content by Key from manager.Layout each render. Feeding the live layout back double-owns the shape and produces a cluster of broken behavior, all at once:

  • drag-out-to-float works, but you can't drag back to re-dock
  • clicking tabs doesn't switch / selection resets
  • splitter drags snap back

This was hit firsthand while building the reactor-ide sample (PR #438). The correct pattern is: the app owns content (which docs are open), open/close changes manager.Layout's key set, reset is a .WithKey() remount, and OnLiveLayoutChanged is observation-only. That's now documented in the reactor-docking agent-kit skill (PR #438), but docs alone don't stop the mistake.

Goal

Make the round-trip mistake impossible — or at least caught at build time.

Option 1 — Analyzer REACTOR_DOCK_001 (cheap, non-breaking)

Flag OnLiveLayoutChanged = p => <call>(p) — a lambda assigned to OnLiveLayoutChanged whose body forwards its parameter straight into an invocation (i.e. a state setter). Warning severity, message pointing at the reactor-docking skill. Mirrors the existing MissingWithKeyAnalyzer (REACTOR_DSL_001) heuristic + test pattern in src/Reactor.Analyzers / tests/Reactor.Tests/AnalyzerTests.

  • Pro: cheap, no API churn, catches the common form.
  • Con: heuristic — can't prove the setter feeds Layout; a determined caller can still do it indirectly.

Option 2 — API-level prevention (stronger, breaking)

Change OnLiveLayoutChanged to hand back a read-only snapshot type that is not a DockNode (e.g. DockLayoutSnapshot with a ToJson()), so it can't be assigned to Layout — turning the footgun into a compile error.

  • Pro: makes the mistake structurally impossible.
  • Con: breaking change for existing observers. Mitigations: observers that want to persist/inspect use snapshot.ToJson(); layout persistence already has PersistenceId.

Proposal

Ship Option 1 now (low risk, immediate value); evaluate Option 2 for the next docking API revision. Open to doing both.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions