jorgebucaran/hyperapp

An apparent bug with `memo()` ?

icylace opened this issue · 4 comments

I've come across what appears to be a bug when using memo():

Try it out on Flems

So, the main idea here is the Increment action fiddles with data related to the memoized view but the memoized view does not re-render.

The reason is because of the current way memo props are checked for changes. Only index-for-index comparisons are considered, so any indexable type, like strings and arrays, can be compared with one another and potentially be considered "equal" when it comes to determining if a re-render should happen.

So, now the question is:

Is this valid (albeit nonstandard/silly) usage and therefore a true bug or is this instead invalid usage and therefore just some curious behavior ?

import { h, text, app, memo } from "https://unpkg.com/hyperapp"

const randomHex = () => "0123456789ABCDEF"[Math.floor(Math.random() * 16)]
const randomColor = () => "#" + Array.from({ length: 6 }, randomHex).join("")

const listView = (list) =>
  h("p", {
    style: {
      backgroundColor: randomColor(),
      color: randomColor(),
    },
  }, text(list))

const MoreItems = (state) => ({
  ...state,
  list: Array.isArray(state.list)
    ? [...state.list, randomHex()]
    : state.list + "" + randomHex(),
})

const Increment = (state) => ({
  ...state,
  counter: state.counter + 1,

  // The following should cause the memoized view to rerender but it doesn't.
  list: Array.isArray(state.list)
    ? state.list.join("")
    : state.list.split(""),
})

app({
  init: {
    list: ["a", "b", "c"],
    counter: 0,
  },
  view: (state) =>
    h("main", {}, [
      h("button", { onclick: MoreItems }, text("Grow list")),
      h("button", { onclick: Increment }, text("+1 to counter")),
      h("p", {}, text(`Counter: ${state.counter}`)),
      h("p", {}, text("Regular view showing list:")),
      listView(state.list),
      h("p", {}, text("Memoized view showing list (unaffected by the `Increment` action!):")),
      memo(listView, state.list),
    ]),
  node: document.getElementById("app")
})

I'm going with curious behavior here. Changing the type of the prop seems dangerous too.

Given your wording, do you imply objects should always be used for the memo data ?

I mean changing the type of the property. You're going from array to string and back.

Primitive types are generally fine. 👌

Thanks, good to know !