[Compiler Bug]: Compiler incorrectly assumes non-nullability and lifts property access

[pawelblaszczyk5]

What kind of issue is this?

• [x] 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

https://playground.react.dev/#N4Igzg9grgTgxgUxALhHCA7MAXABAUQA8BDAWwAcAbBXAXlwAoBKOgPl2AB0Ndd0s8AbRgIw5TGAQAaXJOwAlUeKwIAunVxRJAZWzFsCADzBcAE33FkuDFEqVcAX1wAfDmYtWTAMwQJTAI2I4AGsrHBgASwwAcwBuXGwAT3IEK05wKDhEMDB0x3zXE3M9TwTk1Nx0hBgYCBg8pwdWBiKPa1t7ByZY7m5efhxcAAtiDFNqbUzssC9bSf9SCJyIzA0tBHwvHzhsfAA3BAxsBmY2Dj7eXGIc6uORMQkEADpi4gB+J6SUulp6dLApqJciAZOkACpDJayIbQSimXD+GhRPYQYJ+XCYSiJXAAdyGh3celwUIBWSBszs2IBCwi2AMpnS3V6PEuAwg1CelAg0RauB8fkCISs92UkheFie-ICQWCjiZLK6PQwF3Wm22x1OtHYXBZvAiXkYIse4r0Hy+NF+fwyZJyjPOusuIzGE0BOQp80Wy0wzCVl0cFwcMmESmNr1U8ouImwsB4DAuvEM-igdMw8cumAAwpQIiFaMBNdq0365IoHipea9SlLBaFKiADDh0jJzWlrdMGnLfX7HA4i2UUrR0kmU8qQGnWH2ABIIOwQXF1OFpwwAemH2EwE5Z8ocSpBaEwXgi0RQIAiFDqeHNbgACpQoNEogB5cjYFZYfJeWqkXAAckCiMoABaKh7yiQCRCCbBAPQCgImoGBl1MJZsB-JVuBaC5l2XGDyDg-Q3wAWQgUwKnSYg7HSbgnDAfCZgiURcFvUCMGfV8JG6PcwBhHEAEkjmqDByLAFAvCEhAHCAA

Repro steps

I'm using React Router fetchers for mutations, while migrating the app to 19.2.0 and trying to migrate some useRef() latest pattern to useEffectEvent I spotted weird bug with something being accessed despite being undefined at render.

Basically, having a function that's supposed to be invoked only when some state is in shape X (e.g. successful response) eagerly accesses state that this function uses to cache it. It causes the error "cannot read property of undefined" error to occur

1. I'm having a state which is an union of few different states (in original example this is RR useFetcher)
2. I'm having a useEffectEvent callback that's supposed to be called within useEffect when data becomes successful, additionaly I have an assertion so TypeScript allows me to access the .feedback part and no-one tries to invoke the callback otherwise
3. Mentioned useEffect that invokes (2)
4. User action invokes state change (in original example this is asynchronous, and happens in background, hence useEffect usage at all).

In the output the data is eagerly accessed to cache the function, causing the error:

Random thoughts: maybe it's somehow partially related to my assert function, where compiler doesn't know what does it do? But it causes me to don't use optional chaining, but compiler doesn't know why I'm not using it and is not informed by the types 🤔 Also it's not strictly related to the useEffectEvent or anything like that, because if I remove it - it'll still cause the same problem

How often does this bug happen?

Every time

What version of React are you using?

19.2.0

What version of React Compiler are you using?

19.1.0-rc.3

[mattcosta7]

Some additional bits here:

I tried to minimize another usecase, and see how it modified the ouptut with a minor input tweak (extracted from a project where we ran into this issue)

const InnerChatMessage = ({
  parentMessage,
  messageIndex,
  ...props
}: InnerChatMessageProps) => {
  const currentSubthread =
    (parentMessage?.childMessageIndexes?.findIndex(messageIdx => messageIdx === messageIndex) || 0) + 1 || 1

  const handlePrevSubthreadClick = useCallback(() => {
    const { messages } = getChatState()
-   console.log(messages[parentMessage!.childMessageIndexes![currentIndex - 1]!]!)
+   console.log(messages[parentMessage?.childMessageIndexes![currentIndex - 1]!]!)
  }, [parentMessage, getChatState])

  return (
    <div>
      {!editing && (
        <div className={styles.content}>
          {hasSubthreads && (
            <ChatPagingComponent
              currentPage={currentSubthread}
              onPrev={handlePrevSubthreadClick}
              isUserMessage={isUser}
            />
          )}
        </div>
      )}
    </div>
  )
}

