Now that we are not dependent on routeFromModel
I was thinking: Maybe we should do a rethink of the way we store models in admin only.
I think caching is a performance/complexity tradeoff that requires reasoning and a conscious decision.
Right now we have all the page models stored as Maybe SomeModel
in a large record, we literally cache everything in every model for every page, even pages that have nothing we would like to cache.
I think we should rethink this and come up with an opt-in way of caching.
- Whenever the url changes we have to check whether there is already a cached model for the page or not.
updatePage ... =
case route of
Route.SomePage ->
case model.someModel of
Nothing ->
-- We don't have a cached model yet
SomePage.init
Just someModel ->
-- We already have a model so
SomePage.updatePage someModel
This is first of all boilerplate that repeats over and over and over for every page, but second of all, it has some additional problems like this:
We have a model for the UserStatsPage
and when we leave that page, we don't throw away that model.
Now when we visit that page again for a diffent user, instead of reinitializing, we use the model from the previous user.
To prevent this kind of stuff we have to put checks like this in the codebase.
updatePage ... =
if model.uid /= uid then
init embed uid route
else
..actual update page..
This requires us to keep state of the user id.
- We have to use
Maybe.map
to update the page model.
Cmd.pure { model | someModel = Maybe.map (SomePage.update msg_) model.someModel }
This is not a huge deal, but it's everywhere. It would definitely be cleaner if we were able to get rid of this.
Maybe there are some other places where this kind of caching makes things more tricky.
To get rid of caching page models completely, let's take a look at a sum type approach for page models.
The code could look like this
type Model
= FooPage FooPage.Model
| BarPage BarPage.Model
init : (Msg -> msg) -> Route -> (Model, Cmd msg)
init embed route =
case route of
Route.Foo fooParams ->
FooPage.init (embed << FooMsg) fooParams
Route.Bar ->
BarPage.init (embed << BarMsg)
update : (Msg -> msg) -> Route -> Msg -> Model -> (Model, Cmd msg)
update embed route msg model =
case (route, msg, model) of
(Route.Foo fooParams, FooMsg fooMsg, FooPage fooModel) ->
Tuple.mapFirst FooPage <|
FooPage.update (embed << FooMsg) fooParams fooMsg fooModel
(Route.Bar, BarMsg barMsg, BarPage barModel) ->
Tuple.mapFirst BarPage <|
BarPage.update (embed << BarMsg) barMsg barModel
_ -> Cmd.pure model
updatePage : (Msg -> msg) -> Route -> Model -> (Model, Cmd msg)
updatePage embed route model =
case (route, model) of
(Route.Foo fooParams, FooPage fooModel) ->
Tuple.mapFirst FooPage (FooPage.updatePage (embed << FooMsg) fooParams fooModel)
(Route.Bar, BarPage barModel) ->
Cmd.pure (BarPage barModel)
_ ->
-- The route doesn't match with the current page the model is on.
init embed route
view : (Msg -> msg) -> Route -> Model -> Html msg
view embed route model =
case (route, model) of
(Route.Foo fooParams, FooPage fooModel) ->
FooPage.view (embed << FooMsg) fooParams fooModel
(Route.Bar, BarPage barModel) ->
BarPage.view (embed << BarMsg) barModel
_ -> Html.text "Something went wrong. Please contact the elm team!"
I think this would be an improvement to the readability of the code becaude there's a lot less boilerplate and the boilerplate that is there is simpler.
To reintroduce the caching functionality, I think we should add a new kind of object called the Cache
that only contains things we have consciously decided to cache. It's going to be an argument to init
functions that explicitly say they want to try to derive something from the Cache
and each page that wants to cache some of their model will define a new cache : Model -> Cache -> Cache
function that will be derive the new cache from the model whenever the page is left.
All the details of the Cache
I want to leave open, also the principles to decide what to cache and what not to cache need to be formed, but I think this would be a major simplification of the codebase.
I like the idea in general, I'm little bit worried about loosing performance for the "tabbed" detail view as we would need to load every tab when we would like to switch to it (and some can take quite a while for sure) :(
Regarding the explicit caching, I think that we would end up with something like Store pattern in the end and we will end up with
RemoteData
states instead ofMaybe
(or maybeMaybe
will be enough there)So I'm little bit torn apart here, I definitely agree that we don't need caching on every level - e.g. the top one. But it is quite handy to have it on the "Detail" level. Also I remember how I hated that we were hitting BE on each tab switch inside the Statistics page even though nothing could change in between those two.