fix(preferences): make restore() reactive by using TrackedMap #149
Conversation
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
The `plugins` and `columns` Maps in TrackedPreferences were regular Maps, causing restore() to silently fail to trigger reactivity. Plugins with @cached getters (like ColumnVisibility.visibleColumns) would not update when preferences were restored. Changes: - Change `plugins` and `columns` from Map to TrackedMap - Add non-mutating getters (getPlugin/getColumn) for read operations to avoid "update during render" errors - Update preferences.forColumn().get() and forTable().get() to use non-mutating getters Behavior change: Empty plugin preferences are no longer persisted. Previously, reading preferences (without writing) would create empty entries that were then serialized. Now only plugins that actually write preferences data will appear in the persisted output.
Contributor
Author
|
this can be closed if #150 is merged. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes reactivity issue where
restore()on preferences would not trigger updates in plugins with@cachedgetters (likeColumnVisibility.visibleColumns).Problem
TrackedPreferences.pluginsandTrackedPluginPrefs.columnswere regularMaps. Whenrestore()replaced entries, cached getters continued watching the oldTrackedMapinstances and never invalidated.## Solution
TrackedMapforpluginsandcolumnscontainers so.set()triggers trackinggetPlugin,getColumn) for read/delete paths to avoid Glimmer's "mutation during render" error## Test changes
Updated column-reordering tests to remove
ColumnVisibilityfrom expected persisted data. Previously, just reading a plugin's preferences would create empty entries that got serialized. Now only plugins that write data appear in persisted output—this is the correct behavior since empty preferences shouldn't be persisted.