ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/react
File: packages/react-devtools-shared/src/backend/fiber/renderer.js (JavaScript)

Code before:
      ) {
        return resource.instance;
      }
    }
    return null;
  }

  function findAllCurrentHostInstances(
    fiberInstance: FiberInstance,
  ): $ReadOnlyArray<HostInstance> {
    const hostInstances = [];
    const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance);
    if (!fiber) {
      return hostInstances;
    }

    // Next we'll drill down this component to find all HostComponent/Text.
    let node: Fiber = fiber;
    while (true) {
      if (
        node.tag === HostComponent ||
        node.tag === HostText ||
        node.tag === HostSingleton ||
        node.tag === HostHoistable
      ) {
        const hostInstance = node.stateNode || getResourceInstance(fiber);
        if (hostInstance) {
          hostInstances.push(hostInstance);
        }
      } else if (node.child) {
        node.child.return = node;
        node = node.child;
        continue;
      }
      if (node === fiber) {
        return hostInstances;
      }
      while (!node.sibling) {
        if (!node.return || node.return === fiber) {
          return hostInstances;
        }
        node = node.return;
      }
      node.sibling.return = node.return;
      node = node.sibling;
    }
    // Flow needs the return here, but ESLint complains about it.
    // eslint-disable-next-line no-unreachable
    return hostInstances;
  }


Review comment (suggestion):
Shouldn't it be like this?

```suggestion
        const hostInstance = node.stateNode || getResourceInstance(node);
```

Code after:
      ) {
        return resource.instance;
      }
    }
    return null;
  }

  function findAllCurrentHostInstances(
    fiberInstance: FiberInstance,
  ): $ReadOnlyArray<HostInstance> {
    const hostInstances = [];
    const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance);
    if (!fiber) {
      return hostInstances;
    }

    // Next we'll drill down this component to find all HostComponent/Text.
    let node: Fiber = fiber;
    while (true) {
      if (
        node.tag === HostComponent ||
        node.tag === HostText ||
        node.tag === HostSingleton ||
        node.tag === HostHoistable
      ) {
        const hostInstance = node.stateNode || getResourceInstance(node);
        if (hostInstance) {
          hostInstances.push(hostInstance);
        }
      } else if (node.child) {
        node.child.return = node;
        node = node.child;
        continue;
      }
      if (node === fiber) {
        return hostInstances;
      }
      while (!node.sibling) {
        if (!node.return || node.return === fiber) {
          return hostInstances;
        }
        node = node.return;
      }
      node.sibling.return = node.return;
      node = node.sibling;
    }
    // Flow needs the return here, but ESLint complains about it.
    // eslint-disable-next-line no-unreachable
    return hostInstances;
  }
Repository: facebook/react
File: compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts (TypeScript)

Code before:
      directive =>
        directive.isDirective() &&
        OPT_OUT_DIRECTIVES.has(directive.node.value.value),
    );
  }
  return [];
}

function isCriticalError(err: unknown): boolean {
  return !(err instanceof CompilerError) || err.isCritical();
}

function isConfigError(err: unknown): boolean {
  if (err instanceof CompilerError) {
    return err.details.some(
      detail => detail.severity === ErrorSeverity.InvalidConfig,
    );
  }
  return false;
}

export type BabelFn =
  | NodePath<t.FunctionDeclaration>
  | NodePath<t.FunctionExpression>
  | NodePath<t.ArrowFunctionExpression>;

export type CompileResult = {
  /**
   * Distinguishes existing functions that were compiled ('original') from
   * functions which were outlined. Only original functions need to be gated
   * if gating mode is enabled.
   */
  kind: 'original' | 'outlined';
  originalFn: BabelFn;
  compiledFn: CodegenFunction;
};

