ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/react
File: packages/react-client/src/ReactFlightClient.js (JavaScript)

Code before:
        );
      } else {
        resolveErrorProd(response, id, errorInfo.digest);
      }
      return;
    }
    case 84 /* "T" */: {
      resolveText(response, id, row);
      return;
    }
    default: {
      // We assume anything else is JSON.
      resolveModel(response, id, row);
      return;
    }
  }
}

export function processBinaryChunk(
  response: Response,
  chunk: Uint8Array,
): void {
  let i = 0;
  while (i < chunk.length) {
    let lastIdx = -1;
    switch (response._rowState) {
      case ROW_ID: {
        const byte = chunk[i++];
        if (byte === 58 /* ":" */) {
          // Finished the rowID, next we'll parse the tag.
          response._rowState = ROW_TAG;
        } else {
          response._rowID =
            (response._rowID << 4) | (byte > 96 ? byte - 87 : byte - 48);
        }
        continue;
      }
      case ROW_TAG: {
        const resolvedRowTag = chunk[i];
        if (resolvedRowTag === 84 /* "T" */) {
          response._rowTag = resolvedRowTag;
          response._rowState = ROW_LENGTH;
          i++;
        } else if (resolvedRowTag > 64 && resolvedRowTag < 91 /* "A"-"Z" */) {
          response._rowTag = resolvedRowTag;
          response._rowState = ROW_CHUNK_BY_NEWLINE;
          i++;
        } else {
          response._rowTag = 0;
          response._rowState = ROW_CHUNK_BY_NEWLINE;
          // This was an unknown tag so it was probably part of the data.

Review comment (suggestion):
I suggest copying all these property accesses into local variables and only assigning as properties if necessary at the end

Code after:
      return;
    }
    case 84 /* "T" */: {
      resolveText(response, id, row);
      return;
    }
    default: {
      // We assume anything else is JSON.
      resolveModel(response, id, row);
      return;
    }
  }
}

export function processBinaryChunk(
  response: Response,
  chunk: Uint8Array,
): void {
  let i = 0;
  let rowState = response._rowState;
  let rowID = response._rowID;
  let rowTag = response._rowTag;
  let rowLength = response._rowLength;
  const buffer = response._buffer;
  const chunkLength = chunk.length;
  while (i < chunkLength) {
    let lastIdx = -1;
    switch (rowState) {
      case ROW_ID: {
        const byte = chunk[i++];
        if (byte === 58 /* ":" */) {
          // Finished the rowID, next we'll parse the tag.
          rowState = ROW_TAG;
        } else {
          rowID = (rowID << 4) | (byte > 96 ? byte - 87 : byte - 48);
        }
        continue;
      }
      case ROW_TAG: {
        const resolvedRowTag = chunk[i];
        if (resolvedRowTag === 84 /* "T" */) {
          rowTag = resolvedRowTag;
          rowState = ROW_LENGTH;
          i++;
        } else if (resolvedRowTag > 64 && resolvedRowTag < 91 /* "A"-"Z" */) {
          rowTag = resolvedRowTag;
          rowState = ROW_CHUNK_BY_NEWLINE;
          i++;
        } else {
          rowTag = 0;
          rowState = ROW_CHUNK_BY_NEWLINE;
Repository: facebook/react
File: packages/react/index.classic.fb.js (JavaScript)

Code before:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow
 */

export {
  __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED,
  act as unstable_act,
  Children,
  Component,
  Fragment,
  Profiler,
  PureComponent,
  StrictMode,
  Suspense,
  SuspenseList,
  cloneElement,
  createContext,
  createElement,
  createFactory,
  createRef,
  createServerContext,
  use,
  forwardRef,
  isValidElement,
  lazy,
  memo,
  cache,
  startTransition,
  unstable_Cache,
  unstable_TracingMarker,
  unstable_DebugTracingMode,
  unstable_LegacyHidden,
  unstable_Offscreen,
  unstable_Scope,
  unstable_getCacheSignal,
  unstable_getCacheForType,
  unstable_useCacheRefresh,
  unstable_useMemoCache,
  useId,
  useCallback,
  useContext,

Review comment (suggestion):
This one technically isn't stable yet, despite being used at Meta ("Stage 2.5"). If anything I'd remove the unprefixed one and keep the unstable one. You can move the alias to the wrapper module in Meta's repo.

Code after:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow
 */

export {
  __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED,
  act as unstable_act,
  Children,
  Component,
  Fragment,
  Profiler,
  PureComponent,
  StrictMode,
  Suspense,
  SuspenseList,
  SuspenseList as unstable_SuspenseList, // TODO: Remove once call sights updated to SuspenseList
  cloneElement,
  createContext,
  createElement,
  createFactory,
  createRef,
  createServerContext,
  use,
  forwardRef,
  isValidElement,
  lazy,
  memo,
  cache,
  startTransition,
  unstable_Cache,
  unstable_TracingMarker,
  unstable_DebugTracingMode,
  unstable_LegacyHidden,
  unstable_Offscreen,
  unstable_Scope,
  unstable_getCacheSignal,
  unstable_getCacheForType,
  unstable_useCacheRefresh,
  unstable_useMemoCache,
  useId,
  useCallback,
Repository: facebook/react
File: packages/scheduler/src/forks/SchedulerFeatureFlags.www.js (JavaScript)

Code before:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow
 */

const {
  enableProfiling: enableProfilingFeatureFlag,
  enableSchedulerYield: enableSchedulerYieldFeatureFlag,
} =
  // $FlowFixMe[cannot-resolve-module]
  require('SchedulerFeatureFlags');

export const enableSchedulerDebugging = true;
export const enableProfiling: boolean =
  __PROFILE__ && enableProfilingFeatureFlag;
export const enableSchedulerYield = enableSchedulerYieldFeatureFlag;
export const enableIsInputPending = true;
export const enableIsInputPendingContinuous = true;
export const frameYieldMs = 5;
export const continuousYieldMs = 10;
export const maxYieldMs = 10;


Review comment (suggestion):
i had overlooked updating this before, which is (apparently) what broke the build with these changes

Code after:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow
 */

const {enableProfiling: enableProfilingFeatureFlag} =
  // $FlowFixMe[cannot-resolve-module]
  require('SchedulerFeatureFlags');

export const enableSchedulerDebugging = true;
export const enableProfiling: boolean =
  __PROFILE__ && enableProfilingFeatureFlag;
export const enableIsInputPending = true;
export const enableIsInputPendingContinuous = true;
export const frameYieldMs = 5;
export const continuousYieldMs = 10;
export const maxYieldMs = 10;
Repository: facebook/react
File: packages/react-devtools-shared/src/inspectedElementCache.js (JavaScript)

Code before:
          const [key, value] = createCacheSeed(element, inspectedElement);
          refresh(key, value);
        });
      }
    },
  );
}

function createPromiseWhichResolvesInOneSecond() {
  return new Promise(resolve => setTimeout(resolve, 1000));
}

export function startElementUpdatesPolling({
  bridge,
  element,
  refresh,
  store,
}: {
  bridge: FrontendBridge,
  element: Element,
  refresh: RefreshFunction,
  store: Store,
}): {abort: () => void, pause: () => void, resume: () => void} {
  let aborted = false;
  let paused = false;
  let running = false;

  function abort() {
    aborted = true;
  }

  function resume() {
    if (running) {
      return;
    }

    paused = false;
    poll();
  }

  function pause() {
    if (paused) {
      return;
    }

    running = false;
    paused = true;
  }

  function poll(): Promise<void> {
    running = true;

Review comment (refactor):
this seems a bit brittle to have 3 separate state variables, as they could go out of sync in a future refactor. would it make sense to combine them into a single `status` variable that is either aborted, paused or running?

Code after:
          const [key, value] = createCacheSeed(element, inspectedElement);
          refresh(key, value);
        });
      }
    },
  );
}

function createPromiseWhichResolvesInOneSecond() {
  return new Promise(resolve => setTimeout(resolve, 1000));
}

type PollingStatus = 'idle' | 'running' | 'paused' | 'aborted';

export function startElementUpdatesPolling({
  bridge,
  element,
  refresh,
  store,
}: {
  bridge: FrontendBridge,
  element: Element,
  refresh: RefreshFunction,
  store: Store,
}): {abort: () => void, pause: () => void, resume: () => void} {
  let status: PollingStatus = 'idle';

  function abort() {
    status = 'aborted';
  }

  function resume() {
    if (status === 'running' || status === 'aborted') {
      return;
    }

    status = 'idle';
    poll();
  }

  function pause() {
    if (status === 'paused' || status === 'aborted') {
      return;
    }

    status = 'paused';
  }

  function poll(): Promise<void> {
    status = 'running';
Repository: facebook/react
File: packages/react-devtools-shared/src/inspectedElementCache.js (JavaScript)

Code before:
    aborted = true;
  }

  function resume() {
    if (running) {
      return;
    }

    paused = false;
    poll();
  }

  function pause() {
    if (paused) {
      return;
    }

    running = false;
    paused = true;
  }

  function poll(): Promise<void> {
    running = true;

    return Promise.allSettled([
      checkForUpdate({bridge, element, refresh, store}),
      createPromiseWhichResolvesInOneSecond(),
    ])
      .then(([{status: updateStatus, reason}]) => {
        // There isn't much to do about errors in this case,
        // but we should at least log them, so they aren't silent.
        // Log only if polling is still active, we can't handle the case when
        // request was sent, and then bridge was remounted (for example, when user did navigate to a new page),
        // but at least we can mark that polling was aborted
        if (updateStatus === 'rejected' && !aborted) {
          // This is expected Promise rejection, no need to log it
          if (reason instanceof ElementPollingCancellationError) {
            return;
          }

          console.error(reason);
        }
      })
      .finally(() => {
        running = false;

        if (!aborted && !paused) {
          return poll();
        }
      });
  }

Review comment (suggestion):
feel free to ignore, don't feel too strongly: should this per second throttling be part of this or the caller? it's not entirely clear this function does this

Code after:
    status = 'aborted';
  }

  function resume() {
    if (status === 'running' || status === 'aborted') {
      return;
    }

    status = 'idle';
    poll();
  }

  function pause() {
    if (status === 'paused' || status === 'aborted') {
      return;
    }

    status = 'paused';
  }

  function poll(): Promise<void> {
    status = 'running';

    return Promise.allSettled([
      checkForUpdate({bridge, element, refresh, store}),
      createPromiseWhichResolvesInOneSecond(),
    ])
      .then(([{status: updateStatus, reason}]) => {
        // There isn't much to do about errors in this case,
        // but we should at least log them, so they aren't silent.
        // Log only if polling is still active, we can't handle the case when
        // request was sent, and then bridge was remounted (for example, when user did navigate to a new page),
        // but at least we can mark that polling was aborted
        if (updateStatus === 'rejected' && status !== 'aborted') {
          // This is expected Promise rejection, no need to log it
          if (reason instanceof ElementPollingCancellationError) {
            return;
          }

          console.error(reason);
        }
      })
      .finally(() => {
        const shouldContinuePolling =
          status !== 'aborted' && status !== 'paused';

        status = 'idle';

        if (shouldContinuePolling) {
          return poll();
        }
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js (JavaScript)

Code before:
          </Suspense>
        </div>
      );
    }

    const prerendered = await ReactDOMFizzStatic.prerender(<App />);
    expect(prerendered.postponed).not.toBe(null);

    prerendering = false;

    const resumed = await ReactDOMFizzServer.resume(
      <App />,
      prerendered.postponed,
    );

    await readIntoContainer(prerendered.prelude);

    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    await readIntoContainer(resumed);

    // TODO: expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
  });

  it('supports postponing in lazy in prerender and resuming later', async () => {
    let prerendering = true;
    const Hole = React.lazy(async () => {
      React.unstable_postpone();
    });

    function Postpone() {
      return 'Hello';
    }

    function App() {
      return (
        <div>
          <Suspense fallback="Loading...">
            Hi
            {prerendering ? Hole : <Postpone />}
          </Suspense>
        </div>
      );
    }

    const prerendered = await ReactDOMFizzStatic.prerender(<App />);
    expect(prerendered.postponed).not.toBe(null);

    prerendering = false;

    const resumed = await ReactDOMFizzServer.resume(

Review comment (suggestion):
```suggestion
  // @gate enablePostpone
  it('supports postponing in lazy in prerender and resuming later', async () => {
```

Code after:
          </Suspense>
        </div>
      );
    }

    const prerendered = await ReactDOMFizzStatic.prerender(<App />);
    expect(prerendered.postponed).not.toBe(null);

    prerendering = false;

    const resumed = await ReactDOMFizzServer.resume(
      <App />,
      prerendered.postponed,
    );

    await readIntoContainer(prerendered.prelude);

    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    await readIntoContainer(resumed);

    // TODO: expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
  });

  // @gate enablePostpone
  it('supports postponing in lazy in prerender and resuming later', async () => {
    let prerendering = true;
    const Hole = React.lazy(async () => {
      React.unstable_postpone();
    });

    function Postpone() {
      return 'Hello';
    }

    function App() {
      return (
        <div>
          <Suspense fallback="Loading...">
            Hi
            {prerendering ? Hole : <Postpone />}
          </Suspense>
        </div>
      );
    }

    const prerendered = await ReactDOMFizzStatic.prerender(<App />);
    expect(prerendered.postponed).not.toBe(null);

    prerendering = false;
Repository: facebook/react
File: scripts/flow/environment.js (JavaScript)

Code before:
}

declare class AsyncLocalStorage<T> {
  disable(): void;
  getStore(): T | void;
  run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
  enterWith(store: T): void;
}

declare module 'async_hooks' {
  declare class AsyncLocalStorage<T> {
    disable(): void;
    getStore(): T | void;
    run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
    enterWith(store: T): void;
  }
}

declare module 'node:worker_threads' {
  declare class MessageChannel {
    port1: MessagePort;
    port2: MessagePort;
  }
}

declare var Bun: {
  hash(
    input: string | TypedArray | DataView | ArrayBuffer | SharedArrayBuffer,
  ): number,
};


Review comment (suggestion):
We should really have separate libdefs for each environment but since they're already all mixed together in this one file already I just tacked it on here. Can improve later.

Code after:
}

declare class AsyncLocalStorage<T> {
  disable(): void;
  getStore(): T | void;
  run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
  enterWith(store: T): void;
}

declare module 'async_hooks' {
  declare class AsyncLocalStorage<T> {
    disable(): void;
    getStore(): T | void;
    run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
    enterWith(store: T): void;
  }
}

declare module 'node:worker_threads' {
  declare class MessageChannel {
    port1: MessagePort;
    port2: MessagePort;
  }
}

declare var Bun: {
  hash(
    input: string | $TypedArray | DataView | ArrayBuffer | SharedArrayBuffer,
  ): number,
};
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js (JavaScript)

Code before:
    });

    const resumed = await ReactDOMFizzServer.resumeToPipeableStream(
      <App />,
      JSON.parse(JSON.stringify(prerendered.postponed)),
    );

    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Read what we've completed so far
    await act(() => {
      resumed.pipe(writable);
    });

    // Still blocked
    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Resolve the first promise, this unblocks the inner boundary
    await act(() => {
      resolveA('Hello');
    });

    // Still blocked
    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Resolve the first promise, this unblocks the inner boundary
    await act(() => {
      resolveB('World');
    });

    expect(getVisibleChildren(container)).toEqual(
      <div>
        <p>
          <span>
            {'Hello'}
            {'World'}
          </span>
        </p>
      </div>,
    );
  });
});


Review comment (suggestion):
```suggestion
    // Resolve the second promise, this unblocks the outer boundary
```

Code after:
    });

    const resumed = await ReactDOMFizzServer.resumeToPipeableStream(
      <App />,
      JSON.parse(JSON.stringify(prerendered.postponed)),
    );

    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Read what we've completed so far
    await act(() => {
      resumed.pipe(writable);
    });

    // Still blocked
    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Resolve the first promise, this unblocks the inner boundary
    await act(() => {
      resolveA('Hello');
    });

    // Still blocked
    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

    // Resolve the second promise, this unblocks the outer boundary
    await act(() => {
      resolveB('World');
    });

    expect(getVisibleChildren(container)).toEqual(
      <div>
        <p>
          <span>
            {'Hello'}
            {'World'}
          </span>
        </p>
      </div>,
    );
  });
});
Repository: facebook/react
File: packages/shared/forks/ReactFeatureFlags.native-oss.js (JavaScript)

Code before:
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const forceConcurrentByDefaultForTesting = false;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = false;
export const enableCustomElementPropertySupport = false;

export const consoleManagedByDevToolsDuringStrictMode = false;
export const enableServerContext = true;

export const enableTransitionTracing = false;

export const enableFloat = true;
export const enableHostSingletons = true;

export const useModernStrictMode = false;
export const enableDO_NOT_USE_disableStrictPassiveEffect = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;

