Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: react got runtime error when user breaks the rules of hooks, instead of telling user what to do #28165

Closed
RexSkz opened this issue Jan 31, 2024 · 4 comments

Comments

@RexSkz
Copy link

RexSkz commented Jan 31, 2024

React version: 18.2.0

Steps To Reproduce

  1. Open the link below, it will show a page with a button.
  2. Click the button, and you will get the error.

Link to code example: https://codesandbox.io/p/sandbox/a-react-hooks-edge-case-that-causes-react-crash-rddmkh?file=%2Fsrc%2FApp.js%3A1%2C1

The current behavior

Users get a runtime error Cannot read properties of undefined (reading 'length').

image

This is because React uses different data structures in memoizedState, e.g. object for useEffect, array for useCallback. If users break the rules of hooks, the following code will crash, which will confuse users:

function updateCallback(callback, deps) {
  ...
    if (nextDeps !== null) {
      var prevDeps = prevState[1];                  // <- 1. prevState is now an object

      if (areHookInputsEqual(nextDeps, prevDeps)) { // <- 2. passes an undefined here
        return prevState[0];
      }
    }
  }
  ...
}

function areHookInputsEqual(nextDeps, prevDeps) {
  ...
    if (nextDeps.length !== prevDeps.length) {      // <- 3. this will crash - prevDeps is undefined
      error('The final argument passed to %s changed size between renders. The ' + 'order and size of this array must remain constant.\n\n' + 'Previous: %s\n' + 'Incoming: %s', currentHookNameInDev, "[" + prevDeps.join(', ') + "]", "[" + nextDeps.join(', ') + "]");
    }
  }
  ...
}

The expected behavior

Users should receive a React message like other scenarios that break the rules of hooks (Rendered more hooks than during the previous render.).

Actually, if we modify the last hook from useCallback to useEffect, we can get this message.

image

ESLint can not identify this scenario and will not report errors. If it's not the responsibility of React, at least the eslint-plugin-react should detect it.

@RexSkz RexSkz added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 31, 2024
@li-jia-nan
Copy link

Mark

1 similar comment
@c0dedance
Copy link
Contributor

Mark

@gsathya
Copy link
Contributor

gsathya commented Jan 31, 2024

React does track the type of hook called (along with their order):

function updateHookTypesDev(): void {
if (__DEV__) {
const hookName = ((currentHookNameInDev: any): HookType);
if (hookTypesDev !== null) {
hookTypesUpdateIndexDev++;
if (hookTypesDev[hookTypesUpdateIndexDev] !== hookName) {
warnOnHookMismatchInDev(hookName);
}
}
}
}

This check should (and does) catch the error when user breaks the rules of react. This is what I see in the console:

Warning: React has detected a change in the order of Hooks called by App. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

   Previous render            Next render
   ------------------------------------------------------
1. useState                   useState
2. useRef                     useRef
3. useRef                     useRef
4. useEffect                  useRef
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Arguably there could be a case made for throwing and not just warning on hook type mismatch. I don't know the context here on why we chose to warn, @acdlite do you remember?

EDIT: Found the original PR (#14585), but not a lot of context there either. I'm going to close as this is working mostly ok -- we do warn with the correct error.

@gsathya gsathya closed this as completed Jan 31, 2024
@RexSkz
Copy link
Author

RexSkz commented Feb 1, 2024

Thanks for your reply. I look through the console and do see this warning message.

There are lots of console messages in my project. When this error occurs, the last few messages in console and the Webpack error overlay are Cannot read properties of undefined, leading me to wonder if it's a React bug... 😂

If this should remain "warning", maybe I'll find another way to let devs notice it.

@gsathya gsathya added Resolution: Wontfix and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants