React.memo with generic types

If I create a component with a generic type like:

@react.component
let make = (~foo: 'a) {
   ...
}

It works fine, but if I wrap it in React.memo, the type system complains that:

This expression's type contains type variables that can't be generalized

I can’t seem to figure out how to annotate the type to make the type system happy.

Can you share a minimal repro? This code compiles for me in the ReScript Playground:

module Button = {
  @react.component
  let make = React.memo((~thing: 'a, ~getThing: 'a => unit) => {
    <button onClick={_ => getThing(thing)}> {React.string("wow")} </button>
  })
}

let foo = <Button thing=1 getThing=Js.log />

Played around with this a bit more and I am able to get a similar error if I’m not using the generic prop inside of my component. It goes away once used. Please share if you have a case where it’s used and you still see the error.

The ReScript compiler is very conservative when you have generic values next to mutation. Take the example code with error:

// trying to trick the type system into allowing this ref to be option<'a>
// this would allow for runtime type errors where we think foo.contents is something it isn't
let foo = ref(None)
foo.contents = Some(1)
// nope. This has type: string, Somewhere wanted: int
foo.contents = Some("haha I've fooled the type system")

But if I get trickier don’t use the ref right away, the compiler knows something is up:

// This expression's type contains type variables that can't be generalized
let foo = ref(None)

React internals use a bunch of mutation and ReScript is aware that might be the case. So when you have an unused generic in React, ReScript gets very nervous and lets you know that it’s uncomfortable with this message.

1 Like

The issue comes up when the component itself is only used in another file. So in essence it would be like your first example, but with the last line in another file. I’m not sure how to represent multiple files in the playground. Thanks for your insight.

1 Like

You can emulate multiple files by writing multiple wrapper modules in the playground.

@bdj so is this how the code looks like?

module Button = {
  @react.component
  let make = React.memo((~thing: 'a, ~getThing: 'a => unit) => {
    <button onClick={_ => getThing(thing)}> {React.string("wow")} </button>
  })
}
module Foo = {
  @react.component
  let make = () => {
    <Button thing=1 getThing=Js.log />
  }
}

PlayGround Link

My suggestion is to memoize the component only in the file you’re using it in, not in the definition of the original component. E.g.,

// Button.res

@react.component
let make = (~count, ~onCount) => {    
  React.useEffect0(() => {
    onCount(count)
    None
  })

  <button>{React.string("Click me")}</button>
}

// Test.res

module MemoButton = {
  include Button
  let make = React.memo(make)
}

let test = <MemoButton count=1 onCount=ignore />

This also has the nice effect of encouraging memoizing only when you actually need it.

3 Likes

Yes, and that will produce the error if they are in separate files (even though it doesn’t seem to produce an error on the Playground)

This seems like a fine solution, but what is wrong with memoizing always? It seems like the arguments against it are to not prematurely optimize if you don’t need to, but that seems like a pretty hand-wavy argument when you aren’t breaking any abstractions to do the optimization.

From the docs:

This method only exists as a performance optimization. Do not rely on it to “prevent” a render, as this can lead to bugs.

I am not relying on it to “prevent” a render; I do want the performance optimization. I just want to be able to specify the performance optimization once, not everywhere the component is used. The component in question for my use case is a Table component to display data, which I think should always be memoized on its props.

If you can provide a more detailed and specific code sample, perhaps we can suggest other approaches.

Here is what I want to work:

https://rescript-lang.org/try?code=C4TwDgpgBAxg9gGwK4FsB2AeA5AQwHxQC8UA3gBYQ4AmEATgFxQBKlMwAdBAhChGsABpYXBI1xECLHG07de-AL4AoJQAFarDvBRg4aPsCXdgUFDgDW0YlJm8UcABQOAflRzAcjHLVo4Q2fCFneGR0AGcvHz8MENRMXDw8AEoJUiUoKAwPACNuPHSMzOAKanzCwqzaMvLCkljwgpqoAFo8ACEuDgBBKJB2MzAHerRUrLICOsQ49hKaWgVMgHpi5Maa1psOb18QZSaMZaq1g+LKKmqMrOy4KhAL2rcPNYzWjoRu3v6cQdo4AHcJM8KsAjk1ypNQmgwkDyq9OuwejsvoNhqNgOdSL8-q1huwYCIFidzkkYYUNpp2Ns-HswSdQTUSU1ydItr0aZdltdbtUTjhchB8goSUA

So, in this example I’m not convinced that there’s much performance optimization to be had outside ‘preventing a render’, which we discussed above.

Anyway, another approach might be to use a functor to factor out the type parameter and instead pass it in (like dependency injection): https://rescript-lang.org/try?code=LYewJgrgNgpgBAFQIYCNZwLxwBQIJ4AOMAXHAMowAuAdAPIBOYM9MY+RAlJgHxwDeAKDhxKheAGMQUCMAB2mfgAsYSJvVIAlFeJoxYwGLMoAaOOL1RS7GNUo84WpDup6YBowF8BQuAAEWTjSSwAQgsoaUPrB2wEgA1vBYjs4GoNjYAH5gSJRIphmS0nIAzlwY3D7CADy5aDAVwo1wNcqqDU2NNfTtHY18hTKyxZW9cAC03ABCejQAgvT0SHjUsQTYA3L2Lbz9UoPUrWoezQD0lIrcHCO9E8k0SAtLXqOnlN3Xr4c9nZQo4HjfJp8bK5D7CCbTKBzR7LVbYeggADuPDBP3eLyBGyGqKaEJm1Hmi1hSDWWK2lDAOwRiImWOo5igUGOVTOlKuGMat209xhzwxLLel1RXMC1AeRL5TQFfzAAJGAtQsCFXgEJwAVAJQJB0GQ3gBLWQAc2QdQUJtg2F19ANhquAmiIhgxTsGB8VStNvNMBGIKQGAA2gAiJCBgC6IyxxQDfA9Rq9BxUak03OosmgUFMDMsDhTzutRo8obgJwqapO3lERBEClo4TgAB9EIiQA3EIoWN77VQzCAQg9EjhKABGUyUABMZV4xUReso4kUg5HIgn-B8jdw7ZgMFMCGb7NbuGbplrMEncCHa5wCE327bHf3693IB3e8v2BPx-CZ4ADJeAPr2GMF4qg6lBOi6PZ9iwG4dp+p5AA

The above shows creating the concrete component: module StringTable = Table(String). The Table functor just requires as an argument a module which has a type t and a function compare. E.g.:

type t = One | Two | Three

let compare = (t1, t2) => switch (t1, t2) {
  | (Three, One)
  | (Three, Two)
  | (Two, One) => 1
  | (Three, Three)
  | (Two, Two)
  | (One, One) => 0
  | _ => -1
}

You can instantiate one concrete Table component per actual type you want to use, which may save you a few definitions here and there. Not sure if the added complexity is worth it though.

Btw, you may have intentionally simplified the code to do this, but right now it will render the same cell in every column.

Wanted to add a bit more here - first on memoization in general. As @yawaramin has mentioned a couple times, it’s very hard to say that any memoization on a specific use will be worthwhile, let alone on a component for all of its uses. That said - if you are confident that all uses will benefit then you get even more control with useMemo as you can can define your equality checks and you also don’t run into this type issue. There is a slight perf loss, but in your case the expensive computation in the table should still be saved. This control is important as a naive memoization of your props is actually unsafe. Arrays can be pushed to dynamically so a === check in JS is not sufficient to determine whether two renders will return identical results. Additionally this allows you to put in dev-time checks for things like array identity changes where they are not expected.

I think the complexity of the functor is pretty high so I am hesitant to recommend that. I am a big fan of memoization at the callsite, but I also think useMemo could work in the right situation. That said, you know the project better than any of us :slight_smile:

3 Likes

Thanks for the reply. I think you guys have convinced me to memoize at the callsite if necessary, so good job changing my stubborn mind! I still find it interesting that the type system has trouble with the following code. Is it because there is the potential for foo to somehow change the underlying function is passed in as an argument, even though the type implies to me that it cannot change the underlying function?

FYI, there is a proposal going on explaining why relaxing value restriction is hard in a non pure language while remaining soundness https://github.com/ocaml/RFCs/pull/5

2 Likes