export const enableAsyncActions = false;

export const alwaysThrottleRetries = true;

export const useMicrotasksForSchedulingInFabric = false;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);


Review comment (bug):
Open problem: I need this to get the test to run both variants, but it fails the OSS RN build

```
/home/circleci/project/build/react-native/implementations/ReactFabric-prod.js
  1317:45  error  '__VARIANT__' is not defined  no-undef
```

Code after:
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
export const forceConcurrentByDefaultForTesting = false;
export const enableUnifiedSyncLane = false;
export const allowConcurrentByDefault = false;
export const enableCustomElementPropertySupport = false;

export const consoleManagedByDevToolsDuringStrictMode = false;
export const enableServerContext = true;

export const enableTransitionTracing = false;

export const enableFloat = true;
export const enableHostSingletons = true;

export const useModernStrictMode = false;
export const enableDO_NOT_USE_disableStrictPassiveEffect = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;

export const enableAsyncActions = false;

export const alwaysThrottleRetries = true;

export const useMicrotasksForSchedulingInFabric = false;
export const passChildrenWhenCloningPersistedNodes = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
Repository: facebook/react
File: packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/ReactNativeViewConfigRegistry.js (JavaScript)

Code before:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow strict-local
 */

'use strict';

import type {
  ReactNativeBaseComponentViewConfig,
  ViewConfigGetter,
} from './ReactNativeTypes';

// Event configs
const customBubblingEventTypes = {};
const customDirectEventTypes = {};
const eventTypes = {};

exports.customBubblingEventTypes = customBubblingEventTypes;
exports.customDirectEventTypes = customDirectEventTypes;
exports.eventTypes = eventTypes;

const viewConfigCallbacks = new Map();
const viewConfigs = new Map();

function processEventTypes(
  viewConfig: ReactNativeBaseComponentViewConfig<>,
): void {
  const {bubblingEventTypes, directEventTypes} = viewConfig;

  if (__DEV__) {
    if (bubblingEventTypes != null && directEventTypes != null) {
      for (const topLevelType in directEventTypes) {
        if (bubblingEventTypes[topLevelType] != null) {
          throw new Error(
            `Event cannot be both direct and bubbling: ${topLevelType}`,
          );
        }
      }
    }
  }

  if (bubblingEventTypes != null) {
    for (const topLevelType in bubblingEventTypes) {
      if (customBubblingEventTypes[topLevelType] == null) {
        eventTypes[topLevelType] = customBubblingEventTypes[topLevelType] =

Review comment (suggestion):
I think this also needs removed from here: https://github.com/facebook/react/blob/151e75a128d0fd436dce365335b96c5686f704d4/scripts/flow/react-native-host-hooks.js#L137

Code after:
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @flow strict-local
 */

'use strict';

import {type ViewConfig} from './ReactNativeTypes';

// Event configs
export const customBubblingEventTypes = {};
export const customDirectEventTypes = {};

const viewConfigCallbacks = new Map();
const viewConfigs = new Map();

function processEventTypes(viewConfig: ViewConfig): void {
  const {bubblingEventTypes, directEventTypes} = viewConfig;

  if (__DEV__) {
    if (bubblingEventTypes != null && directEventTypes != null) {
      for (const topLevelType in directEventTypes) {
        if (bubblingEventTypes[topLevelType] != null) {
          throw new Error(
            `Event cannot be both direct and bubbling: ${topLevelType}`,
          );
        }
      }
    }
  }

  if (bubblingEventTypes != null) {
    for (const topLevelType in bubblingEventTypes) {
      if (customBubblingEventTypes[topLevelType] == null) {
        customBubblingEventTypes[topLevelType] =
          bubblingEventTypes[topLevelType];
      }
    }
  }

  if (directEventTypes != null) {
    for (const topLevelType in directEventTypes) {
      if (customDirectEventTypes[topLevelType] == null) {
        customDirectEventTypes[topLevelType] = directEventTypes[topLevelType];
      }