这是indexloc提供的服务,不要输入任何密码
Skip to content

Bug: Dev mode sometimes evaluates unrelated getters in props objects causing side-effects #35126

@yGuy

Description

@yGuy

In dev mode, there is a feature that inspects the properties of all props passed to a component, recursively. This happens using a simple for (key in obj) loop and thus traverses all enumerable properties, recursively, including getters. This feature cannot be disabled without disabling dev mode and it will only trigger in slightly advanced setups and thus is hard to debug.
Iterating all enumerable keys for getters in an object is a problem, because these getters are also evaluated using simple unconditional obj[key] syntax, causing possible side-effects to happen for non-trivial getters.
Although frowned-upon in many cases, it is valid for getters to have side-effects that could change the state of the objects or throw exceptions. Specifically in objects that are unrelated to the React components.
However, in dev mode, this causes the application to break or to behave differently in non-dev mode with hard to diagnose bugs.

React version: 19.2.0

(this is a regression)

Steps To Reproduce

  1. Run the following demo in dev mode
import {useEffect, useState} from 'react'
import {createRoot} from "react-dom/client";

function getData() {
    const result = {}
    result._someInnocentUnusedProp = {}
    Object.defineProperty(result._someInnocentUnusedProp, "nestedUnusedCalculatedProp", {
        get: () => {
            console.log('get nestedUnusedCalculatedProp called - why?')
            throw new Error("I should not call getters unconditionally")
        }, enumerable: true
    })
    return result._someInnocentUnusedProp
}

function DataRenderer() { return null }

function Child(props) {
    return <div>child: {props.count}<DataRenderer data={getData()}></DataRenderer></div>;
}

function App() {
    const [count, setCount] = useState(0);

    // just to cause a re-render
    useEffect(() => {
        if (count > 5) return
        setCount((c) => c + 1);
    }, [count]);

    return <Child count={count}/>;
}

createRoot(document.getElementById("root")).render(<App/>);
  1. You will get a stacktrace like the following:
 Uncaught Error: I should not call getters unconditionally
    at Object.get [as nestedUnusedCalculatedProp] (main.jsx:10:19)
    at addObjectDiffToProperties (react-dom-client.development.js:3968:21)
    at addObjectDiffToProperties (react-dom-client.development.js:4016:23)
    at logComponentRender (react-dom-client.development.js:4130:22)
    at commitPassiveMountOnFiber (react-dom-client.development.js:15469:13)
    at recursivelyTraversePassiveMountEffects (react-dom-client.development.js:15439:11)
    at commitPassiveMountOnFiber (react-dom-client.development.js:15718:11)
    at recursivelyTraversePassiveMountEffects (react-dom-client.development.js:15439:11)
    at commitPassiveMountOnFiber (react-dom-client.development.js:15476:11)
    at recursivelyTraversePassiveMountEffects (react-dom-client.development.js:15439:11)

Link to code example:

Code Sandbox

(Sometimes you need to reload the page a couple of times for the issue to appear. The timing is not deterministic, which makes the issue especially hard to diagnose.)

The current behavior

React Dev mode evaluates getters recursively from time to time, possibly causing side effects.

The expected behavior

Rect dev mode should not evaluate all getters in props objects unconditionally.

My thoughts

The problematic code is here, which evaluates all enumerable properties, even from prototypes:

The workaround here (other than not using dev mode) is to make the properties non-enumerable, however that is not always feasible.

My suggested fix would be to not evaluate getters for the logging purposes. They are likely to return new objects, perform a lot of work and thus slow down the execution, may change the state of the application, or may even throw. Only ever compare fields to avoid changing the state or executing code at all costs.

Using Object.keys() would improve the situation for properties defined on classes, but ideally "getter"s should not be called at all, no matter whether they are defined as own properties or prototype properties.

The current state requires complicated workarounds and causes hard to debug issues in complex applications. As it only affects dev-mode, I would love to see this improved in a bugfix so that people can upgrade to 19.2

Metadata

Metadata

Assignees

No one assigned

    Labels

    Status: UnconfirmedA potential issue that we haven't yet confirmed as a bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions