Skip to content

refactor: [2322] Dashboard data store - Move chart series into a class + add utils file - #2330

Open
n-lark wants to merge 4 commits into
2322-data-store-chartsfrom
2322-data-store-series
Open

n-lark wants to merge 4 commits into
2322-data-store-chartsfrom
2322-data-store-series

Conversation

@n-lark

@n-lark n-lark commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Description

This is a refactor that moves utilities into their own files, so reactive.js stops growing, and moves chart series into a class:

  • a chart's points, stored messages and history live in one object, with the rules that keep them in step next to them
  • state sits in named fields instead of closures passed between functions, so checks like "is this proxy still current" are a method you can read
  • flow writes to charts (next PR) need per-chart state and their own proxy traps, which fit on this class

No new functionality or behavior changes. Focused on readability and preparing for flow writes.

Review tip: git diff --color-moved shows the moves as moves. Commits 1 and 2 are moves; commit 3 is the class.

Test plan

  • New specs pin behaviorr that already held and the class must keep: history across value/chart switches, an old value reference going quiet, a failed copy leaving the key unchanged, a datapoint with 200,000 points
  • 1,500 seeded random runs (about 225,000 operations) give identical results, onChange logs and errors on the old and new store
  • Breaking each new seam fails a spec
  • Chart benchmarks within noise of feat: [2322] Dashboard data store - Chart performance #2324, except the 1,000-point time window (about 44 → 50 µs), a V8 inlining difference in the benchmark loop rather than extra work

Related Issue(s)

Part of #2322. Based on #2324.

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@n-lark
n-lark requested a review from cstns October 22, 2026 20:47
@n-lark n-lark self-assigned this Oct 22, 2026
@n-lark
n-lark added this pull request to stack #2331 October 22, 2026 20:50

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant