Skip to content

Instantly share code, notes, and snippets.

@stefan-wullems
Last active June 9, 2022 11:59
Show Gist options
  • Save stefan-wullems/58fad5f5e715aae11f009901341c762b to your computer and use it in GitHub Desktop.
Save stefan-wullems/58fad5f5e715aae11f009901341c762b to your computer and use it in GitHub Desktop.

Caching models rethink

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.

Everything from every page is cached

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.

Boilerplate and complexity caused by the everything from every page is cached approach

  1. 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.

  1. 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.

Cacheless

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.

Explicit caching

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.

@kraklin
Copy link

kraklin commented Jun 9, 2022

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 of Maybe (or maybe Maybe 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment