[React Compiler] React Fast Refresh Compatibility Issue

[daniel-nagy]

Summary

When using the React Compiler, Fast Refresh is broken when importing a non-JavaScript file, .e.g.

import content from "./content.md";

export const App = () => {
  return <div>{content}</div>;
};

In this example, if the content of the markdown file changes, then the app does not update without a page refresh. A repo for this example can be found at https://github.com/daniel-nagy/react-compiler-bug.

From a quick investigation, the issue appears to be that the compiler memo cache is global and that it does not treat imported values as mutable.

[josephsavona]

Thanks for posting! In React, globals are expected to not change, but this example shows that can break in fast refresh mode. We already have dev-mode support for fast refresh where we reset the memo cache if the source of the file changes, maybe we can also clear it on changes to referenced globals.

Out of curiosity, does the example work if you use useMemo(() => contents, []) ? That’s effectively what the compiler is doing.

[daniel-nagy]

Out of curiosity, does the example work if you use useMemo(() => contents, []) ?

Yes, without the compiler and with useMemo e.g.

import { useMemo } from "react";
import content from "./content.md";

export const App = () => {
  return <div>{useMemo(() => content, [])}</div>;
};

Fast refresh works.

From what I can tell, every time the content of an imported file changes, Vite will replace every file that imports that file with a new file. For example, this is what I see in the browser's developer tools after editing the markdown file a bunch of times.

The problem is that the compiler memo cache is external to the file. So even though the file is replaced, the cache still has the old value and becomes stale. If the cache were local to the file, I believe the problem would go away.

On a somewhat related but also unrelated note, I'm a little surprised the compiler uses a memo cache instead of just hoisting the static bits. In this case, no optimization is actually required by the compiler because the import is outside the component.

[josephsavona]

Thanks for confirming. What’s happening here is that the compiler is memoizing the <div> wrapping the contents. I have an idea for what we can do here to make this work - basically consider global as reactive in dev mode. This would incur some extra if/else checks for dependencies in dev mode but shouldn’t otherwise change behavior.

I’ll experiment w this.

[josephsavona]

I added some test fixtures for this in #29175 — not a fix yet, but just reproducing the problem in tests.

[josephsavona]

As I thought about how we might make this case work, it's pretty tricky. The challenge is that in production we don't want to incur the overhead of comparing whether known-constants have changed (they're constants!). But in development, you might change the value of a constant and it's reasonable to expect that to show up with fast refresh. We already reset the cache if the source file itself changes, so we're really just talking about contents from other files.

A naive approach would be to reset the cache if any constants have changed — but we have to think through the implications. It's important that behavior is consistent between development and production, and we wouldn't want fast refresh support to mask bugs in your application (by resetting the cache more often) that then show up in production (as values not appearing to update).

We'll continue to explore this and also debug the repro you sent. We actually wouldn't expect the useMemo version to reset, so this may be a plugin ordering issue.

[jmswrnr]

We'll continue to explore this and also debug the repro you sent. We actually wouldn't expect the useMemo version to reset, so this may be a plugin ordering issue.

I had this same issue today when updating CSS modules with Vite HMR.
The component still updates with fast refresh but the cache isn't reset, resulting in stale CSS class names.

Instead of relying on the component source hash comparison for resetting the cache, could we use the same logic with a compile ID or timestamp value instead? With the aim of resetting the cache if the component was updated to a different compiled version, even without source changes. This would still treat globals as constants, but would allow a HMR implementations to fast refresh the component if an import was updated, which appears to be the current behaviour with Vite HMR already.

[josephsavona]

@jmswrnr Using a compile revision from eg Vite seems reasonable. The question is how we could access that value. This could either be directly in the compiler (ie at build-time), or to have the compiler emit code that will access that value at runtime. What type of API does Vite HMR expose for checking the version of a file?

[jmswrnr]

@josephsavona I attempted using a timestamp instead of source hash in the babel-plugin-react-compiler enableResetCacheOnSourceFileChanges check, but without the component source contents changing Vite would refresh the component with the same compiled version containing matching timestamps.

But I did implement a different approach which allowed us to workaround the issue in dev:

I created a wrapper function for useMemoCache which uses a useMemo to clear the cache when the component is fast refreshed, and I used the runtimeModule option for the compiler to import this wrapper in dev. I believe with an empty dep array, this useMemo will always run on the initial render and every render following a fast refresh, but not on other renders.

import { useMemo } from 'react'
import { c as useMemoCache } from 'react/compiler-runtime'

export const c = (size: number) => {
  const cache = useMemoCache(size)

  useMemo(() => {
    cache.fill(Symbol.for('react.memo_cache_sentinel'))
  }, [])

  return cache
}

This works in our case because Vite refreshes the React component with the same compiled version if the imported module changes. I don't believe it differs too much from the enableResetCacheOnSourceFileChanges except it will clear cache on every fast refresh.

[josephsavona]

Interesting, that might work. Seems like it could also reset too much, but it's definitely an option.

[github-actions]

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!

[ArnaudBarre]

I discovered the issue via vitejs/vite-plugin-react#361 (reply in thread)

You can test a recent repro of the bug with: https://stackblitz.com/github/hi-ogawa/react-compiler-css-module/tree/chore-minimal-repro?file=src%2Frepro.tsx

With a version of the compiler from May it didn't work with both strict mode and non strict mode, now the bug only appear with strict mode.

This is not the only issue where strict mode breaks HMR: #29915

Happy to discuss HMR api updates if needed to make it works!

[hi-ogawa]

I tested on Next.js canary with experimental.reactCompiler: true and it looks like the same issue reproduces. Here is a reproduction https://github.com/hi-ogawa/reproductions/tree/main/next-react-compiler-dep-hmr

[josephsavona]

@poteto and @gaearon helped to land fixes for this, so the latest release of the compiler should work without issue with the latest experimental release of React itself. We'd appreciate if someone could test that combination and confirm! (We had also seen fast refresh issues internally and this combination fixed it for us)

[jmswrnr]

@poteto and @gaearon helped to land fixes for this, so the latest release of the compiler should work without issue with the latest experimental release of React itself. We'd appreciate if someone could test that combination and confirm! (We had also seen fast refresh issues internally and this combination fixed it for us)

I've tested my previous case on the latest experimental versions:

"react": "0.0.0-experimental-94e652d5-20240912",
"react-dom": "0.0.0-experimental-94e652d5-20240912",
"babel-plugin-react-compiler": "^0.0.0-experimental-fe484b5-20240912",
"eslint-plugin-react-compiler": "0.0.0-experimental-5c9a529-20240912",

I can confirm my fast refresh issue we discussed earlier has been resolved and I no longer need the useMemoCache wrapper! However, Fast Refresh is also consistently slower since my previous comments (from ~400ms up to ~900ms)

[josephsavona]

@jmswrnr thank you for confirming! I'm not sure about the slowdown you experienced, lets see if that is a widespread issue or a one-off.