facebook / react

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

RequestUpdateLane render phase update comment says not supported #31633

Open stuartkeith opened 6 days ago

stuartkeith commented 6 days ago

Apologies for not following the template but this isn't a bug but also isn't about docs on react.dev - it's about a comment in the code.

While debugging a render phase setState call, I noticed requestUpdateLane has a comment about render phase updates not being officially supported:

https://github.com/facebook/react/blob/7670501b0dc1a97983058b5217a205b62e2094a1/packages/react-reconciler/src/ReactFiberWorkLoop.js#L673-L682

However the docs recommend using a render phase update to adjust state when props change: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

function List({ items }) {
  const [isReverse, setIsReverse] = useState(false);
  const [selection, setSelection] = useState(null);

  // Better: Adjust the state while rendering
  const [prevItems, setPrevItems] = useState(items);
  if (items !== prevItems) {
    setPrevItems(items);
    setSelection(null);
  }
  // ...
}

Is this comment out of date?

eps1lon commented 3 days ago

The comment is probably outdated. Can you find the original PR when it got introduced (the current blame points to a PR which likely just moved the code).

stuartkeith commented 2 days ago

The PR is #18850, specifically the commit https://github.com/facebook/react/commit/47ebc90b08be7a2e6955dd3cfd468318e0b8fdfd

eps1lon commented 2 days ago

Thanks for digging up the PR. The comment refers to render phase updates of different Components which is not supported and issues a warning. E.g. when you pass a setState to a different Component and the other Component calls the setState. This would trigger "Cannot update a component (ExampleComponent) while rendering a different component".

Render phase updates are only supported if you update the same Component.

stuartkeith commented 2 days ago

Thanks for digging up the PR. The comment refers to render phase updates of different Components which is not supported and issues a warning. E.g. when you pass a setState to a different Component and the other Component calls the setState. This would trigger "Cannot update a component (ExampleComponent) while rendering a different component".

Render phase updates are only supported if you update the same Component.

Thanks for getting back to me. I have made this codepen: https://codepen.io/stuartkeith-the-selector/pen/mybyqdd

This is a scenario similar to https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes:

const [counter, setCounter] = React.useState(0);

const [lastCounter, setLastCounter] = React.useState(counter);

if (counter !== lastCounter) {
  setLastCounter(counter);
}

Both counter and lastCounter are in the same component.

When I start debugging at setLastCounter, I get to this part of the code early on:

https://github.com/user-attachments/assets/171bbcd9-e53a-4c64-a652-a99ba26e3507

There's no warning there (or any problem with the behaviour/output). I have encountered the warning that happens when the setState is called in another component in the past, but unless I'm missing something, this is a different scenario.

I also tested with a child component syncing from a prop and hit the same code path: https://codepen.io/stuartkeith-the-selector/pen/GgKgOZJ