function logError(
  err: unknown,
  pass: CompilerPass,
  fnLoc: t.SourceLocation | null,
): void {
  if (pass.opts.logger) {
    if (err instanceof CompilerError) {
      for (const detail of err.details) {
        pass.opts.logger.logEvent(pass.filename, {
          kind: 'CompileError',
          fnLoc,
          detail: detail.options,
        });
      }

Review comment (refactor):
Split this up so we can decide to only log in some cases instead of throwing (eg a file contains at least one function that was compiled although there were opt outs + errors in others)

Code after:
function isConfigError(err: unknown): boolean {
  if (err instanceof CompilerError) {
    return err.details.some(
      detail => detail.severity === ErrorSeverity.InvalidConfig,
    );
  }
  return false;
}

export type BabelFn =
  | NodePath<t.FunctionDeclaration>
  | NodePath<t.FunctionExpression>
  | NodePath<t.ArrowFunctionExpression>;

export type CompileResult = {
  /**
   * Distinguishes existing functions that were compiled ('original') from
   * functions which were outlined. Only original functions need to be gated
   * if gating mode is enabled.
   */
  kind: 'original' | 'outlined';
  originalFn: BabelFn;
  compiledFn: CodegenFunction;
};

function logError(
  err: unknown,
  pass: CompilerPass,
  fnLoc: t.SourceLocation | null,
): void {
  if (pass.opts.logger) {
    if (err instanceof CompilerError) {
      for (const detail of err.details) {
        pass.opts.logger.logEvent(pass.filename, {
          kind: 'CompileError',
          fnLoc,
          detail: detail.options,
        });
      }
    } else {
      let stringifiedError;
      if (err instanceof Error) {
        stringifiedError = err.stack ?? err.message;
      } else {
        stringifiedError = err?.toString() ?? '[ null ]';
      }

      pass.opts.logger.logEvent(pass.filename, {
        kind: 'PipelineError',
        fnLoc,
        data: stringifiedError,
Repository: facebook/react
File: compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts (TypeScript)

Code before:
          ],
        });
      }
    }
    if (shouldReportUnusedOptOutDirective) {
      return {
        FunctionDeclaration(fnDecl) {
          for (const stmt of fnDecl.body.body) {
            reportUnusedOptOutDirective(stmt);
          }
        },
        ArrowFunctionExpression(fnExpr) {
          if (fnExpr.body.type === 'BlockStatement') {
            for (const stmt of fnExpr.body.body) {
              reportUnusedOptOutDirective(stmt);
            }
          }
        },
        FunctionExpression(fnExpr) {
          for (const stmt of fnExpr.body.body) {
            reportUnusedOptOutDirective(stmt);
          }
        },
      };
    } else {
      return {};
    }
  },
};

export default rule;


Review comment (suggestion):
This won't necessarily match 1:1 with functions that the compiler attempted to compile, in particular it might find more functions. But I guess that's okay, if you have a "use no memo" in a component that isn't being recognized as a component/hook that's also an error.

Code after:
                return fixer.remove(stmt);
              },
            },
          ],
        });
      }
    }
    if (shouldReportUnusedOptOutDirective) {
      return {
        FunctionDeclaration(fnDecl) {
          for (const stmt of fnDecl.body.body) {
            reportUnusedOptOutDirective(stmt);
          }
        },
        ArrowFunctionExpression(fnExpr) {
          if (fnExpr.body.type === 'BlockStatement') {
            for (const stmt of fnExpr.body.body) {
              reportUnusedOptOutDirective(stmt);
            }
          }
        },
        FunctionExpression(fnExpr) {
          for (const stmt of fnExpr.body.body) {
            reportUnusedOptOutDirective(stmt);
          }
        },
      };
    } else {
      return {};
    }
  },
};

export default rule;
Repository: facebook/react
File: compiler/apps/playground/components/Editor/EditorImpl.tsx (TypeScript)

Code before:
  ],
  [
    'useLazyLoadQuery',
    {
      valueKind: ValueKind.Frozen,
      effectKind: Effect.Freeze,
      noAlias: true,
      transitiveMixedData: true,
    },
  ],
  [
    'usePreloadedQuery',
    {
      valueKind: ValueKind.Frozen,
      effectKind: Effect.Freeze,
      noAlias: true,
      transitiveMixedData: true,
    },
  ],
];

function isHookName(s: string): boolean {
  return /^use[A-Z0-9]/.test(s);
}

function getReactFunctionType(
  id: NodePath<t.Identifier | null | undefined>,
): ReactFunctionType {
  if (id && id.node && id.isIdentifier()) {
    if (isHookName(id.node.name)) {
      return 'Hook';
    }

    const isPascalCaseNameSpace = /^[A-Z].*/;
    if (isPascalCaseNameSpace.test(id.node.name)) {
      return 'Component';
    }
  }
  return 'Other';
}

