search

camdendotlol / topstersorg

Book and music charts
https://topsters.org
GNU Affero General Public License v3.0
18 stars 9 forks source link

Fix data loss when using Topsters 3 across multiple tabs #36

Open camdendotlol opened 7 months ago

camdendotlol commented 7 months ago

Closes #35

This was a suspiciously quick 10 minute change, will leave this PR unmerged until I can get a little more time to test out edge cases, etc.

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
topstersorg ✅ Ready (Inspect) Visit Preview Jan 31, 2024 1:10am
camdendotlol commented 7 months ago

Works great on Safari. Firefox and Chrome both go into an infinite loop:

  1. Tab A changes state.
  2. Tab B changes state to match, which triggers renderChart(), which writes the current chart state to localStorage.
  3. Tab A notices that Tab B set its localStorage value, and changes state to match.
  4. Tab B notices, and so on.

It seems like Safari has an efficiency check where it doesn't do anything if the old localStorage value equals the new value, while Firefox/Chrome don't have that. Big win for Safari here.

The way to fix this is for this particular mutation to not trigger a write to localStorage. We are, after all, just reading from localStorage in the wake of another tab's changes. I tried setting a new setEntireChartWithoutLocalStorage variant of setEntireChart and watching for that in ChartCanvas.vue, but it seems like another render is triggered shortly after that first one and writes to localStorage anyway. This points to a deeper issue in the chart Canvas logic that ties into #13 - the chart renders too many times.