ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/react
File: packages/react-devtools-extensions/deploy.js (JavaScript)

Code before:
#!/usr/bin/env node

'use strict';

const {exec, execSync} = require('child_process');
const {readFileSync, writeFileSync} = require('fs');
const {join} = require('path');
const shell = require('shelljs');
const main = async buildId => {
  const root = join(__dirname, buildId);
  const buildPath = join(root, 'build');

  execSync(`node ${join(root, './build')}`, {
    cwd: __dirname,
    env: {
      ...process.env,
      NODE_ENV: 'production',
    },
    stdio: 'inherit',
  });
  shell.pushd(root);
  shell.cp(join(root, 'now.json'), join(buildPath, 'now.json'));
  shell.popd();
  const file = readFileSync(join(root, 'now.json'));
  const json = JSON.parse(file);
  const alias = json.alias[0];

  const commit = execSync('git rev-parse HEAD').toString().trim().slice(0, 7);

  let date = new Date();
  date = `${date.toLocaleDateString()} – ${date.toLocaleTimeString()}`;

  const installationInstructions =
    buildId === 'chrome'
      ? readFileSync(join(__dirname, 'deploy.chrome.html'))
      : readFileSync(join(__dirname, 'deploy.firefox.html'));

  let html = readFileSync(join(__dirname, 'deploy.html')).toString();
  html = html.replace(/%commit%/g, commit);
  html = html.replace(/%date%/g, date);
  html = html.replace(/%installation%/, installationInstructions);

  writeFileSync(join(buildPath, 'index.html'), html);

  await exec(`now deploy && now alias ${alias}`, {
    cwd: buildPath,
    stdio: 'inherit',
  });

Review comment (suggestion):
I think both `root` and `buildPath` should be absolute paths. If so, then specifying `cwd` is not required, so we can remove `pushd` and `popd` calls

Code after:
#!/usr/bin/env node

'use strict';

const {exec, execSync} = require('child_process');
const {readFileSync, writeFileSync} = require('fs');
const {join} = require('path');
const shell = require('shelljs');
const main = async buildId => {
  const root = join(__dirname, buildId);
  const buildPath = join(root, 'build');

  execSync(`node ${join(root, './build')}`, {
    cwd: __dirname,
    env: {
      ...process.env,
      NODE_ENV: 'production',
    },
    stdio: 'inherit',
  });
  shell.cp(join(root, 'now.json'), join(buildPath, 'now.json'));
  const file = readFileSync(join(root, 'now.json'));
  const json = JSON.parse(file);
  const alias = json.alias[0];

  const commit = execSync('git rev-parse HEAD').toString().trim().slice(0, 7);

  let date = new Date();
  date = `${date.toLocaleDateString()} – ${date.toLocaleTimeString()}`;

  const installationInstructions =
    buildId === 'chrome'
      ? readFileSync(join(__dirname, 'deploy.chrome.html'))
      : readFileSync(join(__dirname, 'deploy.firefox.html'));

  let html = readFileSync(join(__dirname, 'deploy.html')).toString();
  html = html.replace(/%commit%/g, commit);
  html = html.replace(/%date%/g, date);
  html = html.replace(/%installation%/, installationInstructions);

  writeFileSync(join(buildPath, 'index.html'), html);

  await exec(`now deploy && now alias ${alias}`, {
    cwd: buildPath,
    stdio: 'inherit',
  });

  console.log(`Deployed to https://${alias}.now.sh`);
Repository: facebook/react
File: packages/react-test-renderer/shallow.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.
 *
 */

import ReactShallowRenderer from 'react-shallow-renderer';
import {enableReactTestRendererWarning} from 'shared/ReactFeatureFlags';

const emptyObject = {};

export default class ReactShallowRendererWithWarning extends ReactShallowRenderer {
  render(element, context = emptyObject) {
    if (__DEV__) {
      if (enableReactTestRendererWarning === true) {
        console.warn(
          "React's Shallow Renderer export will be removed in a future release. " +
            'Please use @testing-library/react instead.',
        );
      }
    }

    return super.render(element, context);
  }
}


Review comment (suggestion):
IMO we should provide a link here, that explains the deprecation better and we can keep the recommendations up to date on that page. Here's an example of where we've done this in the past: https://legacy.reactjs.org/warnings/legacy-factories.html

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 {default} from 'react-shallow-renderer';
Repository: facebook/react
File: packages/react-dom/unstable_testing.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 * from './index.js';
export {
  createComponentSelector,
  createHasPseudoClassSelector,
  createRoleSelector,
  createTestNameSelector,
  createTextSelector,
  getFindAllNodesFailureDescription,
  findAllNodes,
  findBoundingRects,
  focusWithin,
  observeVisibleRects,
} from 'react-reconciler/src/ReactFiberReconciler';


Review comment (suggestion):
Heads up @gnoff, I needed to update the imports in the unstable_testing entrypoints so using create/hydrateRoot doesn't error. Let me know if there are any concerns, but I think this is what we want to do, at least while this is unstable.

Code after:
 * @flow
 */

export {
  createPortal,
  findDOMNode,
  flushSync,
  hydrate,
  render,
  unmountComponentAtNode,
  unstable_batchedUpdates,
  unstable_createEventHandle,
  unstable_renderSubtreeIntoContainer,
  unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
  useFormStatus,
  useFormState,
  prefetchDNS,
  preconnect,
  preload,
  preloadModule,
  preinit,
  preinitModule,
  version,
} from './index.js';

export {createRoot, hydrateRoot} from './client.js';

export {
  createComponentSelector,
  createHasPseudoClassSelector,
  createRoleSelector,
  createTestNameSelector,
  createTextSelector,
  getFindAllNodesFailureDescription,
  findAllNodes,
  findBoundingRects,
  focusWithin,
  observeVisibleRects,
} from 'react-reconciler/src/ReactFiberReconciler';
Repository: facebook/react
File: packages/shared/__tests__/ReactError-test.internal.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';

let React;
let ReactDOM;

describe('ReactError', () => {
  let globalErrorMock;

  beforeEach(() => {
    if (!__DEV__) {
      // In production, our Jest environment overrides the global Error
      // class in order to decode error messages automatically. However
      // this is a single test where we actually *don't* want to decode
      // them. So we assert that the OriginalError exists, and temporarily
      // set the global Error object back to it.
      globalErrorMock = global.Error;
      global.Error = globalErrorMock.OriginalError;
      expect(typeof global.Error).toBe('function');
    }
    jest.resetModules();
    React = require('react');
    ReactDOM = require('react-dom');
  });

  afterEach(() => {
    if (!__DEV__) {
      global.Error = globalErrorMock;
    }
  });

  // @gate build === "production"
  // @gate !source
  it('should error with minified error code', () => {
    expect(() => ReactDOM.render('Hi', null)).toThrowError(
      'Minified React error #200; visit ' +
        'https://react.dev/errors/200' +
        ' for the full message or use the non-minified dev environment' +
        ' for full errors and additional helpful warnings.',
    );
  });

  it('should serialize arguments', () => {
    function Oops() {
      return;
    }
    Oops.displayName = '#wtf';
    const container = document.createElement('div');
    expect(() => ReactDOM.render(<Oops />, container)).not.toThrowError();
  });
});


Review comment (suggestion):
Reverts this back to what it was before https://github.com/facebook/react/pull/21869

In that PR, we allow returning undefined from render. The test was updated (stupidly, by me) to just pass. But the test is to confirm arguments are serialized correctly, so the right fix is to change it to an error that includes arguments. 

I've chosen rendering an object as the error. 

Code after:
        'https://reactjs.org/docs/error-decoder.html?invariant=200' +
        ' for the full message or use the non-minified dev environment' +
        ' for full errors and additional helpful warnings.',
    );
  });

  // @gate build === "production"
  // @gate !source
  it('should serialize arguments', async () => {
    function Oops() {
      return {};
    }
    Oops.displayName = '#wtf';

    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    await expect(async () => {
      await act(async () => {
        root.render(<Oops />);
      });
    }).rejects.toThrow(
      'Minified React error #152; visit ' +
        'https://reactjs.org/docs/error-decoder.html?invariant=152&args[]=%23wtf' +
        ' for the full message or use the non-minified dev environment' +
        ' for full errors and additional helpful warnings.',
    );
  });
});
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactDOMOption-test.js (JavaScript)