function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] {
  const results = new Map<string, PrintedCompilerPipelineValue[]>();
  const error = new CompilerError();
  const upsert = (result: PrintedCompilerPipelineValue) => {
    const entry = results.get(result.name);
    if (Array.isArray(entry)) {
      entry.push(result);
    } else {
      results.set(result.name, [result]);
    }

Review comment (bug):
For ArrowFunctionExpressions, we now incorrectly assume components to be of type 'Other' as they don't have an identifier. There are optimisations and assumptions that depend on getting this type correct, which cause incorrectly generated code.


Code after:
  ],
  [
    'useLazyLoadQuery',
    {
      valueKind: ValueKind.Frozen,
      effectKind: Effect.Freeze,
      noAlias: true,
      transitiveMixedData: true,
    },
  ],
  [
    'usePreloadedQuery',
    {
      valueKind: ValueKind.Frozen,
      effectKind: Effect.Freeze,
      noAlias: true,
      transitiveMixedData: true,
    },
  ],
];

function isHookName(s: string): boolean {
  return /^use[A-Z0-9]/.test(s);
}

function getReactFunctionType(id: t.Identifier | null): ReactFunctionType {
  if (id != null) {
    if (isHookName(id.name)) {
      return 'Hook';
    }

    const isPascalCaseNameSpace = /^[A-Z].*/;
    if (isPascalCaseNameSpace.test(id.name)) {
      return 'Component';
    }
  }
  return 'Other';
}

function getFunctionIdentifier(
  fn:
    | NodePath<t.FunctionDeclaration>
    | NodePath<t.ArrowFunctionExpression>
    | NodePath<t.FunctionExpression>,
): t.Identifier | null {
  if (fn.isArrowFunctionExpression()) {
    return null;
  }
  const id = fn.get('id');
  return Array.isArray(id) === false && id.isIdentifier() ? id.node : null;
}
Repository: facebook/react
File: packages/react-server/src/ReactFlightServer.js (JavaScript)

Code before:
  return '$@';
}

function serializePromiseID(id: number): string {
  return '$@' + id.toString(16);
}

function serializeServerReferenceID(id: number): string {
  return '$F' + id.toString(16);
}

function serializeSymbolReference(name: string): string {
  return '$S' + name;
}

function serializeLimitedObject(): string {
  return '$Y';
}

function serializeHaltedReference(): string {
  return '$&';
}
const reusableHaltedReferenceModel = '"$&"';

function serializeLazyHaltedReference(): string {
  return '$&L';
}

function serializeNumber(number: number): string | number {
  if (Number.isFinite(number)) {
    if (number === 0 && 1 / number === -Infinity) {
      return '$-0';
    } else {
      return number;
    }
  } else {
    if (number === Infinity) {
      return '$Infinity';
    } else if (number === -Infinity) {
      return '$-Infinity';
    } else {
      return '$NaN';
    }
  }
}

function serializeUndefined(): string {
  return '$undefined';
}

function serializeDateFromDateJSON(dateJSON: string): string {

Review comment (suggestion):
Shouldn't it be:
```suggestion
  return '$L&';
```
Assuming `&` is for halted references what otherwise would be the ID.

Code after:
  return '$@';
}

function serializePromiseID(id: number): string {
  return '$@' + id.toString(16);
}

function serializeServerReferenceID(id: number): string {
  return '$F' + id.toString(16);
}

function serializeSymbolReference(name: string): string {
  return '$S' + name;
}

function serializeLimitedObject(): string {
  return '$Y';
}

function serializeNumber(number: number): string | number {
  if (Number.isFinite(number)) {
    if (number === 0 && 1 / number === -Infinity) {
      return '$-0';
    } else {
      return number;
    }
  } else {
    if (number === Infinity) {
      return '$Infinity';
    } else if (number === -Infinity) {
      return '$-Infinity';
    } else {
      return '$NaN';
    }
  }
}

function serializeUndefined(): string {
  return '$undefined';
}

function serializeDateFromDateJSON(dateJSON: string): string {
  // JSON.stringify automatically calls Date.prototype.toJSON which calls toISOString.
  // We need only tack on a $D prefix.
  return '$D' + dateJSON;
}

function serializeBigInt(n: bigint): string {
  return '$n' + n.toString(10);
}
Repository: facebook/react
File: packages/react-client/src/ReactFlightClient.js (JavaScript)

