[Compiler Bug]: wrong check with all `props` object (`$[0] !== props`) when accessing the function from the `props` object and calls it, e.g. `props.reportBug()`

[dimaMachina]

What kind of issue is this?

• React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
• babel-plugin-react-compiler (build issue installing or using the Babel plugin)
• eslint-plugin-react-compiler (build issue installing or using the eslint plugin)
• react-compiler-healthcheck (build issue installing or using the healthcheck script)

Link to repro

Repro steps

I found an issue with react-compiler, which makes fails this test https://github.com/graphql/graphiql/blob/8092f100ae62710bece1666589eda0455dc04009/packages/graphiql/src/GraphiQL.spec.tsx#L76-L84

As you can see function introspect contains reference to props.schema, props.fetcher and props.onSchemaChange but the react-compiler simplified it to $[13] !== props which is wrong!

For now, I fixed this with this commit: graphql/graphiql@3ecae05 by avoid having props.something... inside body of introspect function

I hope this will be prioritized and fixed soon, friendly ping gsathya

How often does this bug happen?

Every time

What version of React are you using?

from playground

What version of React Compiler are you using?

from playground

[dimaMachina]

and here is minimal reproduction https://playground.react.dev/#N4Igzg9grgTgxgUxALhASwLYAcIwC4AEwBUYCAogGaUJyEC+BlMEGBAOiDAgIZ2cBudgDsRlKMLpoIwggBUEYPAAosLLGACUREQQJwZSphAgEAvCTIAlBGogATKFJnLNQ2fsOEARjxjmCV3MAPh0PPR4wAE9JJglnWQBHKAAPIOBdPSy7DQA6DCibGhhJBABlAAtoABt7ACEEAGEK2gBrBHtlShM3TL16PoJktN6Penc9bjxYWQAebyg8PBkCGUbqtDhWs2BfGEYAemD3AeEQeiA

import { useEffect } from "react";

function Test(props) {
  const foo = useReproduction();
  const bar = () => {
    async function qux() {
      props.myRefernceShouldBeChecked(foo);
    }
    qux();
  };
  return <button onClick={bar} />;
}

compiled to if ($[0] !== foo || $[1] !== props) { which is wrong

---

following fails too

import { useEffect } from "react";

function Test(props) {
  function bar() {
    Promise.resolve().then(() => {
      props.myRefernceShouldBeChecked();
    })
  };
  return <button onClick={bar} />;
}

compiled to if ($[0] !== props) {

[dimaMachina]

I think even this is wrong

function Test(props) {
  function bar() {
    props.myRefernceShouldBeChecked();
  };
  return <button onClick={bar} />;
}

compiled to if ($[0] !== props) { but should if ($[0] !== props.myRefernceShouldBeChecked) {

I think this is an issue when you call function by accessing props object, like props.someFunction(), because following is right

function Test(props) {
  function bar() {
    console.log(props.myRefernceShouldBeChecked)
  };
  return <button onClick={bar} />;
}

compiled to if ($[0] !== props.myRefernceShouldBeChecked) {

[dimaMachina]

Deep access of props should be fixed as well:

function useTest(props) {
  return () => {
    props.foo.bar.CHECK_ME()
  };
}

compiled to if ($[0] !== props.foo.bar) { but should if ($[0] !== props.foo.bar.CHECK_ME) {

[dimaMachina]

Note

For anybody who wants to prevent this bug until react-compiler is fixed, here is ESLint selector for no-restricted-syntax rule CallExpression Identifier[name=props]. And ESLint playground with it

[josephsavona]

Thanks for posting! What's happening here is that the compiler is aware of prototypes, and has to assume that if code accesses only object.method(), that obj.method may not always change when object does. Consider:

const array1 = [];
const array2 = [];
array1 === array2; // false
array1.push === array2.push; // true!

This is because array1.push and array2.push both evaluate to Array.prototype.push, ie the same function.

Thus, in some of the examples you provided it is not safe to take the more precise dependency. Consider your last example:

function useTest(props) {
  return () => {
    props.foo.bar.CHECK_ME()
  };
}

Here, props.foo.bar is the receiver, and this will look up CHECK_ME on the prototype chain. Per the above example with array push, there's no guarantee that props.foo.bar.CHECK_ME will change when props.foo.bar changes — the compiler is correctly taking props.foo.bar as the dependency. A few of the other examples you provided fall into this category.

Note that if you know that the function is not a method (doesn't depend on the this receiver), you can manually rewrite to

function useTest(props) {
  return () => {
    const CHECK_ME = props.foo.bar.CHECK_ME; // now the compiler will depend on this full path
    CHECK_ME();
  };
}

However, there's still a catch here because inside a function expression, the compiler can't know that props.foo.bar is non-nullable, so we may have to be pessimistic and take a less precise dependency. We do a bunch of analysis (courtesy of @mofeiZ) to infer non-null paths and take the most precise dependency that is safe though.

That brings us to the first example, which we can optimize. In the general case we don't know which methods are prototype methods that won't change w the object. But props is an object that the user cannot construct themselves — we know that props.method() is not looking up something on the prototype, because that makes no sense! So specifically when props is the receiver, we can safely rewrite props.method() to just const temp = props.method; temp(), allowing us to take a dependency on props.method. I've implemented that specific optimization in #31771.

[dimaMachina]

@josephsavona Huge thanks for the explanation, reactive fix and your amazing work on the react compiler.

For my side, I am always happy to share the reproduction of issues which I find while optimizing React app 👍

[josephsavona]

We appreciate the concise examples, this made it really easy to narrow down. I’m going to close since we’ve landed the fix.