facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.65k stars 47.02k forks source link

[Compiler Bug]: Memoizing a debounced function complains about ref access #31290

Open RobinClowers opened 1 month ago

RobinClowers commented 1 month ago

What kind of issue is this?

Link to repro

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAgHgBwjALgAgBMEAzAQygBsCSoA7OXASwjvwFkBPAQU0wAoAlPmAAdNvjiswBQhAC2AUUoAlUvgC8+NWUYA6KGARqSQ8fknSClJjIDyUXAhgnN2hLtwGjJsxPOWdDL4APpgcDAQlJQAKhAAQhC4uApuhggAwmTRAEa6ANb8QpoAfCIBFnJKqqR6cLAwCHRe4ZHRcfxiEhYWKZjI+Db2js4mdQ1NLRFRlAASCEwA5gAWuAA0FT05CMtkAG4sMAOi4PIQScsnG934AL6CANwBt2v4ANoAuo8BUkEErTM4olkqktOl2AgzvxNsUNGViDloAwEPwwtN2gkkil5K8AIwABnxgmuPXeaLasUxIPkHxJ+G+dHEAUauFgbAAPIQmHsSvNohB8AB1HCUQjsgD0XJ5Tzot3EIFuQA

Repro steps

Attempting to memoize a debounced function that access a ref gives me this react compiler lint violation:

Ref values (the current property) may not be accessed during render.

However, the underlying function isn't called during render, it's simply referenced, and only called when the memoized, debounced function is called. In my case, it's called in an event handler, which is safe.

How often does this bug happen?

Every time

What version of React are you using?

18.3.1

josephsavona commented 1 month ago

Thanks for reporting! We'll have to think about how to handle this case.

const scrollToBottom = useMemo(
    () => debounce(_scrollToBottom, 100),
    [_scrollToBottom],
  );

What's happening here is that the compiler sees that _scrollToBottom is passed as an argument to a function (in this case debounce()). The compiler doesn't know what debounce does with that function, and has to assume that the function might immediately call its argument — might immediately call _scrollToBottom — which would access a ref during render. So we report an error.

One option would be to try to recognize common functions like debounce, which are known to return but not call their arguments.

RobinClowers commented 1 month ago

Yeah, that's more or less what I figured. Debounce in particular seems like a common enough pattern that it would be worth a special case? Currently I'm just suppressing the error, and I'm not actually using the compiler yet, but I'm curious, what would the compiler do here? I'm guessing it also detects this and bails out of optimization on this component?

josephsavona commented 1 month ago

what would the compiler do here? I'm guessing it also detects this and bails of of optimization on this component?

Exactly, the compiler would skip memoizing the component.

guillaumebrunerie commented 1 month ago

I might be misunderstanding the example but why do you call debounce in a useMemo? What's the return value and what are you doing with it? Isn't it useEffect that you want?

RobinClowers commented 1 month ago

debounce is a higher order function, it returns a new function that has the debounce behavior. Since we pass it as a prop, we don't want to create a new reference every render.