Code before:
    container.innerHTML = ReactDOMServer.renderToString(children);

    expect(container.firstChild.getAttribute('value')).toBe(null);
    expect(container.firstChild.getAttribute('defaultValue')).toBe(null);

    let option = container.firstChild.firstChild;
    expect(option.nodeName).toBe('OPTION');

    expect(option.textContent).toBe('BarFooBaz');
    expect(option.selected).toBe(true);

    await expect(async () => {
      await act(async () => {
        ReactDOMClient.hydrateRoot(container, children);
      });
    }).toErrorDev(
      [
        'Warning: Text content did not match. Server: "FooBaz" Client: "Foo"',
        'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>',
        'Warning: validateDOMNesting(...): <div> cannot appear as a child of <option>',
        'Error: Text content does not match server-rendered HTML.',
        'Error: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.',
      ],
      {withoutStack: 1},
    );
    option = container.firstChild.firstChild;

    expect(option.textContent).toBe('BarFooBaz');
    expect(option.selected).toBe(true);

    expect(ref.current.nodeName).toBe('DIV');
    expect(ref.current.parentNode).toBe(option);
  });
});


Review comment (suggestion):
The entire markup is now replaced with client rendering so we need to capture the newly created `option`

Code after:
    container.innerHTML = ReactDOMServer.renderToString(children);

    expect(container.firstChild.getAttribute('value')).toBe(null);
    expect(container.firstChild.getAttribute('defaultValue')).toBe(null);

    let option = container.firstChild.firstChild;
    expect(option.nodeName).toBe('OPTION');

    expect(option.textContent).toBe('BarFooBaz');
    expect(option.selected).toBe(true);

    await expect(async () => {
      await act(async () => {
        ReactDOMClient.hydrateRoot(container, children, {
          onRecoverableError: () => {},
        });
      });
    }).toErrorDev(
      [
        'Warning: Text content did not match. Server: "FooBaz" Client: "Foo"',
        'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>',
        'Warning: validateDOMNesting(...): <div> cannot appear as a child of <option>',
      ],
      {withoutStack: 1},
    );
    option = container.firstChild.firstChild;

    expect(option.textContent).toBe('BarFooBaz');
    expect(option.selected).toBe(true);

    expect(ref.current.nodeName).toBe('DIV');
    expect(ref.current.parentNode).toBe(option);
  });
});
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactDOM-test.js (JavaScript)

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

    function Child() {
      return (
        <form>
          <input type="submit" ref={button => (buttonRef = button)} />
        </form>
      );
    }

    document.body.appendChild(container);
    try {
      ReactDOM.render(<Parent />, container);
      buttonRef.click();
      expect(count).toBe(1);
    } finally {
      document.body.removeChild(container);
    }
  });

  it('allows a DOM element to be used with a string', () => {
    const element = React.createElement('div', {className: 'foo'});
    const node = ReactTestUtils.renderIntoDocument(element);
    expect(node.tagName).toBe('DIV');
  });

  it('should allow children to be passed as an argument', () => {
    const argNode = ReactTestUtils.renderIntoDocument(
      React.createElement('div', null, 'child'),
    );
    expect(argNode.innerHTML).toBe('child');
  });

  it('should overwrite props.children with children argument', () => {
    const conflictNode = ReactTestUtils.renderIntoDocument(
      React.createElement('div', {children: 'fakechild'}, 'child'),
    );
    expect(conflictNode.innerHTML).toBe('child');
  });

  /**
   * We need to make sure that updates occur to the actual node that's in the
   * DOM, instead of a stale cache.
   */
  it('should purge the DOM cache when removing nodes', () => {
    let myDiv = ReactTestUtils.renderIntoDocument(
      <div>

Review comment (suggestion):
Wonder if we should have a test where the click/focus/etc inside of act, to show what happens if you click before the render completes, since it's async in createRoot? Maybe we have a test for that somewhere.

Code after:
    function Parent() {
      return (
        <div
          onSubmit={event => {
            event.preventDefault();
            count++;
          }}>
          <Child />
        </div>
      );
    }

    function Child() {
      return (
        <form>
          <input type="submit" ref={button => (buttonRef = button)} />
        </form>
      );
    }

    document.body.appendChild(container);
    const root = ReactDOMClient.createRoot(container);
    try {
      await act(() => {
        root.render(<Parent />);
      });
      buttonRef.click();
      expect(count).toBe(1);
    } finally {
      document.body.removeChild(container);
    }
  });

  it('allows a DOM element to be used with a string', () => {
    const element = React.createElement('div', {className: 'foo'});
    const node = ReactTestUtils.renderIntoDocument(element);
    expect(node.tagName).toBe('DIV');
  });

  it('should allow children to be passed as an argument', () => {
    const argNode = ReactTestUtils.renderIntoDocument(
      React.createElement('div', null, 'child'),
    );
    expect(argNode.innerHTML).toBe('child');
  });

  it('should overwrite props.children with children argument', () => {
    const conflictNode = ReactTestUtils.renderIntoDocument(
      React.createElement('div', {children: 'fakechild'}, 'child'),
    );
    expect(conflictNode.innerHTML).toBe('child');
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js (JavaScript)

Code before:

'use strict';

let React;
let ReactDOM;

let TestComponent;

describe('ReactCompositeComponent-state', () => {
  beforeEach(() => {
    React = require('react');
    ReactDOM = require('react-dom');

    TestComponent = class extends React.Component {
      constructor(props) {
        super(props);
        this.peekAtState('getInitialState', undefined, props);
        this.state = {color: 'red'};
      }

      peekAtState = (from, state = this.state, props = this.props) => {
        props.stateListener(from, state && state.color);
      };

      peekAtCallback = from => {
        return () => this.peekAtState(from);
      };

      setFavoriteColor(nextColor) {
        this.setState(
          {color: nextColor},
          this.peekAtCallback('setFavoriteColor'),
        );
      }

      render() {
        this.peekAtState('render');
        return <div>{this.state.color}</div>;
      }

      UNSAFE_componentWillMount() {
        this.peekAtState('componentWillMount-start');
        this.setState(function (state) {
          this.peekAtState('before-setState-sunrise', state);
        });
        this.setState(
          {color: 'sunrise'},
          this.peekAtCallback('setState-sunrise'),
        );
        this.setState(function (state) {
          this.peekAtState('after-setState-sunrise', state);

Review comment (suggestion):
Replaced the `.render` callback with an effect, which adds logs but the more logs are better here.

Code after:

'use strict';

let React;
let ReactDOM;
let ReactDOMClient;
let act;
let Scheduler;
let assertLog;
let TestComponent;
let testComponentInstance;

describe('ReactCompositeComponent-state', () => {
  beforeEach(() => {
    React = require('react');
    ReactDOM = require('react-dom');
    ReactDOMClient = require('react-dom/client');
    act = require('internal-test-utils').act;
    Scheduler = require('scheduler');

    const InternalTestUtils = require('internal-test-utils');
    assertLog = InternalTestUtils.assertLog;

    function LogAfterCommit({children, color}) {
      React.useEffect(() => {
        Scheduler.log(`commit ${color}`);
      });
      return children;
    }

    TestComponent = class extends React.Component {
      constructor(props) {
        super(props);
        this.peekAtState('getInitialState', undefined, props);
        this.state = {color: 'red'};
        testComponentInstance = this;
      }

      peekAtState = (from, state = this.state, props = this.props) => {
        Scheduler.log(`${from} ${state && state.color}`);
      };

      peekAtCallback = from => {
        return () => this.peekAtState(from);
      };

      setFavoriteColor(nextColor) {
        this.setState(
          {color: nextColor},
          this.peekAtCallback('setFavoriteColor'),
        );
Repository: facebook/react
File: packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js (JavaScript)

Code before:
        };
      };

      const el = document.createElement('div');
      ReactDOM.render(<Child incrementBy={0} />, el);
      expect(el.textContent).toBe('count:1');

      ReactDOM.render(<Child incrementBy={2} />, el);
      expect(el.textContent).toBe('count:3');

      ReactDOM.render(<Child incrementBy={1} />, el);
      expect(el.textContent).toBe('count:4');
    });
  }

  it('should support setState in componentWillUnmount', () => {
    let subscription;
    class A extends React.Component {
      componentWillUnmount() {
        subscription();
      }
      render() {
        return 'A';
      }
    }

    class B extends React.Component {
      state = {siblingUnmounted: false};
      UNSAFE_componentWillMount() {
        subscription = () => this.setState({siblingUnmounted: true});
      }
      render() {
        return 'B' + (this.state.siblingUnmounted ? ' No Sibling' : '');
      }
    }

    const el = document.createElement('div');
    ReactDOM.render(<A />, el);
    expect(el.textContent).toBe('A');

    ReactDOM.render(<B />, el);
    expect(el.textContent).toBe('B No Sibling');
  });
});


Review comment (suggestion):
`createRoot` automatically retries when we error in render.

Code after:
      render() {
        Scheduler.log(
          `render -- step: ${this.state.step}, extra: ${!!this.state.extra}`,
        );
        return null;
      }
    }

    // Mount
    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    expect(() => {
      ReactDOM.flushSync(() => {
        root.render(<Test />);
      });
    }).toErrorDev(
      'Warning: Test.componentWillMount(): Assigning directly to ' +
        "this.state is deprecated (except inside a component's constructor). " +
        'Use setState instead.',
    );

    assertLog([
      'render -- step: 3, extra: false',
      'callback -- step: 3, extra: false',

      // A second time for the retry.
      'render -- step: 3, extra: false',
      'callback -- step: 3, extra: false',
    ]);
  });

  if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
    it('should support stateful module pattern components', async () => {
      function Child() {
        return {
          state: {
            count: 123,
          },
          render() {
            return <div>{`count:${this.state.count}`}</div>;
          },
        };
      }

      const el = document.createElement('div');
      const root = ReactDOMClient.createRoot(el);
      expect(() => {
        ReactDOM.flushSync(() => {
          root.render(<Child />);
        });
      }).toErrorDev(
Repository: facebook/react
File: packages/react-dom/src/__tests__/refs-test.js (JavaScript)

Code before:
    const container = document.createElement('div');
    let result = ReactDOM.render(<div ref={ref} />, container);
    expect(ref).toHaveBeenCalledTimes(1);
    expect(ref.mock.calls[0][0]).toBeInstanceOf(HTMLDivElement);
    expect(result).toBe(ref.mock.calls[0][0]);
    ReactDOM.unmountComponentAtNode(container);
    expect(ref).toHaveBeenCalledTimes(2);
    expect(ref.mock.calls[1][0]).toBe(null);

    // composite
    class Comp extends React.Component {
      method() {
        return true;
      }
      render() {
        return <div>Comp</div>;
      }
    }

    inst = null;
    ref = jest.fn(value => (inst = value));
    result = ReactDOM.render(<Comp ref={ref} />, container);

    expect(ref).toHaveBeenCalledTimes(1);
    expect(inst).toBeInstanceOf(Comp);
    expect(result).toBe(inst);

    // ensure we have the correct instance
    expect(result.method()).toBe(true);
    expect(inst.method()).toBe(true);

    ReactDOM.unmountComponentAtNode(container);
    expect(ref).toHaveBeenCalledTimes(2);
    expect(ref.mock.calls[1][0]).toBe(null);

    // fragment
    inst = null;
    ref = jest.fn(value => (inst = value));
    let divInst = null;
    const ref2 = jest.fn(value => (divInst = value));
    result = ReactDOM.render(
      [
        <Comp ref={ref} key="a" />,
        5,
        <div ref={ref2} key="b">
          Hello
        </div>,
      ],
      container,
    );


Review comment (suggestion):
We can't test this with the new API since `root.render` does not return the instance so I copied the original test into `refsLegacy-test.js`.

Code after:
      await act(() => {
        root.render(<div ref={Symbol('foo')} />);
      });
    }).rejects.toThrow(
      'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
    );

    await act(() => {
      root.render(<div ref={undefined} />);
    });

    await act(() => {
      root.render({
        $$typeof: Symbol.for('react.element'),
        type: 'div',
        props: {},
        key: null,
        ref: null,
      });
    });

    // But this doesn't
    await expect(async () => {
      await act(() => {
        root.render({
          $$typeof: Symbol.for('react.element'),
          type: 'div',
          props: {},
          key: null,
          ref: undefined,
        });
      });
    }).rejects.toThrow(
      'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
    );
  });
});

describe('root level refs', () => {
  it('attaches and detaches root refs', async () => {
    let inst = null;

    // host node
    let ref = jest.fn(value => (inst = value));
    const container = document.createElement('div');
    let root = ReactDOMClient.createRoot(container);
    await act(() => {
      root.render(<div ref={ref} />);
    });
    let result = container.firstChild;
    expect(ref).toHaveBeenCalledTimes(1);