import { c as _c } from "react/compiler-runtime";
const InnerChatMessage = (t0) => {
  const $ = _c(10);
  const { parentMessage, messageIndex } = t0;
  let t1;
  if ($[0] !== messageIndex || $[1] !== parentMessage?.childMessageIndexes) {
    t1 =
      (parentMessage?.childMessageIndexes?.findIndex(
        (messageIdx) => messageIdx === messageIndex,
      ) || 0) + 1 || 1;
    $[0] = messageIndex;
    $[1] = parentMessage?.childMessageIndexes;
    $[2] = t1;
  } else {
    t1 = $[2];
  }
  const currentSubthread = t1;
  let t2;
-  if ($[3] !== parentMessage.childMessageIndexes) {
+  if ($[3] !== parentMessage?.childMessageIndexes) {
    t2 = () => {
      const { messages } = getChatState();
      console.log(
        messages[parentMessage?.childMessageIndexes[currentIndex - 1]],
      );
    };
    $[3] = parentMessage?.childMessageIndexes;
    $[4] = t2;
  } else {
    t2 = $[4];
  }
  const handlePrevSubthreadClick = t2;
  let t3;
  if ($[5] !== currentSubthread || $[6] !== handlePrevSubthreadClick) {
    t3 = !editing && (
      <div className={styles.content}>
        {hasSubthreads && (
          <ChatPagingComponent
            currentPage={currentSubthread}
            onPrev={handlePrevSubthreadClick}
            isUserMessage={isUser}
          />
        )}
      </div>
    );
    $[5] = currentSubthread;
    $[6] = handlePrevSubthreadClick;
    $[7] = t3;
  } else {
    t3 = $[7];
  }
  let t4;
  if ($[8] !== t3) {
    t4 = <div>{t3}</div>;
    $[8] = t3;
    $[9] = t4;
  } else {
    t4 = $[9];
  }
  return t4;
};

https://playground.react.dev/#N4Igzg9grgTgxgUxALhHCA7MAXABASQwwRgGEALAQ2wFkEwxKBzBXAXlwApgAdDXXAAdKMBBlr1GLADR8BAW0nMEhACYIAHrP64AdPsEwIgsHwC+yAkRIVqdBsoAKRkwEp2APly8d6LHjhYUXEAZSgAI2xyUUpVdjkBLmFgiQcWAH5dOHIASwAbVXspFQx1DXpMgDMc0rVNTkU0lVUNT1xG4vwW9jYODuU6jXcAH2HcAAZ3AGpcAEZcUbm+BL8cXCpSvIRnBAA3MMjohFjSPJy4AGt2XCgwBFJKPLzwykvOTnc2Lx9E3FW8YDtJQsMC4MzXFjYWzYELYagID4JASrCBbXR5CBMBrA+gAbWSYlSxQAhFlcgUigNSpp6MTcYEYClBrgALRzAC6xM5rgSZmkuHxIkJlJkuEh0Nh8PZPIwCVE2Fg-E4SNwAB5VDldh4VQJgMSEBrsDUmLgAGSmrg6xLqzV-PKUBgAOUoijYwBwAE8tmAsphsISzNqdL9fsAqGADlEYqpQebLcGQ79VdDHMxjaQIPJBJhCVbE8igoTUyw3QyUpGjrEzHn85gdrs3RtVFt6xXo6dzhdqwn84kcmAAKp3GAihBu-tDkjd3u-AD0QZnuFc0-zqtnGq1VuXKrXG4XS-MIGkaEw1SYKBAOSzEBgeGwHsErEBjjyUCYNQA8oIjZhQeDKkY8i4AA5C84QIHkLKCK+74YCyMRwNgLLoFm+QkOu-bYMBADcywYNwCSzrOKGCPk1A5JgNAQOolg8CAjx5HR5i4IwRpgNU9C4Kmt45I8qovm+n7fhRWAeK42FHuA5AQAA7oQ-owBgjxgCglTKQgZhAA

https://playground.react.dev/#N4Igzg9grgTgxgUxALhHCA7MAXABASQwwRgGEALAQ2wFkEwxKBzBXAXlwApgAdDXXAAdKMBBlr1GLADR8BAW0nMEhACYIAHrP64AdPsEwIgsHwC+yAkRIVqdBsoAKRkwEp2APly8d6LHjhYUXEAZSgAI2xyUUpVdjkBLmFgiQcWAH5dOHIASwAbVXspFQx1DXpMgDMc0rVNTkU0lVUNT1xG4vwW9jYODuU6jXcAH2HcAAZ3AGpcAEZcUbm+BL8cXCpSvIRnBAA3MMjohFjSPJy4AGt2XCgwBFJKPLzwykvOTnc2Lx9E3FW8YDtJQsMC4MzXFjYWzYELYagID4JASrCBbXR5CBMBrA+gAbWSYlSxUy2XyhRxg3oAEJcYEYClBrgALRzAC6VPZrgSZmkuHxIkJRWUvMh0Nh8NZXIwCVE2Fg-E4SNwAB5VDldh4lQJgFSEGrsDUmLgAGTGrhaxKq9V-PKUBgAOUoijYwBwAE8tmAsphsISzJqdL9fsAqGADlEYqpQabzYGg79ldDHMxDaQIPJBJhCRb48igoTkywXXSUuGjrEzDnc5gdrsXRtVFta2XI6dzhdK3Hc4kcmAAKp3GBCovAXsDkid7u-AD0AanuFck9zyunao1FsXSpXa7nC-MIGkaEw1SYKBAOQzEBgeGwbsErEBjjyUCYNQA8oIDZhQeDKkZ5LgADkLzhAgeRMoIz6vhgTIxHA2BMugGb5CQq69tggEANzLBg3AJNO05IYI+TUDkmA0BA6iWDwICPHkNHmLgjAGmA1T0LgybXjkjzKk+L7vp+ZFYB4riYQe4DkBAADuhC+jAGCPGAKCVIpCBmEAA

the diff is on line 11

- console.log(messages[parentMessage!.childMessageIndexes![currentIndex - 1]!]!)
+ console.log(messages[parentMessage?.childMessageIndexes![currentIndex - 1]!]!)

when we have a non-conditional access that gets maintained - even if other, conditional access should apply. Maybe it could be a bit more intertwined with the types here to understand that the value is possibly undefined?

I'm seeing this only when updating from rc.3 to v1 in this specific example, but not sure if this is occuring elsewhere for us on rc.3

What version of React are you using?

18.3.0

What version of React Compiler are you using?

1.0.0

[kgssystem]

I'll just add another reproduction, what I think is fairly minimal one, and smaller than the two before:

Link to playground

Here's the component in question, it has one callback:

function assertIsNotEmpty<TValue>(
  value: TValue | null | undefined,
): asserts value is TValue {
  if (value === null || value === undefined)
    throw new Error('assertion failure')
}

export default function Component({
  planPeriod
}: Props) {

  const callback = () => {
    assertIsNotEmpty(planPeriod?.id)
    console.log(planPeriod.id)
  }

  return <div onClick={callback} /> 
}

This gets memoized into:

export default function Component(t0) {
  const $ = _c(2);
  const { planPeriod } = t0;
  let t1;
  if ($[0] !== planPeriod.id) {
    const callback = () => {
      assertIsNotEmpty(planPeriod?.id);
      console.log(planPeriod.id);
    };

    t1 = <div onClick={callback} />;
    $[0] = planPeriod.id;
    $[1] = t1;
  } else {
    t1 = $[1];
  }
  return t1;
}

And immediately crashes on render here: if ($[0] !== planPeriod.id) because planPeriod is undefined.

Thing is, as a developer I know that whenever this callback is invoked, planPeriod will be defined. The code doesn't, so I make the standard/recommended typescript assertion that planPeriod is defined in the callback, otherwise please crash as clearly the assumption is wrong. Thus typescript is quite happy with me doing console.log(planPeriod.id) without optional chaining in the next line. But it seems that react compiler ignores the error throwing part, sees the planPeriod.id access without optional chaining, and uses that as a source of truth. Whereas I'd expect it to see the different nullability that exists before throwing - see that planPeriod has optional chaining, and use that as a source of truth.

[mattcosta7]

https://github.com/facebook/react/issues/34194 https://github.com/facebook/react/issues/31551 https://github.com/facebook/react/issues/31205

Each of these seem like the same issue

[KurtGokhan]

Here is a minimal code example I created for this bug. Basically:

export function App() {
  const state = useStateWhichMayBeNullAtFirst();

  const onConfirm = useEffectEvent(() => {
    console.log(state.test)
  });

  return <button onClick={onConfirm}>Confirm</button>;
}

Compiles to a code which includes this in render function which causes a crash because state is null at first render.

if ($[0] !== state.test) {

[TSMMark]

Is there a workaround for this bug?

[josephsavona]

If a value is nullable, type systems like TypeScript or Flow would require you to guard against that with state?.test or if (state) console.log(state.test) for example. The compiler understands these patterns and will be more conservative in its estimates of dependencies.

[pawelblaszczyk5]

If a value is nullable, type systems like TypeScript or Flow would require you to guard against that with state?.test or if (state) console.log(state.test) for example. The compiler understands these patterns and will be more conservative in its estimates of dependencies.

Not sure about Flow - but when using TS there’s also possibility of using assertion which more clearly represents the intent of function being callable only when state is defined. And compiler isn’t capable of inferring it atm.

[TSMMark]

If a value is nullable, type systems like TypeScript or Flow would require you to guard against that with state?.test or if (state) console.log(state.test) for example. The compiler understands these patterns and will be more conservative in its estimates of dependencies.

Unfortunately, I'm getting this undefined access error even though I am using optional chaining (foo?.bar)

Is that a separate issue that has not already been raised here?

[josephsavona]

I'm getting this undefined access error even though I am using optional chaining (foo?.bar)

Can you share a link to playground w an example?

[josephsavona]

when using TS there’s also possibility of using assertion

Yup, @mofeiZ and I were chatting about that, we’re planning to make that change to also infer non-null assertions as sources of nullability.

[TSMMark]

I'm getting this undefined access error even though I am using optional chaining (foo?.bar)

Can you share a link to playground w an example?

Okay I haven't minimized the repro but I'll try now

[pawelblaszczyk5]

when using TS there’s also possibility of using assertion

Yup, @mofeiZ and I were chatting about that, we’re planning to make that change to also infer non-null assertions as sources of nullability.

That’s super cool - thanks

[KurtGokhan]

If a value is nullable, type systems like TypeScript or Flow would require you to guard against that

I understand that, and in fact we had this issue because somebody in our team used non-null assertion. But for me the problem is that it becomes a page-crashing render time error with the compiler, while it works perfectly without the compiler.

Also, is there a need to check for dependency equality when passing the function to useEffectEvent (or user-land alternatives like useStableCallback). It already returns a stable result, so is there a point to using the compiler there?

[TSMMark]

I'm getting this undefined access error even though I am using optional chaining (foo?.bar)

Can you share a link to playground w an example?

matchingOptionFromCustomProperties?.thumbnail_url is memoized as matchingOptionFromCustomProperties.thumbnail_url

I think it has to do with the other reference to matchingOptionFromCustomProperties.id after invariant here:

invariant(matchingOptionFromCustomProperties, 'matchingOptionFromCustomizeConfig is required')
setSelectedVariantId(matchingOptionFromCustomProperties.id)

So I think that TS knows that after invariant checks, it can not be null. But react compiler doesn't have knowledge of the check.

The repro definitely has more than it needs, but I ran out of time

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAegFQAIFgDYCWAdgC4C0AJgWAIYBGeCWcERY1JCplCAZjVDzkEADwAOEGOSI0AtgjQU+AoWVESpZWTRJwAFmV4FGM+VgxoAOkQKyNJLMCwBhViRrEEMLAF8svGAhZLAByAAE0FjtWLhIwNABxGAIKEOtbe0csAHkxEgJWAGVoeAQAGiwASU5ZAFEqEgB9CggAc0aCGsaIPIKiX39A4PC0AEcoLwJcNJtoqUdrLCwAJWhOMsWVtdwN-qwoMAQAORoANwJWnXLrPwCg0JgEGjhyQKhOGBmMyQdgTflZBBdksDghnDQ8Hg6M8ANbA-aHACyCEB8NBhXc6xug3uIUezxIXzmvwRCGqKPqnQAihMYABPAZ3YYAOjGtKmYCJmXBMAoOOGESiEiIsXiPNS6WJWERHiIAAknkpvEzQoKgsLRWgZcQFTQlVyflhioQKAAhd4kVj81WRdUxUjxc0kS1EABqACY0MaUk6XQb5sRTjRkjRSNaQoHgwRQ4TrNZeFAiC8+v5iBRybJTXTnAdLbIAAqBMRefK4AAUmx6+VYYGQVRqlKaLXanRR3V6rAA2gBdLAAH32RCURhFFHhfz2SzgoaDYFceEk8JBYgXeoQ6e0rQQ+Z0eiXWEgsEQ8J8dYnSynM5oc4gC5gdbAJGSRFam2Xq6UG5oW53JD0AH4HyfYhWn7Qdh08Cg3wPEpEEAnIOyIYojyYAdEwg0dNh8ABKOsM0bZo2g6LoqxTNChz4SCFj2R4SFgfpSJrf9mRHCgABlrxICtJywMtzwvS8iFnRoWDvfcp1zIIOk3BBGjAABmRoxF3cSsCDENSDk2Drh4nCsAAXgAPmgwTZ3nSQDP0-TmCvMARNvCyADJHN4kyLzLKAVwgNcvx-XcsGc5hJNkaTv1khSlP8qzrM8j910qGTfz0bC3KWPsBzLQ9SgClyssQSzrPU6NNLyhAUp4pZsPhcqfDjJMawcfCGgMrAAQgLAAB4sFqbIsEM3iy2wgz+v4lg2AcFIWpCABWGYpwarAZHOS5OBa0ETmWq5Bs2MbHywTsKB0GgKnGLw6UKAB3Tp9F7GLDia6laTpMsUnKnaFseLcRRgK42NYVo-r1ECWv4PBDne8asmbYiUTrZsMwGazDvcLB-3-RxaqICG9s7RoKkOEhCgQRgXnXV0oxjSoKFu0kMSuTqD2Al8wPQyjRz63jWZHdc3r2XaHA-YHrIAQnhmo6vmyHpyE69zO8ayQgYWEZmxhxtF0PQQNyasiAAMSGHNHyCQsehLDk1qRFEIDLQbhug1iMyzQ28xN4spA5MsxZRZjGKICp+IE6WzIcmBVNKuttb6ZDSmZWQ6X3HCKk7IPZZDiovdkbtqol5gFr0UMKEYVxSGICYLbBCEoVhG2hqM6iBMjDSuPV-QtcQ-Wgmd42izNnZQhbzWX0j1gO9kLvZAIAAvMFWCMUDqCwR5xgIR5UnKgSCaJknOAocmm6pssB7bnXR-H13e7AZlXugpaLi2gADRQ2jQAASYAUh8NB106NBv0eFFYj33Xr4JOKQKhHyHu3A2wVz7uz7rfFaCBuy80lntOOqx3hMDuggZEgIa7DVcjxQWL43Lo1OvSS6109BuTrNxASAkOrF3cJ4GAxkKr0MYcGKC7D6FYHyCQRg+kQi8AgBAOavCLxsIkRw2wr4eESIhCQIR48p7riwM2LAYhHjnAQBdcR0iN7wH0sACBrRh562gUbAsPc4FgGYn+KAsg6AyGMI0WAeBUboy5pBTGBiLxoCkQYjqaBxRsNSheDq3ozQWlYOEwOIchFaNsMGOk+iDEJkhAAdRSH+OJF4CBjSEWAMQxBvppOkbId4Og+iAyoC+Yxn0uBeF+v9WpIFfF+KwKwZwhA4AwmMfnIcRc3ClwQB0vx7Bp5CJEHgYMW5ykSOKcQBpCAvrNM4H9F8bSXzjIkYEvxdAYlY3kUsYJUTfSsH2RwkJbhZReCuZVKq+1oLENaPuch50roa33I076LStneTqW86CpjzGnxgTY0sYB9yDMLjPEuRAJjwmQTnWi9FCEMO1PKRUXg3L8MEcI0RaSHmdQwZwMAJLTlkoQHk4mADSDGPQdsXZvDlJ-iEXHBZSwAluWCdSil0FglYt1EqQJNVeYPRIMyKgxTZl0hOGYBWkqVYinEIaYcKhGoNgaNYEAZQQBjTnigEA3x5gkDpMWLI+Y8BQFaMQcxYBGRDFCNCOgxMyArltcQMg+IXhkCFMYLwigOAhAANx1T4psNAto7DGGqawREEAlB1ksCASuqbsS0HyGAIwuAsDWq9UQB12FQ16vAHoCAF1KikC8DIMGKBQaHB8EAA

[simonbuchan]

Realistic minimal example:

function SomePage() {
  const data = useLoaderData();

  const handleClick = async () => {
    await api.useSomeThing(data!.id);
  };

  return (
    <PageLayout>
      {!data && "not found"}
      {data && <button onClick={handleClick}>Use Some Thing</button>}
    </PageLayout>
  )
};