Code before:
          key,
          createServerReferenceProxy,
        );
      }
      case 'T': {
        // Temporary Reference
        const reference = '$' + value.slice(2);
        const temporaryReferences = response._tempRefs;
        if (temporaryReferences == null) {
          throw new Error(
            'Missing a temporary reference set but the RSC response returned a temporary reference. ' +
              'Pass a temporaryReference option with the set that was used with the reply.',
          );
        }
        return readTemporaryReference(temporaryReferences, reference);
      }
      case '&': {
        if (enableHalt) {
          if (value === '$&L') {
            // This is a lazy wrapper for a halted reference
            return createLazyChunkWrapper(createBlockedChunk());
          }
          return waitForever();
        }
        // It'd be an error if this ever happened when this flag is off
        // but that shoudl be impossibe
        // fallthrough
      }
      case 'Q': {
        // Map
        const ref = value.slice(2);
        return getOutlinedModel(response, ref, parentObject, key, createMap);
      }
      case 'W': {
        // Set
        const ref = value.slice(2);
        return getOutlinedModel(response, ref, parentObject, key, createSet);
      }
      case 'B': {
        // Blob
        if (enableBinaryFlight) {
          const ref = value.slice(2);
          return getOutlinedModel(response, ref, parentObject, key, createBlob);
        }
        return undefined;
      }
      case 'K': {
        // FormData
        const ref = value.slice(2);
        return getOutlinedModel(
          response,

Review comment (suggestion):
```suggestion
        // but that should be impossibe
```


Code after:
        return getOutlinedModel(response, ref, parentObject, key, createMap);
      }
      case 'W': {
        // Set
        const ref = value.slice(2);
        return getOutlinedModel(response, ref, parentObject, key, createSet);
      }
      case 'B': {
        // Blob
        if (enableBinaryFlight) {
          const ref = value.slice(2);
          return getOutlinedModel(response, ref, parentObject, key, createBlob);
        }
        return undefined;
      }
      case 'K': {
        // FormData
        const ref = value.slice(2);
        return getOutlinedModel(
          response,
          ref,
          parentObject,
          key,
          createFormData,
        );
      }
      case 'i': {
        // Iterator
        const ref = value.slice(2);
        return getOutlinedModel(
          response,
          ref,
          parentObject,
          key,
          extractIterator,
        );
      }
      case 'I': {
        // $Infinity
        return Infinity;
      }
      case '-': {
        // $-0 or $-Infinity
        if (value === '$-0') {
          return -0;
        } else {
          return -Infinity;
        }
      }
      case 'N': {
        // $NaN
Repository: facebook/react
File: packages/react-client/src/ReactFlightClient.js (JavaScript)

Code before:
  const lazyType: LazyComponent<T, SomeChunk<T>> = {
    $$typeof: REACT_LAZY_TYPE,
    _payload: chunk,
    _init: readChunk,
  };
  if (__DEV__) {
    // Ensure we have a live array to track future debug info.
    const chunkDebugInfo: ReactDebugInfo =
      chunk._debugInfo || (chunk._debugInfo = []);
    lazyType._debugInfo = chunkDebugInfo;
  }
  return lazyType;
}

function getChunk(response: Response, id: number): SomeChunk<any> {
  const chunks = response._chunks;
  let chunk = chunks.get(id);
  if (!chunk) {
    chunk = createPendingChunk(response);
    chunks.set(id, chunk);
  }
  return chunk;
}

/**
 * Fork of waitForReference that doesn't ever reslve
 */
function waitForever() {
  if (initializingHandler) {
    initializingHandler.deps++;
  } else {
    initializingHandler = {
      parent: null,
      chunk: null,
      value: null,
      deps: 1,
      errored: false,
    };
  }

  return null;
}

function waitForReference<T>(
  referencedChunk: SomeChunk<T>,
  parentObject: Object,
  key: string,
  response: Response,
  map: (response: Response, model: any) => T,
  path: Array<string>,
): T {

Review comment (suggestion):
```suggestion
 * Fork of waitForReference that doesn't ever resolve
```


Code after:
  const lazyType: LazyComponent<T, SomeChunk<T>> = {
    $$typeof: REACT_LAZY_TYPE,
    _payload: chunk,
    _init: readChunk,
  };
  if (__DEV__) {
    // Ensure we have a live array to track future debug info.
    const chunkDebugInfo: ReactDebugInfo =
      chunk._debugInfo || (chunk._debugInfo = []);
    lazyType._debugInfo = chunkDebugInfo;
  }
  return lazyType;
}

function getChunk(response: Response, id: number): SomeChunk<any> {
  const chunks = response._chunks;
  let chunk = chunks.get(id);
  if (!chunk) {
    chunk = createPendingChunk(response);
    chunks.set(id, chunk);
  }
  return chunk;
}

function waitForReference<T>(
  referencedChunk: SomeChunk<T>,
  parentObject: Object,
  key: string,
  response: Response,
  map: (response: Response, model: any) => T,
  path: Array<string>,
): T {
  let handler: InitializationHandler;
  if (initializingHandler) {
    handler = initializingHandler;
    handler.deps++;
  } else {
    handler = initializingHandler = {
      parent: null,
      chunk: null,
      value: null,
      deps: 1,
      errored: false,
    };
  }

  function fulfill(value: any): void {
    for (let i = 1; i < path.length; i++) {
      while (value.$$typeof === REACT_LAZY_TYPE) {
        // We never expect to see a Lazy node on this path because we encode those as
        // separate models. This must mean that we have inserted an extra lazy node
Repository: facebook/react
File: packages/react-reconciler/src/ReactFiberPerformanceTrack.js (JavaScript)

Code before:
  start: -0,
  end: -0,
  detail: {
    devtools: reusableComponentDevToolDetails,
  },
};

export function setCurrentTrackFromLanes(lanes: number): void {
  reusableComponentDevToolDetails.track =
    getGroupNameOfHighestPriorityLane(lanes);
}

export function logComponentRender(
  fiber: Fiber,
  startTime: number,
  endTime: number,
): void {
  const name = getComponentNameFromFiber(fiber);
  if (name === null) {
    // Skip
    return;
  }
  if (supportsUserTiming) {
    let selfTime: number = (fiber.actualDuration: any);
    for (let child = fiber.child; child !== null; child = child.sibling) {
      selfTime -= (child.actualDuration: any);
    }
    reusableComponentDevToolDetails.color =
      selfTime < 0.5
        ? 'primary-light'
        : selfTime < 10
          ? 'primary'
          : selfTime < 100
            ? 'primary-dark'
            : 'error';
    reusableComponentOptions.start = startTime;
    reusableComponentOptions.end = endTime;
    performance.measure(name, reusableComponentOptions);
  }
}

export function logComponentEffect(
  fiber: Fiber,
  startTime: number,
  endTime: number,
  selfTime: number,
): void {
  const name = getComponentNameFromFiber(fiber);
  if (name === null) {
    // Skip
    return;

Review comment (suggestion):
I don't love this loop for every rendered component. We could maybe change how we keep track of this but that might require another field. We could also maybe keep track of this by reusing the commit loop.

Code after:
  start: -0,
  end: -0,
  detail: {
    devtools: reusableComponentDevToolDetails,
  },
};

export function setCurrentTrackFromLanes(lanes: number): void {
  reusableComponentDevToolDetails.track =
    getGroupNameOfHighestPriorityLane(lanes);
}

export function logComponentRender(
  fiber: Fiber,
  startTime: number,
  endTime: number,
): void {
  const name = getComponentNameFromFiber(fiber);
  if (name === null) {
    // Skip
    return;
  }
  if (supportsUserTiming) {
    let selfTime: number = (fiber.actualDuration: any);
    if (fiber.alternate === null || fiber.alternate.child !== fiber.child) {
      for (let child = fiber.child; child !== null; child = child.sibling) {
        selfTime -= (child.actualDuration: any);
      }
    }
    reusableComponentDevToolDetails.color =
      selfTime < 0.5
        ? 'primary-light'
        : selfTime < 10
          ? 'primary'
          : selfTime < 100
            ? 'primary-dark'
            : 'error';
    reusableComponentOptions.start = startTime;
    reusableComponentOptions.end = endTime;
    performance.measure(name, reusableComponentOptions);
  }
}

export function logComponentEffect(
  fiber: Fiber,
  startTime: number,
  endTime: number,
  selfTime: number,
): void {
  const name = getComponentNameFromFiber(fiber);
  if (name === null) {