ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js (JavaScript)

Code before:
        throw never;
      }
      function onPopstate() {
        React.startTransition(() => {
          setShouldSuspend(val => !val);
        });
      }
      React.useEffect(() => {
        window.addEventListener('popstate', onPopstate);
        return () => window.removeEventListener('popstate', onPopstate);
      }, []);
      Scheduler.log(`Child:${shouldSuspend}/${text}`);
      return text;
    }

    const root = ReactDOMClient.createRoot(container);
    await act(async () => {
      root.render(<App />);
    });
    assertLog(['Child:false/0']);

    await act(() => {
      const popStateEvent = new Event('popstate');
      window.event = popStateEvent;
      window.dispatchEvent(popStateEvent);
      queueMicrotask(() => {
        window.event = undefined;
      });
    });
    assertLog(['Suspend!']);

    await act(async () => {
      _setText('1');
    });
    assertLog(['Child:false/1', 'Suspend!']);

    root.unmount();
  });
});


Review comment (bug):
If the test fails on this line, `window.event` cleanup won't get scheduled. Do you mind clearing `window.event` in `beforeEach` in the test suite to avoid this mistake and to ensure `window.event` is always empty at the beginning of each test?

Code after:
        Scheduler.log('Suspend!');
        throw never;
      }
      function onPopstate() {
        React.startTransition(() => {
          setShouldSuspend(val => !val);
        });
      }
      React.useEffect(() => {
        window.addEventListener('popstate', onPopstate);
        return () => window.removeEventListener('popstate', onPopstate);
      }, []);
      Scheduler.log(`Child:${shouldSuspend}/${text}`);
      return text;
    }

    const root = ReactDOMClient.createRoot(container);
    await act(async () => {
      root.render(<App />);
    });
    assertLog(['Child:false/0']);

    await act(() => {
      const popStateEvent = new Event('popstate');
      window.event = popStateEvent;
      window.dispatchEvent(popStateEvent);
      queueMicrotask(() => {
        window.event = undefined;
      });
    });
    assertLog(['Suspend!']);

    await act(async () => {
      _setText('1');
    });
    assertLog(['Child:false/1', 'Suspend!']);

    root.unmount();
  });
});
Repository: facebook/react
File: packages/react-reconciler/src/__tests__/ReactTransition-test.js (JavaScript)

Code before:

    expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
  });

  it('tracks two pending flags for nested startTransition (#26226)', async () => {
    let update;
    function App() {
      const [isPendingA, startTransitionA] = useTransition();
      const [isPendingB, startTransitionB] = useTransition();
      const [state, setState] = useState(0);

      update = function () {
        startTransitionA(() => {
          startTransitionB(() => {
            setState(1);
          });
        });
      };

      return (
        <>
          <Text text={state} />
          {', '}
          <Text text={'A ' + isPendingA} />
          {', '}
          <Text text={'B ' + isPendingB} />
        </>
      );
    }
    const root = ReactNoop.createRoot();
    await act(async () => {
      root.render(<App />);
    });
    expect(Scheduler).toHaveYielded([0, 'A false', 'B false']);
    expect(root).toMatchRenderedOutput('0, A false, B false');

    await act(async () => {
      update();
    });
    expect(Scheduler).toHaveYielded([
      0,
      'A true',
      'B true',
      1,
      'A false',
      'B false',
    ]);
    expect(root).toMatchRenderedOutput('1, A false, B false');
  });
});


Review comment (suggestion):
Wanna update this to use `assertLog`? If not I can fix post merge.

Code after:
      const [isPendingB, startTransitionB] = useTransition();
      const [state, setState] = useState(0);

      update = function () {
        startTransitionA(() => {
          startTransitionB(() => {
            setState(1);
          });
        });
      };

      return (
        <>
          <Text text={state} />
          {', '}
          <Text text={'A ' + isPendingA} />
          {', '}
          <Text text={'B ' + isPendingB} />
        </>
      );
    }
    const root = ReactNoop.createRoot();
    await act(async () => {
      root.render(<App />);
    });
    assertLog([0, 'A false', 'B false']);
    expect(root).toMatchRenderedOutput('0, A false, B false');

    await act(async () => {
      update();
    });
    assertLog([0, 'A true', 'B true', 1, 'A false', 'B false']);
    expect(root).toMatchRenderedOutput('1, A false, B false');
  });
});
Repository: facebook/react
File: scripts/rollup/build-all-release-channels.js (JavaScript)

