Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camdendotlol
Copy link
Owner

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.

Copy link

vercel bot commented Jan 31, 2024

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
Copy link
Owner Author

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.

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.

Bug: Opening 2 tabs of Topsters 3 can result in Data loss
1 participant