Memoize isn't referentially transparent

[natefaubion]

memoize uses expandos on the state, which breaks referential transparency, and can cause some subtle bugs.

module Main where

import Prelude
import Control.Monad.Eff (Eff)

import Pux (start, CoreEffects)
import Pux.DOM.HTML (HTML, memoize)
import Pux.Renderer.React (renderToDOM)

import Text.Smolder.HTML as H
import Text.Smolder.Markup as HM

type User =
  { name :: String
  , age :: Int
  }

view1 :: ∀ ev. User -> HTML ev
view1 { name } =
  H.div do
    HM.text name

view2 :: ∀ ev. User -> HTML ev
view2 { age } =
  H.div do
    HM.text (show age)

view :: ∀ ev. User -> HTML ev
view user =
  H.div do
    memoize view1 user
    memoize view2 user

main :: ∀ fx. Eff (CoreEffects fx) Unit
main = do
  let user = { name: "Bob", age: 42 }
  app <- start
    { initialState: user
    , view
    , foldp: const <<< absurd
    , inputs: []
    }
  renderToDOM "#app" app.markup app.input

This example will render view1 twice because the render tree is tied to the user reference.

[natefaubion]

I think there are a few other issues to consider. With your fix, you construct a mutable closure after applying the render function. This will only work if you use memoize at the top level:

-- This will be memoized
view1 :: State -> HTML Event
view1 = memoize \st -> div >>= text "hello"

-- This will not be memoized
view2 :: State -> HTML Event
view2 st = div >>= memoize someView st

Consequently, usage of memoize in child will fail to cache because it constructs the memoized function every time you call it. You'd have to also push anything with child to the top-level as well. I'm not saying this isn't a fair trade-off, but it's worth documenting that if you expect to have memoized views, it must declared at the top-level (you can't use it inline). Elm thunks are similar, but because they can actually hook into the diff cycle, they can be applied after the fact. The views you apply the thunk to have the same limitation though (they must be top-level declarations) because it does a referential check on the render function you give it.

[natefaubion]

I think that there's also the issue that it memoizes the Smolder representation, which unfortunately does not translate to a referentially equal React representation, so while it appears to memoize if you trace it, it won't be memoized once you hit React (we had this same problem with the original Halogen). The only way to do it is to propagate a first class memo node of some kind to the rendering engine.

[damncabbage]

@alexmingoia: Do you mind re-opening this? It still seems to be an issue (see above).

[natefaubion]

I think the issue regarding referential transparency was fixed, since that could actually break rendering. @alexmingoia has made a lot of changes to how memoize works (since I made the above comments). If there are still problems, it probably warrants a new issue and test case.

[alexmingoia]

Sorry I accidentally closed this with my commit message. @damncabbage This has been fixed.

The memoized view will return the same HTML node, and the React renderer will cache any previously rendered node. So any view that was memoized should return the same node, and will use the cached React elements during render. The reason Pux provides its own memoize instead of purescript-memoize is because purescript-memoize is actually tabulation, which has very different performance characteristics than memoization. The Pux memoize is a simple closure that uses JS equality to cache the view function.

Because of PureScript's eager evaluation memoize needs to be used at the top of a view declaration, not inside a view. This could be made more obvious by memoize returning Lazy (st -> HTML ev) instead and making people use force. I don't like this API because you can't just add the memoize calls to the top of existing views without changing anything else.

I have my own benchmark that uses a large list and memoization with the caching by the React renderer performs similarly to using React classes, Redux, and shouldComponentUpdate.

[alexmingoia]

Somewhat related... the best thing for rendering performance would be for someone to write a virtual DOM library in PureScript that uses purescript-smolder. Then an intermediate representation like React or Snabbdom would be unnecessary.

[natefaubion]

I wrote purescript-halogen-vdom which is a native PS virtual-dom implementation (and we use it in Halogen and in production at SlamData). It was built with extensibility and performance in mind. It would be easy to implement a Smolder-like-Free-monad-newtype over it, though that always comes with a fairly significant overhead itself. In that case you could directly embed a memo-node like you'd have in Elm, and you'd also get things like map-fusion out of the box.

[i-am-the-slime]

Looks like it's back!

[damncabbage]

@i-am-the-slime Could you possibly elaborate, eg. with an example?