You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
One reason our state management is complex is because we have:
a) Many sources of state (which is unavoidable, per our requirements)
b) State merging happening in multiple, disconnected places. We have a waterfall of state merges.
To simplify, I’d propose we merge the state together in one canonical place, instead of the current waterfall approach.
I count these sources of state (in increasing order of priority):
Rill-opinionated default
developer-specified preset
user time zone preference
home bookmark
magic token state
local storage ("last viewed state")
session storage
URL query parameters
Some discrete issues:
Multiple, disconnected state merges: getDefaultExplorePreset() does a subset of state merge: it merges the Rill-opinionated default, the developer-specified default, and the user's time zone preference . Then convertURLToExplorePreset() does another subset of state merge: it merges the defaultExplorePreset with URL parameters. These functions encompass two state merges, yet at some other point we also merge state from bookmarks, local storage, and session storage. It's hard to track.
Conversions into ExplorePreset: We should not be using the ExplorePreset type as a type for intermediate state handoffs. convertSourceXToExplorePreset() shouldn’t be necessary. We should only have to convert state sources into Partial<ExploreState>.
Suggestions:
Replace convertSourceXToExplorePreset(), getExplorePresetForSourceX() functions with getPartialStateFromSourceX() functions. Colocate these functions.
Add one canonical merge, e.g. const state = mergeState(sourceXPartialState, sourceYPartialState, ...)
In the merge function, clearly list sources in priority order from lowest (Rill defaults) to highest (URL params)
The text was updated successfully, but these errors were encountered:
Follow-up to #5675
One reason our state management is complex is because we have:
a) Many sources of state (which is unavoidable, per our requirements)
b) State merging happening in multiple, disconnected places. We have a waterfall of state merges.
To simplify, I’d propose we merge the state together in one canonical place, instead of the current waterfall approach.
I count these sources of state (in increasing order of priority):
Some discrete issues:
getDefaultExplorePreset()
does a subset of state merge: it merges the Rill-opinionated default, the developer-specified default, and the user's time zone preference . ThenconvertURLToExplorePreset()
does another subset of state merge: it merges thedefaultExplorePreset
with URL parameters. These functions encompass two state merges, yet at some other point we also merge state from bookmarks, local storage, and session storage. It's hard to track.ExplorePreset
: We should not be using theExplorePreset
type as a type for intermediate state handoffs.convertSourceXToExplorePreset()
shouldn’t be necessary. We should only have to convert state sources intoPartial<ExploreState>
.Suggestions:
convertSourceXToExplorePreset()
,getExplorePresetForSourceX()
functions withgetPartialStateFromSourceX()
functions. Colocate these functions.const state = mergeState(sourceXPartialState, sourceYPartialState, ...)
The text was updated successfully, but these errors were encountered: