refactor(graph): group and test the properties that cannot move to a mixin - #1131

First implementation step of the plan recorded in ADR 0003 (#1130). No behavior change for library users.

Problem

mixInto installs mixin members on the AbstractGraph prototype. A property with a mutable default declared in a mixin is therefore shared by every graph instance: mutating it on one graph mutates it for all of them. There is no compile error, and until this PR, no failing test either.

AbstractGraph already has a dedicated group for the properties concerned, Variables that should be in the mixins but requiring per-instance initialization. That group was neither complete nor guarded.

Changes

pageFormat and warningImage join the group. Both have exactly that constraint, yet were declared in the generic Variables managed here group, so nothing at the declaration said why they must stay. The source diff is a pure relocation, 20 lines out and the same 20 lines in.

Every property of the group is now covered by a test, in the existing Expect no global state for properties coming from mixins block: two graphs, distinct references, then mutate one and check the other is untouched. Each case also asserts that its own mutation is effective, so a case cannot pass by accident with a mutation that does nothing.

Verified that the tests can actually fail, by temporarily sharing pageFormat between instances the way a mixin would: only that case failed, the other six passed.

The group header now points at these tests, so adding a property there without its case becomes an obvious omission rather than a silent one.

sizeDidChange gets a comment explaining why it cannot be extracted. It sits in EventsMixin but reads container-sizing members only, so it looks like an obvious candidate for a move, and the question would otherwise be re-opened by every contributor who spots the mismatch. AbstractGraph calls it from its constructor, from graphModelChanged and from refresh, none of which can depend on an optional plugin. Written as a plain comment rather than JSDoc on purpose: a reader of the public API has no use for a refactoring constraint.

One consequence worth reviewing

Declaration order drives the order of the child elements produced by the Codec export, so moving the two properties moved their elements in the exported XML. The expected XML of all-graph-classes.test.ts is updated accordingly, and the changelog records it.

This is not breaking: decoding matches elements by their as attribute and is order-independent, existing documents decode identically, and previously exported documents stay valid. It is recorded only for consumers comparing exported XML as text, for instance in golden-file tests.

The changelog entry sits under a new Other Changes heading, since the Unreleased section is documented as holding breaking changes only. Happy to drop it if you would rather keep that section strictly for breaking changes.

Validation

Run locally on Node 24 (.nvmrc): build, test-check, the full suite (505 tests, 60 suites), lint, check:circular-dependencies. All passing.

Summary by CodeRabbit

• Bug Fixes

  • Fixed graph settings so changes to one graph no longer unintentionally affect other graph instances.
  • Ensured graph-specific page formats and warning images remain independently configurable.

• Serialization

  • XML exports now place pageFormat and warningImage immediately after options for graphs.
  • XML imports remain compatible regardless of element order.

• Documentation

  • Updated the changelog with the XML export ordering change.