Compiler bailout not caught by linter

[yepitschunked]

React Compiler Playground link: https://playground.react.dev/#N4Igzg9grgTgxgUxALhAHQHYEsC2AHCGAFwAIAlBAQzlIDMYIcSByGKm5gbk0wQA8CxEgBMEtSlAA2dKBhpYIGEgGFGBDAgxEAFAEoSwTCRJxFYUnFhstAVTAIYASWEkAvCQDKjBAHFJEACNKST0AOixhbgwjEzNSHABPAFlKDChgpIQcCDcSKHtM7O0Y4z03AD4SCCIACwc7B2c3V3dLGGsiBqdhAH5QoggPIhgsDABzPQAaEpIAbTaOrudJqtr6+26AXWmlEl0omLYiWCUAHmEsADdy4ESUtIysiABfU4B6C+vMZ5BnoA

The compiler bails out of optimizing this component because of

CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly (9:9)

However, the linter doesn't seem to mention this as an issue.

[poteto]

For some background, the compiler's lint plugin doesn't report all errors, only the ones that are actionable by the user. We only want to surface errors that someone can actually fix in their code if it breaks a Rule of React.

The compiler currently uses manual memoization as a hint. We compare our inferred memoization against manual memoization and bail out if they don't line up exactly. If the manual memoization was used to preserve stability in say, a useEffect downstream, changing memoization can result in unexpected behavior. But manual memoization not lining up with our inferred memoization isn't wrong, it just means the developer has more information than the compiler does about referential stability. So from the compiler's perspective, this is not an actionable error.

If you do happen to notice unnecessary manual useMemo/useCallback/React.memo, you can remove them.

We are in the midst of prototyping an IDE for React so this validation will be something we'll be thinking about as information we can provide to developers that isn't a hard error/warning, which is typically what we'll surface in ESLint instead.

[guillaumebrunerie]

Maybe there could be two ESLint rules, the current one for actionable errors (enabled by default and recommended for everyone), and one for this kind of situation (not recommended, only for people who are curious about which components were not compiled and why)?

[josephsavona]

Our plan is to report this category via the IDE plugin (language server) that we're exploring.

[yepitschunked]

I was more focused on the externally mutable value in useMemo's dependencies, which seems like a clear violation of the idempotency rule. I can see why this would be a more general Rules of React lint violation rather than a compiler-specific one, though.

[nfarina]

If you are, like I was, eager to do everything possible right now to make your components memoizable by the React Compiler, you can try this alternate ESLint plugin that sets the highest "panic threshold" for the compiler which surfaces ~all possible issues in your editor.

I think the argument from @poteto is essentially "it's better not to worry about this since many of these issues aren't actionable" but - I found I was able to stomp out 100% of lints on a pretty large codebase in a weekend.

Also - when writing components, I like to know for sure that the compiler is working without having to copy/paste to Playground, so I know I don't have to add manual memoization.

[SimenB]

Is this any different than having this config?

"react-compiler/react-compiler": [
  "error",
  {
    reportableLevels: new Set([
      "InvalidJS",
      "InvalidReact",
      "InvalidConfig",
      "CannotPreserveMemoization",
      "Todo",
      "Invariant",
    ]),
  },
],

only the first two are the default: https://github.com/facebook/react/blob/5c56b873efb300b4d1afc4ba6f16acf17e4e5800/compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts#L34-L37

With the full enum being https://github.com/facebook/react/blob/5c56b873efb300b4d1afc4ba6f16acf17e4e5800/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts#L11-L39

[nfarina]

The config I had tried with the official plugin was:

      "react-compiler/react-compiler": [
        "error",
        {
          reportableLevels: new Set([ErrorSeverity.InvalidReact]),
          __unstable_donotuse_reportAllBailouts: true,
        },
      ],

And the issue for me was that the lint errors contained too-coarse location information - so it would underline say the entire component/hook function at the top level, instead of the line that triggered the lint error.

[nfarina]

…also I just tried your suggested configuration with the official plugin and it didn't catch at least one lint error in my codebase. The "report all bailouts" seems to be necessary to cast a wider net.