Report misuses of Html.Lazy

[jfmengels]

What the rule should do:

Detect cases where Html.Lazy functions will miss the cache.

What problems does it solve:

Html laziness is a very hard thing to get right, and a very easy thing to break afterwards. I found the best way to discover whether I used Html.Lazy right, since it is just an optimization and having it work or fail won't make a visual difference and won't be noticeable unless the page is very "heavy", is to put Debug.log calls in the lazy-ed function, and checking whether I see it printed out a lot.

After I am done making it work, I remove the Debug.log because they can't stay in my production code, and then I don't have any safety net that makes sure my setup won't break again, which it can easily do since laziness is fragile.

Example of things the rule would report:

Note that this rings true for all variants of lazy: lazy, lazy2... lazy9. We could also report for elm-ui's and elm-css lazy modules either out of the box or through configuration (there are other alternatives, and we probably don't want to hardcode them all, plus maybe it will be expensive computation-wise).

The lazy-ed function's reference must be the same

-- BAD
view value = 
  Html.Lazy.lazy (viewAddition 100) value -- Computing a new reference for every call of `view`

viewAddition a b =
  text <| String.fromInt (a + b)

-- GOOD
view value = 
  Html.Lazy.lazy2 viewAddition 100 value

viewAddition a b =
  text <| String.fromInt (a + b)

The references to the arguments must be the same

-- BAD
view list =
  let
    -- Computing a new reference for every call of `view`
    transformedList = List.map identity list
  in
  Html.Lazy.lazy viewHelper transformedList

viewHelper list =
  -- show something

-- GOOD
view list =
  Html.Lazy.lazy viewHelper list

viewHelper list =
  let
    transformedList = List.map identity list
  in
  -- show something

-- GOOD
view : List Int -> Html msg
view list =
  -- Identity doesn't create a new reference, so we need to detect
  -- which functions create new references and which don't
  Html.Lazy.lazy viewHelper (identity int)

-- GOOD
view : Int -> Html msg
view int =
  -- Primitives are compared by value, not by reference, so this is okay
  Html.Lazy.lazy viewHelper (int + 5)

-- GOOD
type Enum = EnumValue
constant = []
view : Model -> Html msg
view model =
  -- Constants are okay
  Html.Lazy.lazy3 viewHelper model.value constant EnumValue

-- BAD
view _ =
  let
    -- Creates new reference
    newRecord = {}
  in
  Html.Lazy.lazy viewHelper newRecord

Ideally, we should look further up the call chain (until we reach main) to see if a new reference is created

-- BAD
view list =
  let
    transformedList = List.map identity list
  in
  viewThatWillUseLazy transformedList

viewThatWillUseLazy list =
  Html.Lazy.lazy viewHelper transformedList

Values must not have been destructured

This was very surprising to me, but @Arkham talked about it in https://juliu.is/performant-elm-html-lazy/ and from trying it out, seems to be true (but I have no clue why).

-- BAD
view {value} =
  Html.Lazy.lazy viewHelper value

-- GOOD
view params =
  Html.Lazy.lazy viewHelper params.value

When (not) to enable this rule:

This rule won't be useful if you don't use Html.Lazy anywhere.

I am looking for:

• Feedback
• Fact-checks on what I said about when laziness would be defeated, and find other things we should report
• Someone to implement it (with or without me. If this works out well, I guarantee you glory!)

I am sure this is a very complex problem to solve compared to most ones out there. I think a good beginning would be to start detecting when the lazy-ed function is being recomputed everytime, and to support more complex cases iteratively. In this case, I think that false negatives are okay (maybe inevitable here?), but we really don't want false positives in elm-review.

I also would like to figure in which package(s) this would fit best. I originally thought it could go in something like elm-review-performance (along with rules that check that tail-call optimization is working alright), but it might make more sense in something like elm-review-html, though I wouldn't know with which rules.

Side-note: Detecting this problem has been one of my long-standing goals when working on elm-review. I would love this one to be implemented ❤️

[MartinSStewart]

I've run into code at work that misuses Html.lazy so this looks like a useful rule to have.

One suggestion would be to also check for Element.lazy if the user has a dependency to elm-ui.

[gampleman]

Also common gotcha worth detecting:

-- BAD
view model =
     Html.lazy (\data -> text data) model

-- BAD
view model =
      let
          helper data =
              text data
      in
      Html.lazy helper model

-- Good

helper data =
    text data

view model =
    Html.lazy helper model

[Arkham]

Hello! Just popping up here to mention I was wrong in the article about destructuring records (mentioned in the Values must not have been destructured rule). The problem was with constructing records on the fly instead! I've updated the blog post with this section:

---

Constructing records on the fly

A similar problem happens when we construct a record on the fly and pass it to Html.Lazy. Imagine our viewPrime looked like this:

viewPrime : { limit : Int } -> Html Msg
viewPrime { limit } =
    text
        ("There are "
            ++ String.fromInt (sieve limit |> List.length)
            ++ " prime numbers between between 2 and "
            ++ String.fromInt limit
        )

And now we called it by writing:

[ Html.Lazy.lazy viewPrime { limit = 200000 } ]

You will see the app going back to being slow again. This happens because when we build a new record we invalidate the caching mechanism of Html.Lazy.

---

Sorry for the confusion!