Code before:
'use strict';

/* eslint-disable no-for-of-loops/no-for-of-loops */

const fs = require('fs');
const fse = require('fs-extra');
const {spawnSync} = require('child_process');
const path = require('path');
const tmp = require('tmp');

const {
  ReactVersion,
  stablePackages,
  experimentalPackages,
  nextChannelLabel,
} = require('../../ReactVersions');

// Runs the build script for both stable and experimental release channels,
// by configuring an environment variable.

const sha = String(
  spawnSync('git', ['show', '-s', '--no-show-signature', '--format=%h']).stdout
).trim();

let dateString = String(
  spawnSync('git', [
    'show',
    '-s',
    '--no-show-signature',
    '--format=%cd',

Review comment (suggestion):
fyi, even if I install the latest dependencies i cannot run `yarn build react-dom` without this erroring on `cannot find module`

Code after:
'use strict';

/* eslint-disable no-for-of-loops/no-for-of-loops */

const crypto = require('node:crypto');
const fs = require('fs');
const fse = require('fs-extra');
const {spawnSync} = require('child_process');
const path = require('path');
const tmp = require('tmp');

const {
  ReactVersion,
  stablePackages,
  experimentalPackages,
  nextChannelLabel,
} = require('../../ReactVersions');

// Runs the build script for both stable and experimental release channels,
// by configuring an environment variable.

const sha = String(
  spawnSync('git', ['show', '-s', '--no-show-signature', '--format=%h']).stdout
).trim();

let dateString = String(
  spawnSync('git', [
    'show',
    '-s',
    '--no-show-signature',
Repository: facebook/react
File: packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.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
 */

import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';

describe('Store component filters', () => {
  let React;
  let Types;
  let bridge: FrontendBridge;
  let legacyRender;
  let store: Store;
  let utils;
  let internalAct;

  const act = async (callback: Function) => {
    internalAct(callback);
    jest.runAllTimers(); // Flush Bridge operations
  };

  beforeEach(() => {
    bridge = global.bridge;
    store = global.store;
    store.collapseNodesByDefault = false;
    store.componentFilters = [];
    store.recordChangeDescriptions = true;

    React = require('react');
    Types = require('react-devtools-shared/src/types');
    utils = require('./utils');
    internalAct = require('internal-test-utils').act;

    legacyRender = utils.legacyRender;
  });

  // @reactVersion >= 16.0
  it('should throw if filters are updated while profiling', async () => {
    await act(async () => store.profilerStore.startProfiling());
    expect(() => (store.componentFilters = [])).toThrow(
      'Cannot modify filter preferences while profiling',
    );
  });

Review comment (bug):
Missed this in https://github.com/facebook/react/commit/702fc984e68e852c76e0df608cd631a7c2e7763f. The warning was firing but it wasn't causing the test to fail. Changing it to a hard error surfaced it.

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
 */

import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';

describe('Store component filters', () => {
  let React;
  let Types;
  let bridge: FrontendBridge;
  let legacyRender;
  let store: Store;
  let utils;
  let internalAct;

  const act = async (callback: Function) => {
    await internalAct(callback);
    jest.runAllTimers(); // Flush Bridge operations
  };

  beforeEach(() => {
    bridge = global.bridge;
    store = global.store;
    store.collapseNodesByDefault = false;
    store.componentFilters = [];
    store.recordChangeDescriptions = true;

    React = require('react');
    Types = require('react-devtools-shared/src/types');
    utils = require('./utils');
    internalAct = require('internal-test-utils').act;

    legacyRender = utils.legacyRender;
  });

  // @reactVersion >= 16.0
  it('should throw if filters are updated while profiling', async () => {
    await act(async () => store.profilerStore.startProfiling());
    expect(() => (store.componentFilters = [])).toThrow(
      'Cannot modify filter preferences while profiling',
    );
  });
Repository: facebook/react
File: packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.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.
 *
 * @emails react-core
 */

'use strict';

// Polyfills for test environment
global.ReadableStream =
  require('web-streams-polyfill/ponyfill/es6').ReadableStream;
global.TextEncoder = require('util').TextEncoder;
global.TextDecoder = require('util').TextDecoder;

let clientExports;
let webpackMap;
let webpackModules;
let React;
let ReactDOMServer;
let ReactServerDOMWriter;
let ReactServerDOMReader;
let use;

describe('ReactFlightDOMEdge', () => {
  beforeEach(() => {
    jest.resetModules();
    const WebpackMock = require('./utils/WebpackMock');
    clientExports = WebpackMock.clientExports;
    webpackMap = WebpackMock.webpackMap;
    webpackModules = WebpackMock.webpackModules;
    React = require('react');
    ReactDOMServer = require('react-dom/server.edge');
    ReactServerDOMWriter = require('react-server-dom-webpack/server.edge');
    ReactServerDOMReader = require('react-server-dom-webpack/client.edge');
    use = React.use;
  });

  async function readResult(stream) {
    const reader = stream.getReader();
    let result = '';
    while (true) {

Review comment (suggestion):
Need a better plan for testing "act" things in Flight/Fizz. There are other such todos in the Node.js tests already.

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.
 *
 * @emails react-core
 */

'use strict';

// Polyfills for test environment
global.ReadableStream =
  require('web-streams-polyfill/ponyfill/es6').ReadableStream;
global.TextEncoder = require('util').TextEncoder;
global.TextDecoder = require('util').TextDecoder;

// Don't wait before processing work on the server.
// TODO: we can replace this with FlightServer.act().
global.setTimeout = cb => cb();

let clientExports;
let webpackMap;
let webpackModules;
let React;
let ReactDOMServer;
let ReactServerDOMWriter;
let ReactServerDOMReader;
let use;

describe('ReactFlightDOMEdge', () => {
  beforeEach(() => {
    jest.resetModules();
    const WebpackMock = require('./utils/WebpackMock');
    clientExports = WebpackMock.clientExports;
    webpackMap = WebpackMock.webpackMap;
    webpackModules = WebpackMock.webpackModules;
    React = require('react');
    ReactDOMServer = require('react-dom/server.edge');
    ReactServerDOMWriter = require('react-server-dom-webpack/server.edge');
    ReactServerDOMReader = require('react-server-dom-webpack/client.edge');
    use = React.use;
  });
Repository: facebook/react
File: packages/react-server-dom-webpack/src/ReactFlightServerWebpackBundlerConfig.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
 */

import type {ReactClientValue} from 'react-server/src/ReactFlightServer';

type WebpackMap = {
  [id: string]: ClientReferenceMetadata,
};

export type BundlerConfig = WebpackMap;

export type ServerReference<T: Function> = T & {
  $$typeof: symbol,
  $$id: string,
  $$bound: Array<ReactClientValue>,
};

export type ServerReferenceId = string;

// eslint-disable-next-line no-unused-vars
export type ClientReference<T> = {
  $$typeof: symbol,
  $$id: string,
  $$async: boolean,
};

export type ClientReferenceMetadata = {
  id: string,
  chunks: Array<string>,
  name: string,
  async: boolean,
};

export type ClientReferenceKey = string;

const CLIENT_REFERENCE_TAG = Symbol.for('react.client.reference');
const SERVER_REFERENCE_TAG = Symbol.for('react.server.reference');

export function getClientReferenceKey(
  reference: ClientReference<any>,

Review comment (bug):
why isn't the type `$$bound: null | Array<ReactClientValue>`?

in `getServerReferenceBoundArguments` it is passed this type and reads this value but then after calling that you null check the return value. I believe it can return null and that this type is wrong but I'm not sure if I'm just missing something?

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
 */

import type {ReactClientValue} from 'react-server/src/ReactFlightServer';

export type ClientManifest = {
  [id: string]: ClientReferenceMetadata,
};

export type ServerReference<T: Function> = T & {
  $$typeof: symbol,
  $$id: string,
  $$bound: null | Array<ReactClientValue>,
};

export type ServerReferenceId = string;

// eslint-disable-next-line no-unused-vars
export type ClientReference<T> = {
  $$typeof: symbol,
  $$id: string,
  $$async: boolean,
};

export type ClientReferenceMetadata = {
  id: string,
  chunks: Array<string>,
  name: string,
  async: boolean,
};

export type ClientReferenceKey = string;

const CLIENT_REFERENCE_TAG = Symbol.for('react.client.reference');
const SERVER_REFERENCE_TAG = Symbol.for('react.server.reference');

export function getClientReferenceKey(
  reference: ClientReference<any>,
): ClientReferenceKey {
  return reference.$$async ? reference.$$id + '#async' : reference.$$id;
Repository: facebook/react
File: packages/react-native-renderer/src/ReactNativeComponentTree.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.
 */

const instanceCache = new Map();
const instanceProps = new Map();

export function precacheFiberNode(hostInst, tag) {
  instanceCache.set(tag, hostInst);
}

export function uncacheFiberNode(tag) {
  instanceCache.delete(tag);
  instanceProps.delete(tag);
}

function getInstanceFromTag(tag) {
  return instanceCache.get(tag) || null;
}

function getTagFromInstance(inst) {
  let nativeInstance = inst.stateNode;
  let tag = nativeInstance._nativeTag;
  if (tag === undefined) {
    // For compatibility with Fabric
    tag = nativeInstance.canonical && nativeInstance.canonical.nativeTag;
    nativeInstance =
      nativeInstance.canonical && nativeInstance.canonical.publicInstance;
  }

  if (!tag) {
    throw new Error('All native instances should have a tag.');
  }

  return nativeInstance;
}

export {
  getInstanceFromTag as getClosestInstanceFromNode,
  getInstanceFromTag as getInstanceFromNode,
  getTagFromInstance as getNodeFromInstance,
};

export function getFiberCurrentPropsFromNode(stateNode) {
  return instanceProps.get(stateNode._nativeTag) || null;
}

export function updateFiberProps(tag, props) {
  instanceProps.set(tag, props);

Review comment (suggestion):
```suggestion
  if (tag === undefined && nativeInstance.canonical) {
```

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.
 */

const instanceCache = new Map();
const instanceProps = new Map();

export function precacheFiberNode(hostInst, tag) {
  instanceCache.set(tag, hostInst);
}

export function uncacheFiberNode(tag) {
  instanceCache.delete(tag);
  instanceProps.delete(tag);
}

function getInstanceFromTag(tag) {
  return instanceCache.get(tag) || null;
}

function getTagFromInstance(inst) {
  let nativeInstance = inst.stateNode;
  let tag = nativeInstance._nativeTag;
  if (tag === undefined && nativeInstance.canonical != null) {
    // For compatibility with Fabric
    tag = nativeInstance.canonical.nativeTag;
    nativeInstance = nativeInstance.canonical.publicInstance;
  }

  if (!tag) {
    throw new Error('All native instances should have a tag.');
  }

  return nativeInstance;
}

export {
  getInstanceFromTag as getClosestInstanceFromNode,
  getInstanceFromTag as getInstanceFromNode,
  getTagFromInstance as getNodeFromInstance,
};

export function getFiberCurrentPropsFromNode(stateNode) {
  return instanceProps.get(stateNode._nativeTag) || null;
}

export function updateFiberProps(tag, props) {
  instanceProps.set(tag, props);
}
Repository: facebook/react
File: packages/react/src/ReactHooks.js (JavaScript)

Code before:
  return dispatcher.useSyncExternalStore(
    subscribe,
    getSnapshot,
    getServerSnapshot,
  );
}

export function useCacheRefresh(): <T>(?() => T, ?T) => void {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useCacheRefresh();
}

export function use<T>(usable: Usable<T>): T {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.use(usable);
}

export function useMemoCache(size: number): Array<any> {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useMemoCache(size);
}

export function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
  callback: F,
): F {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useEffectEvent(callback);
}


Review comment (suggestion):
This is a little bit shorter, we don't have to define the return type in the type constraint for F:
```suggestion
export function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
export function useEffectEvent<Args, F: (...Args) => mixed>(
```

Code after:
  return dispatcher.useSyncExternalStore(
    subscribe,
    getSnapshot,
    getServerSnapshot,
  );
}

export function useCacheRefresh(): <T>(?() => T, ?T) => void {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useCacheRefresh();
}

export function use<T>(usable: Usable<T>): T {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.use(usable);
}

export function useMemoCache(size: number): Array<any> {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useMemoCache(size);
}

export function useEffectEvent<Args, F: (...Array<Args>) => mixed>(
  callback: F,
): F {
  const dispatcher = resolveDispatcher();
  // $FlowFixMe[not-a-function] This is unstable, thus optional
  return dispatcher.useEffectEvent(callback);
}
Repository: facebook/react
File: packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js (JavaScript)

Code before:
    const Immutable = require('immutable');

    const Example = () => null;

    const set = new Set(['abc', 123]);
    const map = new Map([
      ['name', 'Brian'],
      ['food', 'sushi'],
    ]);
    const setOfSets = new Set([new Set(['a', 'b', 'c']), new Set([1, 2, 3])]);
    const mapOfMaps = new Map([
      ['first', map],
      ['second', map],
    ]);
    const typedArray = Int8Array.from([100, -100, 0]);
    const arrayBuffer = typedArray.buffer;
    const dataView = new DataView(arrayBuffer);
    const immutable = Immutable.fromJS({
      a: [{hello: 'there'}, 'fixed', true],
      b: 123,
      c: {
        '1': 'xyz',
        xyz: 1,
      },
    });
    // $FlowFixMe
    const bigInt = BigInt(123); // eslint-disable-line no-undef

    act(() =>
      ReactDOM.render(
        <Example
          arrayBuffer={arrayBuffer}
          dataView={dataView}
          map={map}
          set={set}
          mapOfMaps={mapOfMaps}
          setOfSets={setOfSets}
          typedArray={typedArray}
          immutable={immutable}
          bigInt={bigInt}
        />,
        document.createElement('div'),
      ),
    );

    const id = ((store.getElementIDAtIndex(0): any): number);
    const rendererID = ((store.getRendererIDForElement(id): any): number);

    // Should copy the whole value (not just the hydrated parts)
    backendAPI.copyInspectedElementPath({
      bridge,

Review comment (bug):
And we should probably also start failing lint on unused suppressions because that is unneeded as of https://github.com/facebook/react/pull/26479 

Code after:
    const Immutable = require('immutable');

    const Example = () => null;

    const set = new Set(['abc', 123]);
    const map = new Map([
      ['name', 'Brian'],
      ['food', 'sushi'],
    ]);
    const setOfSets = new Set([new Set(['a', 'b', 'c']), new Set([1, 2, 3])]);
    const mapOfMaps = new Map([
      ['first', map],
      ['second', map],
    ]);
    const typedArray = Int8Array.from([100, -100, 0]);
    const arrayBuffer = typedArray.buffer;
    const dataView = new DataView(arrayBuffer);
    const immutable = Immutable.fromJS({
      a: [{hello: 'there'}, 'fixed', true],
      b: 123,
      c: {
        '1': 'xyz',
        xyz: 1,
      },
    });
    const bigInt = BigInt(123); // eslint-disable-line no-undef

    act(() =>
      ReactDOM.render(
        <Example
          arrayBuffer={arrayBuffer}
          dataView={dataView}
          map={map}
          set={set}
          mapOfMaps={mapOfMaps}
          setOfSets={setOfSets}
          typedArray={typedArray}
          immutable={immutable}
          bigInt={bigInt}
        />,
        document.createElement('div'),
      ),
    );

    const id = ((store.getElementIDAtIndex(0): any): number);
    const rendererID = ((store.getRendererIDForElement(id): any): number);

    // Should copy the whole value (not just the hydrated parts)
    backendAPI.copyInspectedElementPath({
      bridge,
      id,