Repository navigation
fix(settings): pick up changes made in another tab - #76
Merged
Merged
Conversation
Every pc: key is shared by all tabs of the console, but each tab read them once at mount and never listened for the storage event. Lowering retention in one tab left another pruning the same IndexedDB to the old limit until it was reloaded, so the tab with the higher limit kept counting rows the other had already evicted. - subscribeStored() listens for storage events on given keys (a null key is localStorage.clear()); events from sessionStorage are ignored - useSettings reloads and re-validates all four settings on any of their keys, and keeps the old object when nothing changed so effects keyed on settings do not restart - usePersistentState picks up its key the same way, which covers pc:metricsSort, and useTheme now builds on it for pc:theme. The OS preference is still queried only when nothing is stored - A changed limit takes effect from the next batch: the trace store reads its settings when it evicts, so an eviction already under way finishes on the limit it started with. A new retention-limit scenario pins that - 10 new tests; with the three hooks reverted, 5 of the 9 cross-tab tests fail (the other 4 are guards that pass either way) - Checked with two browser tabs: a write in one switched the other's theme and showed retention 120000 / factor 4x in its settings dialog without a reload - Gates: lint clean (same 7 warnings as main), 212 tests pass Tags: #settings #storage Co-Authored-By: htjulia <htjulia1@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every
pc:key is shared by all tabs of the console, but each tab read them once at mount and never listened for thestorageevent. Lowering retention in one tab left another pruning the same IndexedDB to the old limit until it was reloaded, so the tab with the higher limit kept counting rows the other had already evicted.pc:retention,pc:evictionFactoruseSettingspc:pageSize,pc:sampleDelayMsuseSettingspc:metricsSortusePersistentStatepc:themeuseThemeWhat
subscribeStored(keys, onChange)inuse-persistent-state.tslistens forstorageevents on those keys. Anullkey meanslocalStorage.clear(), and events fromsessionStorageare ignored.useSettingsreloads and re-validates all four settings on any of their keys. When nothing changed it keeps the old object, so effects keyed onsettingsdo not restart. A save writes four keys and can arrive as four events; each one re-reads everything, so the tab ends up with the last value of each.usePersistentStatepicks up its key the same way, which coverspc:metricsSort.useThemenow builds on it forpc:theme, and the OS preference is still queried only when nothing is stored.When a changed limit applies
From the next batch. The trace store reads its settings at eviction time, so an eviction already in progress finishes on the limit it started with. A new
retention-limitscenario pins this: the limit drops from 8 to 6 between batches and the next arrival prunes to 6.Verification
120000and factor4×, with no reload.npm run lint: 7 warnings, the same set asmainnpm test: 18 files, 212 tests pass