Repository: facebook/react
File: compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts (TypeScript)

Code before:

          /**
           * Bailout on void return useMemos. This is an anti-pattern where code might be using
           * useMemo like useEffect: running arbirtary side-effects synced to changes in specific
           * values.
           */
          if (
            func.env.config.enableValidateNoVoidUseMemo &&
            manualMemo.kind === 'useMemo'
          ) {
            const funcToCheck = sidemap.functions.get(
              fnPlace.identifier.id,
            )?.value;
            if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) {
              if (doesFunctionHaveVoidReturn(funcToCheck.loweredFunc.func)) {
                errors.push({
                  severity: ErrorSeverity.InvalidReact,
                  reason: `React Compiler has skipped optimizing this component because ${
                    manualMemo.loadInstr.value.kind === 'PropertyLoad'
                      ? 'React.useMemo'
                      : 'useMemo'
                  } doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects.`,
                  loc: instr.value.loc,
                  suggestions: null,
                  description: null,
                });
              }
            }
          }

          instr.value = getManualMemoizationReplacement(
            fnPlace,
            instr.value.loc,
            manualMemo.kind,
          );
          if (isValidationEnabled) {
            /**
             * Explicitly bail out when we encounter manual memoization
             * without inline instructions, as our current validation
             * assumes that source depslists closely match inferred deps
             * due to the `exhaustive-deps` lint rule (which only provides
             * diagnostics for inline memo functions)
             * ```js
             * useMemo(opaqueFn, [dep1, dep2]);
             * ```
             * While we could handle this by diffing reactive scope deps
             * of the opaque arg against the source depslist, this pattern
             * is rare and likely sketchy.
             */
            if (!sidemap.functions.has(fnPlace.identifier.id)) {
              CompilerError.throwInvalidReact({

Review comment (suggestion):
nit: let's use the new `errors.pushDiagnostic(CompilerDiagnostic.create(...))` API instead? You'll get nicer output.

Code after:
          const {fnPlace, depsList} = extractManualMemoizationArgs(
            instr as TInstruction<CallExpression> | TInstruction<MethodCall>,
            manualMemo.kind,
            sidemap,
          );

          /**
           * Bailout on void return useMemos. This is an anti-pattern where code might be using
           * useMemo like useEffect: running arbirtary side-effects synced to changes in specific
           * values.
           */
          if (
            func.env.config.validateNoVoidUseMemo &&
            manualMemo.kind === 'useMemo'
          ) {
            const funcToCheck = sidemap.functions.get(
              fnPlace.identifier.id,
            )?.value;
            if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) {
              if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) {
                errors.pushDiagnostic(
                  CompilerDiagnostic.create({
                    severity: ErrorSeverity.InvalidReact,
                    category: 'useMemo() callbacks must return a value',
                    description: `This ${
                      manualMemo.loadInstr.value.kind === 'PropertyLoad'
                        ? 'React.useMemo'
                        : 'useMemo'
                    } callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.`,
                    suggestions: null,
                  }).withDetail({
                    kind: 'error',
                    loc: instr.value.loc,
                    message: 'useMemo() callbacks must return a value',
                  }),
                );
              }
            }
          }

          instr.value = getManualMemoizationReplacement(
            fnPlace,
            instr.value.loc,
            manualMemo.kind,
          );
          if (isValidationEnabled) {
            /**
             * Explicitly bail out when we encounter manual memoization
             * without inline instructions, as our current validation
             * assumes that source depslists closely match inferred deps
             * due to the `exhaustive-deps` lint rule (which only provides