search

facebook / react

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

Why error when rendering more hooks if they maintain order? ...and more #17446

Closed ruifortes closed 4 years ago

ruifortes commented 4 years ago

Hi.

Just wondering what is the problem to allow a component to render "less" hooks that maintain order.

I ask because I've been using (although not very convincingly) wrapping some components to allow bailing earlier by returning undefined. The wrapper would just return the last value in that case.

I know I could just useMemo everywhere but sometimes having to provide default values or adding extra conditionals for some edge cases adds some noise to the code.

Is there any reason against this method?

Thanks

miraage commented 4 years ago

https://reactjs.org/docs/hooks-rules.html

ruifortes commented 4 years ago

I understand that the order of the hooks "must" be kept. The question is about the need to call "all" of then.

miraage commented 4 years ago

"Explanation" section gives the answer. Just read it carefully.

ruifortes commented 4 years ago

Has I said, I understand the "React relies on the order in which Hooks are called" part. And I've been using a render bail mechanism that uses the fact that as long has the order of hooks is maintained there's no need to call them all. Of course I may be wrong but (and so could you) I would appreciate if you could point to where exactly is the need to "call them all" addressed cause I couldn't find it.

I just created an issue in the react RFC repo https://github.com/reactjs/rfcs/issues/131

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

miraage commented 4 years ago

@ruifortes it seems like you may have missed the point of "React relies on the order in which Hooks are called". Let's say you have 4 hooks, one of them is called conditionally. If one of them is missing, how could React know which one is missing exactly? No way React would now it. The same goes if hooks were called in a different order. https://www.netlify.com/blog/2019/03/11/deep-dive-how-do-react-hooks-really-work/ https://medium.com/the-guild/under-the-hood-of-reacts-hooks-system-eb59638c9dba

Dergash commented 4 years ago

Hello @ruifortes! There is indeed a reason behind this specific "early bailout" / "early return" rule which is explained here:

We could in theory support them. However we’ve chosen intentionally against that because semantics get very confusing.

What would happen to state below? Would it be reset or preserved? What about effects? Would they be cleaned up (like on unmount) or would they run normally?

If you think about it you’ll find cases that are very confusing regardless of the behavior you pick. Therefore, we disallow this pattern altogether. Put early return after calls to Hooks.

ruifortes commented 4 years ago

I don't understand what problem arise from "early return".

Nevertheless I think it's an elegant soluction to at least have the option to enable it. It simply solves "shouldComponentUpdate" and redux/context "selectors"

I think it might also allow more concise code in cases where state changes might be incrementaly applied before final output update. I don't exactly know what I'm talking about here but imagine something like "Pregel" that updates a node state in multiple steps until some threshold before returning a result.

Not that you can't do it now but "early return" you might avoid some, maybe a lot) conditionals and complex memoization props.

The rules would just be to keep the hooks order and hooks not just keep the same internal value. Also "early returning" null, as oposed to undefined that would alter the renderer element, to remove it seam nice.

If someone can convince me eitherway would be great. Can someone provide an example where it breaks?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

stale[bot] commented 4 years ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!