ronantakizawa/codereview-bench huggingface dataset:
Repository: mui/material-ui
File: packages/mui-joy/src/index.test.js (JavaScript)

Code before:
/* eslint import/namespace: ['error', { allowComputed: true }] */
/**
 * Important: This test also serves as a point to
 * import the entire lib for coverage reporting
 */

import { expect } from 'chai';
import * as materialNext from './index';

describe('@mui/material-next', () => {
  it('should have exports', () => {
    expect(typeof materialNext).to.equal('object');
  });

  it('should not have undefined exports', () => {
    Object.keys(materialNext).forEach((exportKey) =>
      expect(Boolean(materialNext[exportKey])).to.equal(true),
    );
  });
});


Review comment (suggestion):
```suggestion
    Object.keys(joy).forEach((exportKey) =>
      expect(Boolean(joy[exportKey])).to.equal(true),
```

Code after:
/* eslint import/namespace: ['error', { allowComputed: true }] */
/**
 * Important: This test also serves as a point to
 * import the entire lib for coverage reporting
 */

import { expect } from 'chai';
import * as joy from './index';

describe('@mui/joy', () => {
  it('should have exports', () => {
    expect(typeof joy).to.equal('object');
  });

  it('should not have undefined exports', () => {
    Object.keys(joy).forEach((exportKey) => expect(Boolean(joy[exportKey])).to.equal(true));
  });
});
Repository: mui/material-ui
File: docs/src/modules/components/AppNavDrawer.js (JavaScript)

Code before:
    paddingRight: 0,
    height: 'var(--MuiDocs-header-height)',
    display: 'flex',
    flexGrow: 1,
    flexDirection: 'row',
    alignItems: 'center',
    justifyContent: 'space-between',
  };
});

const StyledDrawer = styled(Drawer)(({ theme }) => {
  return {
    [theme.breakpoints.up('xs')]: {
      display: 'none',
    },
    [theme.breakpoints.up('lg')]: {
      display: 'block',
    },
  };
});

const AppNavPaperComponent = styled('div')(({ theme }) => {
  return {
    width: 'var(--MuiDocs-navDrawer-width)',
    boxShadow: 'none',
    paddingBottom: theme.spacing(4),
    [theme.breakpoints.up('xs')]: {
      borderRadius: '0px 10px 10px 0px',
    },
    [theme.breakpoints.up('sm')]: {
      borderRadius: '0px',
    },
  };
});

function renderNavItems(options) {
  const { pages, ...params } = options;

  return (
    <List disablePadding>
      {pages.reduce(
        // eslint-disable-next-line @typescript-eslint/no-use-before-define
        (items, page) => reduceChildRoutes({ items, page, ...params }),
        [],
      )}
    </List>
  );
}

/**
 * @param {object} context

Review comment (suggestion):
```suggestion
    paddingBottom: theme.spacing(5),
```

Code after:
  height: 'var(--MuiDocs-header-height)',
  display: 'flex',
  flexGrow: 1,
  flexDirection: 'row',
  alignItems: 'center',
  justifyContent: 'space-between',
}));

const StyledDrawer = styled(Drawer)(({ theme }) => ({
  [theme.breakpoints.up('xs')]: {
    display: 'none',
  },
  [theme.breakpoints.up('lg')]: {
    display: 'block',
  },
}));

const AppNavPaperComponent = styled('div')(({ theme }) => {
  return {
    width: 'var(--MuiDocs-navDrawer-width)',
    boxShadow: 'none',
    paddingBottom: theme.spacing(5),
    [theme.breakpoints.up('xs')]: {
      borderRadius: '0px 10px 10px 0px',
    },
    [theme.breakpoints.up('sm')]: {
      borderRadius: '0px',
    },
  };
});

function renderNavItems(options) {
  const { pages, ...params } = options;

  return (
    <List sx={{ my: 0.5 }}>
      {pages.reduce(
        // eslint-disable-next-line @typescript-eslint/no-use-before-define
        (items, page) => reduceChildRoutes({ items, page, ...params }),
        [],
      )}
    </List>
  );
}

/**
 * @param {object} context
 * @param {import('docs/src/pages').MuiPage} context.page
 */
function reduceChildRoutes(context) {
  const { onClose, activePage, items, depth, t } = context;
Repository: mui/material-ui
File: scripts/use-react-version.js (JavaScript)

Code before:
/* eslint-disable no-console */
/**
 * Given the dist tag fetch the corresponding
 * version and make sure this version is used throughout the repository.
 *
 * If you work on this file:
 * WARNING: This script can only use built-in modules since it has to run before
 * `yarn install`
 */
const childProcess = require('child_process');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { promisify } = require('util');

const exec = promisify(childProcess.exec);

// packages published from the react monorepo using the same version
const reactPackageNames = ['react', 'react-dom', 'react-is', 'react-test-renderer', 'scheduler'];
const devDependenciesPackageNames = ['@mnajdova/enzyme-adapter-react-18', '@testing-library/react'];

// if we need to support more versions we will need to add new mapping here
const additionalVersionsMappings = {
  17: {
    scheduler: '^0.20.0',
    '@mnajdova/enzyme-adapter-react-18': 'npm:@eps1lon/enzyme-adapter-react-17',

Review comment (suggestion):
This file is basically a rename of `use-reaect-dist-tag.js` with some changes, but for some reason GitHub is not picking it up. I tried to use `git mv` too, and it was shown as a rename in VS code, but not here.

Code after:
/* eslint-disable no-console */
/**
 * Given the dist tag fetch the corresponding
 * version and make sure this version is used throughout the repository.
 *
 * If you work on this file:
 * WARNING: This script can only use built-in modules since it has to run before
 * `yarn install`
 */
const childProcess = require('child_process');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { promisify } = require('util');

const exec = promisify(childProcess.exec);

// packages published from the react monorepo using the same version
const reactPackageNames = ['react', 'react-dom', 'react-is', 'react-test-renderer', 'scheduler'];
const devDependenciesPackageNames = ['@mnajdova/enzyme-adapter-react-18', '@testing-library/react'];

// if we need to support more versions we will need to add new mapping here
const additionalVersionsMappings = {
  17: {
    '@mnajdova/enzyme-adapter-react-18': 'npm:@eps1lon/enzyme-adapter-react-17',
    '@testing-library/react': '^12.1.0',
Repository: mui/material-ui
File: packages/mui-base/src/FormControlUnstyled/FormControlUnstyled.tsx (TypeScript)

Code before:
    children,
    component,
    disabled = false,
    error = false,
    onChange,
    required = false,
    slotProps = {},
    slots = {},
    value: incomingValue,
    ...other
  } = props;

  const [value, setValue] = useControlled({
    controlled: incomingValue,
    default: defaultValue,
    name: 'FormControl',
    state: 'value',
  });

  const filled = hasValue(value);

  const [focused, setFocused] = React.useState(false);
  if (disabled && focused) {
    // NOTE: Setting state during render?
    setFocused(false);
  }

  const ownerState: FormControlUnstyledOwnerState = {
    ...props,
    disabled,
    error,
    filled,
    focused,
    required,
  };

  const childContext: FormControlUnstyledState = React.useMemo(() => {
    return {
      disabled,
      error,
      filled,
      focused,
      onBlur: () => {
        setFocused(false);
      },
      onChange: (event: React.ChangeEvent<NativeFormControlElement>) => {
        setValue(event.target.value);
        onChange?.(event);
      },
      onFocus: () => {
        setFocused(true);

Review comment (suggestion):
Yeah, this should be in a useEffect. Good catch.

```suggestion
  React.useEffect(() => {
    if (disabled && focused) {
      setFocused(false);
    }
  }, [disabled, focused]);
```

Code after:
    children,
    component,
    disabled = false,
    error = false,
    onChange,
    required = false,
    slotProps = {},
    slots = {},
    value: incomingValue,
    ...other
  } = props;

  const [value, setValue] = useControlled({
    controlled: incomingValue,
    default: defaultValue,
    name: 'FormControl',
    state: 'value',
  });

  const filled = hasValue(value);

  const [focused, setFocused] = React.useState(false);
  if (disabled && focused) {
    setFocused(false);
  }

  const ownerState: FormControlUnstyledOwnerState = {
    ...props,
    disabled,
    error,
    filled,
    focused,
    required,
  };

  const childContext: FormControlUnstyledState = React.useMemo(() => {
    return {
      disabled,
      error,
      filled,
      focused,
      onBlur: () => {
        setFocused(false);
      },
      onChange: (event: React.ChangeEvent<NativeFormControlElement>) => {
        setValue(event.target.value);
        onChange?.(event);
      },
      onFocus: () => {
        setFocused(true);
      },
Repository: mui/material-ui
File: docs/config.js (JavaScript)

Code before:
// Valid languages to server-side render in production
const LANGUAGES = ['en', 'zh', 'pt'];

// Server side rendered languages
const LANGUAGES_SSR = ['en', 'zh', 'pt'];

// Work in progress
const LANGUAGES_IN_PROGRESS = LANGUAGES.slice();

const LANGUAGES_IGNORE_PAGES = (pathname) => {
  // We don't have the bandwidth like Qt to translate our blog posts
  // https://www.qt.io/zh-cn/blog
  if (pathname === '/blog' || pathname.startsWith('/blog/')) {
    return true;
  }

  if (pathname === '/size-snapshot/') {
    return true;
  }

  return false;
};

module.exports = {
  LANGUAGES,
  LANGUAGES_IN_PROGRESS,
  LANGUAGES_SSR,

Review comment (suggestion):
```suggestion
const LANGUAGES = ['en'];
```

https://github.com/mui/material-ui/pull/34820 was merged, we no longer support non-en languages.

Code after:
// Valid languages to server-side render in production
const LANGUAGES = ['en'];

// Server side rendered languages
const LANGUAGES_SSR = ['en'];

// Work in progress
const LANGUAGES_IN_PROGRESS = LANGUAGES.slice();

const LANGUAGES_IGNORE_PAGES = (pathname) => {
  // We don't have the bandwidth like Qt to translate our blog posts
  // https://www.qt.io/zh-cn/blog
  if (pathname === '/blog' || pathname.startsWith('/blog/')) {
    return true;
  }

  if (pathname === '/size-snapshot/') {
    return true;
  }

  return false;
};

module.exports = {
  LANGUAGES,
  LANGUAGES_IN_PROGRESS,
  LANGUAGES_SSR,
Repository: mui/material-ui
File: test/utils/components.tsx (TypeScript)

Code before:
import * as React from 'react';
import PropTypes from 'prop-types';

/**
 * A basic error boundary that can be used to assert thrown errors in render.
 * @example <ErrorBoundary ref={errorRef}><MyComponent /></ErrorBoundary>;
 *          expect(errorRef.current.errors).to.have.length(0);
 */
export class ErrorBoundary extends React.Component<{ children: React.ReactNode }> {
  static propTypes = {
    children: PropTypes.node.isRequired,
  };

  state = {
    error: null,
  };

  /**
   * @public
   */
  errors: any[] = [];

  static getDerivedStateFromError(error: any) {
    return { error };
  }

  componentDidCatch(error: any) {
    this.errors.push(error);
  }

  render() {
    if (this.state.error) {
      return null;
    }
    return this.props.children;
  }
}

/**
 * Allows counting how many times the owner of `RenderCounter` rendered or
 * a component within the RenderCounter tree "commits" an update.
 * @example <RenderCounter ref={getRenderCountRef}>...</RenderCounter>
 *          getRenderCountRef.current() === 2
 */
export const RenderCounter = React.forwardRef<() => number, { children: React.ReactNode }>(
  function RenderCounter({ children }, ref) {

Review comment (suggestion):
Let's avoid `any` wherever we can

```suggestion
  errors: unknown[] = [];
```

Code after:
import * as React from 'react';
import PropTypes from 'prop-types';

/**
 * A basic error boundary that can be used to assert thrown errors in render.
 * @example <ErrorBoundary ref={errorRef}><MyComponent /></ErrorBoundary>;
 *          expect(errorRef.current.errors).to.have.length(0);
 */
export class ErrorBoundary extends React.Component<{ children: React.ReactNode }> {
  static propTypes = {
    children: PropTypes.node.isRequired,
  };

  state = {
    error: null,
  };

  /**
   * @public
   */
  errors: unknown[] = [];

  static getDerivedStateFromError(error: unknown) {
    return { error };
  }

  componentDidCatch(error: unknown) {
    this.errors.push(error);
  }

  render() {
    if (this.state.error) {
      return null;
    }
    return this.props.children;
  }
}

/**
 * Allows counting how many times the owner of `RenderCounter` rendered or
 * a component within the RenderCounter tree "commits" an update.
 * @example <RenderCounter ref={getRenderCountRef}>...</RenderCounter>
 *          getRenderCountRef.current() === 2
 */
export const RenderCounter = React.forwardRef<() => number, { children: React.ReactNode }>(
  function RenderCounter({ children }, ref) {
Repository: mui/material-ui
File: test/utils/userEvent.ts (TypeScript)

Code before:

export function touch(target: Element): void {
  fireEvent.touchStart(target);
  fireEvent.touchEnd(target);
}

export const mousePress: (...args: Parameters<FireFunction>) => void = (target, options) => {
  if (React.version.startsWith('18')) {
    fireEvent.mouseDown(target, options);
    fireEvent.mouseUp(target, options);
    fireEvent.click(target, options);
  } else {
    mouseDown(target, options);
    mouseUp(target, options);
    click(target, options);
    act(() => {});
  }
};

export function keyPress(target: Element, options: { key: string }): void {
  if (React.version.startsWith('18')) {
    fireEvent.keyDown(target, options);
    fireEvent.keyUp(target, options);
  } else {
    keyDown(target, options as any as Event);
    keyUp(target, options as any as Event);
    act(() => {});
  }
}


Review comment (suggestion):
```suggestion
    keyDown(target, options as unknown as Event);
    keyUp(target, options as unknown as Event);
```

Code after:

export function touch(target: Element): void {
  fireEvent.touchStart(target);
  fireEvent.touchEnd(target);
}

export const mousePress: (...args: Parameters<FireFunction>) => void = (target, options) => {
  if (React.version.startsWith('18')) {
    fireEvent.mouseDown(target, options);
    fireEvent.mouseUp(target, options);
    fireEvent.click(target, options);
  } else {
    mouseDown(target, options);
    mouseUp(target, options);
    click(target, options);
    act(() => {});
  }
};

export function keyPress(target: Element, options: { key: string }): void {
  if (React.version.startsWith('18')) {
    fireEvent.keyDown(target, options);
    fireEvent.keyUp(target, options);
  } else {
    keyDown(target, options as unknown as Event);
    keyUp(target, options as unknown as Event);
    act(() => {});
  }
}
Repository: mui/material-ui
File: docs/data/base/pages.ts (TypeScript)

Code before:
      },
      {
        pathname: '/base/components/utils',
        subheader: 'utils',
        children: [
          { pathname: '/base/react-click-away-listener', title: 'Click-Away Listener' },
          { pathname: '/base/react-focus-trap', title: 'Focus Trap' },
          { pathname: '/base/react-form-control', title: 'Form Control' },
          { pathname: '/base/react-modal', title: 'Modal' },
          { pathname: '/base/react-no-ssr', title: 'No-SSR' },
          { pathname: '/base/react-popper', title: 'Popper' },
          { pathname: '/base/react-portal', title: 'Portal' },
          { pathname: '/base/react-textarea-autosize', title: 'Textarea Autosize' },
        ],
      },
    ],
  },
  {
    title: 'Component API',
    pathname: '/base/api',
    icon: 'CodeIcon',
    children: pagesApi,
  },
  {
    pathname: '/base/guides',
    title: 'How to guides',
    icon: 'VisibilityIcon',
    children: [
      {
        pathname: '/base/guides/working-with-tailwind-css',
        title: 'Working with Tailwind CSS',
      },
    ],
  },
];

export default pages;


Review comment (suggestion):
```suggestion
    title: 'How-to guides',
```

(https://www.merriam-webster.com/dictionary/how-to, https://www.wikihow.com/%22How-to%22-or-How-to)

Code after:
      },
      {
        pathname: '/base/components/utils',
        subheader: 'utils',
        children: [
          { pathname: '/base/react-click-away-listener', title: 'Click-Away Listener' },
          { pathname: '/base/react-focus-trap', title: 'Focus Trap' },
          { pathname: '/base/react-form-control', title: 'Form Control' },
          { pathname: '/base/react-modal', title: 'Modal' },
          { pathname: '/base/react-no-ssr', title: 'No-SSR' },
          { pathname: '/base/react-popper', title: 'Popper' },
          { pathname: '/base/react-portal', title: 'Portal' },
          { pathname: '/base/react-textarea-autosize', title: 'Textarea Autosize' },
        ],
      },
    ],
  },
  {
    title: 'Component API',
    pathname: '/base/api',
    icon: 'CodeIcon',
    children: pagesApi,
  },
  {
    pathname: '/base/guides',
    title: 'How-to guides',
    icon: 'VisibilityIcon',
    children: [
      {
        pathname: '/base/guides/working-with-tailwind-css',
        title: 'Working with Tailwind CSS',
      },
    ],
  },
];

export default pages;
Repository: mui/material-ui
File: scripts/releaseChangelog.mjs (JavaScript)

Code before:
   * @type {AsyncIterableIterator<Octokit.Response<Octokit.ReposCompareCommitsResponse>>}
   */
  const timeline = octokit.paginate.iterator(
    octokit.repos.compareCommits.endpoint.merge({
      owner: 'mui',
      repo,
      base: lastRelease,
      head: release,
    }),
  );

  /**
   * @type {Octokit.ReposCompareCommitsResponseCommitsItem[]}
   */
  const commitsItems = [];
  for await (const response of timeline) {
    const { data: compareCommits } = response;
    commitsItems.push(...compareCommits.commits.filter(filterCommit));
  }

  const getAuthor = (commit) => {
    if (!commit.author) {
      console.warn(
        `The author of the commit: ${commit.commit.tree.url} cannot be retrieved. Please add the github username manually.`,
      );
      return null;
    }

    return commit.author?.login;
  };

  const authors = Array.from(
    new Set(
      commitsItems.map((commitsItem) => {
        return getAuthor(commitsItem);
      }),
    ),
  ).filter((author) => author != null);

  const contributorHandles = authors
    .sort((a, b) => a.localeCompare(b))
    .map((author) => `@${author}`)
    .join(', ');

  // We don't know when a particular commit was made from the API.
  // Only that the commits are ordered by date ASC
  const commitsItemsByDateDesc = commitsItems.slice().reverse();
  // Sort by tags ASC, date desc
  // Will only consider exact matches of tags so `[Slider]` will not be grouped with `[Slider][Modal]`
  commitsItems.sort((a, b) => {
    const aTags = parseTags(a.commit.message);

Review comment (security):
```suggestion
      return 'TODO add the author username';
```

Code after:
  /**
   * @type {AsyncIterableIterator<Octokit.Response<Octokit.ReposCompareCommitsResponse>>}
   */
  const timeline = octokit.paginate.iterator(
    octokit.repos.compareCommits.endpoint.merge({
      owner: 'mui',
      repo,
      base: lastRelease,
      head: release,
    }),
  );

  /**
   * @type {Octokit.ReposCompareCommitsResponseCommitsItem[]}
   */
  const commitsItems = [];
  for await (const response of timeline) {
    const { data: compareCommits } = response;
    commitsItems.push(...compareCommits.commits.filter(filterCommit));
  }

  let warnedOnce = false;

  const getAuthor = (commit) => {
    if (!commit.author) {
      if (!warnedOnce) {
        console.warn(
          `The author of the commit: ${commit.commit.tree.url} cannot be retrieved. Please add the github username manually.`,
        );
      }
      warnedOnce = true;
      return chalk.red("TODO INSERT AUTHOR'S USERNAME");
    }

    return commit.author?.login;
  };

  const authors = Array.from(
    new Set(
      commitsItems.map((commitsItem) => {
        return getAuthor(commitsItem);
      }),
    ),
  );
  const contributorHandles = authors
    .sort((a, b) => a.localeCompare(b))
    .map((author) => `@${author}`)
    .join(', ');

  // We don't know when a particular commit was made from the API.
  // Only that the commits are ordered by date ASC
Repository: mui/material-ui
File: docs/src/modules/utils/useRouterExtra.ts (TypeScript)

Code before:
import { useRouter } from 'next/router';

export default function useRouterExtra() {
  const router = useRouter();

  const asPathWithoutLang = router.asPath.replace(/^\/+[a-zA-Z]{2}\//, '/');
  let product = asPathWithoutLang.replace(/^\/+([^/]+)\/.*/, '$1');
  if (asPathWithoutLang.startsWith('/x')) {
    product = asPathWithoutLang.replace('/x/react-', '').replace(/\/.*/, '');
  }
  return {
    ...router,
    asPathWithoutLang,
    product: product as 'base' | 'material-ui' | 'joy-ui' | 'system' | 'data-grid' | 'date-pickers',
    isMuiX: asPathWithoutLang.startsWith('/x'),
  };
}


Review comment (bug):
Found this edge case when the URL has double `//`, e.g. clicking on the link in GoogleAnalytics does not toggle the active page.

In the image below, the `components` section should collapse.

<img width="572" alt="image" src="https://user-images.githubusercontent.com/18292247/217870697-4211492d-ef30-4dd6-80c8-78f0acbc7f86.png">


Code after:
import { useRouter } from 'next/router';

export default function useRouterExtra() {
  const router = useRouter();

  const asPathWithoutLang = router.asPath.replace(/^\/+[a-zA-Z]{2}\//, '/');
  let product = asPathWithoutLang.replace(/^\/+([^/]+)\/.*/, '$1');
  if (asPathWithoutLang.startsWith('/x/')) {
    product = asPathWithoutLang.replace('/x/react-', '').replace(/\/.*/, '');
  }
  return {
    ...router,
    asPathWithoutLang,
    product: product as 'base' | 'material-ui' | 'joy-ui' | 'system' | 'data-grid' | 'date-pickers',
    isMuiX: asPathWithoutLang.startsWith('/x'),
  };
}
Repository: mui/material-ui
File: docs/src/modules/utils/useRouterExtra.ts (TypeScript)

Code before:
import { useRouter } from 'next/router';

export default function useRouterExtra() {
  const router = useRouter();

  const asPathWithoutLang = router.asPath.replace(/^\/+[a-zA-Z]{2}\//, '/');
  let product = asPathWithoutLang.replace(/^\/+([^/]+)\/.*/, '$1');
  if (asPathWithoutLang.startsWith('/x')) {
    product = asPathWithoutLang.replace('/x/react-', '').replace(/\/.*/, '');
  }
  return {
    ...router,
    asPathWithoutLang,
    product: product as 'base' | 'material-ui' | 'joy-ui' | 'system' | 'data-grid' | 'date-pickers',
    isMuiX: asPathWithoutLang.startsWith('/x'),
  };
}


Review comment (suggestion):
In case we have URLs that also start with /x?

```suggestion
  if (asPathWithoutLang.startsWith('/x/')) {
```

Code after:
import { useRouter } from 'next/router';

export default function useRouterExtra() {
  const router = useRouter();

  const asPathWithoutLang = router.asPath.replace(/^\/+[a-zA-Z]{2}\//, '/');
  let product = asPathWithoutLang.replace(/^\/+([^/]+)\/.*/, '$1');
  if (asPathWithoutLang.startsWith('/x/')) {
    product = asPathWithoutLang.replace('/x/react-', '').replace(/\/.*/, '');
  }
  return {
    ...router,
    asPathWithoutLang,
    product: product as 'base' | 'material-ui' | 'joy-ui' | 'system' | 'data-grid' | 'date-pickers',
    isMuiX: asPathWithoutLang.startsWith('/x'),
  };
}
Repository: mui/material-ui
File: packages/mui-material/src/Dialog/DialogContext.ts (TypeScript)

Code before:
import { createContext } from 'react';

interface DialogContextValue {
  titleId?: string;
}

const DialogContext = createContext<DialogContextValue>({});

if (process.env.NODE_ENV !== 'production') {
  DialogContext.displayName = 'DialogContext';
}

export default DialogContext;


Review comment (bug):
This case is interesting. I think that the initial problem was that:

```jsx
import ReactDOMServer from 'react-dom/server';
```

is wrong because React plans to no longer export default https://github.com/facebook/react/pull/18102.

This change goes after something else but why not.

Code after:
import * as React from 'react';

interface DialogContextValue {
  titleId?: string;
}

const DialogContext = React.createContext<DialogContextValue>({});

if (process.env.NODE_ENV !== 'production') {
  DialogContext.displayName = 'DialogContext';
}

export default DialogContext;