ronantakizawa/codereview-bench huggingface dataset:
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 {
  __CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE,
  act,
  Children,
  Component,
  Fragment,
  Profiler,
  PureComponent,
  StrictMode,
  Suspense,
  cloneElement,
  createContext,
  createElement,
  createRef,
  use,
  forwardRef,
  isValidElement,
  lazy,
  memo,
  cache,
  startTransition,
  unstable_Activity,
  unstable_TracingMarker,
  unstable_DebugTracingMode,
  unstable_LegacyHidden,
  unstable_Scope,
  unstable_SuspenseList,
  unstable_getCacheForType,
  unstable_useCacheRefresh,
  unstable_useMemoCache,
  useId,
  useCallback,
  useContext,
  useDebugValue,
  useDeferredValue,
  useEffect,
  experimental_useEffectEvent,
  useImperativeHandle,
  useLayoutEffect,
  useInsertionEffect,
  useMemo,
  useOptimistic,
  useReducer,
  useRef,
  useState,
  useSyncExternalStore,
  useTransition,
  useActionState,
  version,
} from './src/ReactClient';
export {jsx, jsxs, jsxDEV} from './src/jsx/ReactJSX';


Review comment (suggestion):
I think @acdlite and @gnoff may have suggested the idea of moving the impl as well into it's own package and dynamically injecting it into the dispatcher iirc. This feels okay to me as a first step though

Code after:
  useDebugValue,
  useDeferredValue,
  useEffect,
  experimental_useEffectEvent,
  useImperativeHandle,
  useLayoutEffect,
  useInsertionEffect,
  useMemo,
  useOptimistic,
  useReducer,
  useRef,
  useState,
  useSyncExternalStore,
  useTransition,
  useActionState,
  version,
} from './src/ReactClient';

export {jsx, jsxs, jsxDEV} from './src/jsx/ReactJSX';

// export for backwards compatibility during upgrade
export {useMemoCache as unstable_useMemoCache} from './src/ReactHooks';

// export to match the name of the OSS function typically exported from
// react/compiler-runtime
export {useMemoCache as c} from './src/ReactHooks';
Repository: facebook/react
File: packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js (JavaScript)

Code before:
        prevProp,
        ((attributeConfig: any): AttributeConfiguration),
      );
    }
  }
  return updatePayload;
}

function fastAddProperties(
  updatePayload: null | Object,
  nextProps: Object,
  validAttributes: AttributeConfiguration,
): null | Object {
  let attributeConfig;
  let nextProp;

  for (const propKey in nextProps) {
    nextProp = nextProps[propKey];

    if (nextProp === undefined) {
      continue;
    }

    attributeConfig = validAttributes[propKey];

    if (!attributeConfig) {
      continue;
    }

    let newValue;

    if (typeof nextProp === 'function') {
      newValue = (true: any);
    } else if (typeof attributeConfig !== 'object') {
      newValue = nextProp;
    } else if (typeof attributeConfig.process === 'function') {
      newValue = attributeConfig.process(nextProp);
    } else if (typeof attributeConfig.diff === 'function') {
      newValue = nextProp;
    }

    if (newValue !== undefined) {
      if (!updatePayload) {
        updatePayload = ({}: {[string]: $FlowFixMe});
      }
      updatePayload[propKey] = newValue;
      continue;
    }

    if (isArray(nextProp)) {
      for (let i = 0; i < nextProp.length; i++) {

Review comment (suggestion):
I personally prefer the explicitness of `attributeConfig == null` (which checks for both undefined and null), but this works, too.

Code after:
        prevProp,
        ((attributeConfig: any): AttributeConfiguration),
      );
    }
  }
  return updatePayload;
}

function fastAddProperties(
  payload: null | Object,
  props: Object,
  validAttributes: AttributeConfiguration,
): null | Object {
  let attributeConfig;
  let prop;

  for (const propKey in props) {
    prop = props[propKey];

    if (prop === undefined) {
      continue;
    }

    attributeConfig = ((validAttributes[propKey]: any): AttributeConfiguration);

    if (attributeConfig == null) {
      continue;
    }

    let newValue;

    if (typeof prop === 'function') {
      // A function prop. It represents an event handler. Pass it to native as 'true'.
      newValue = true;
    } else if (typeof attributeConfig !== 'object') {
      // An atomic prop. Doesn't need to be flattened.
      newValue = prop;
    } else if (typeof attributeConfig.process === 'function') {
      // An atomic prop with custom processing.
      newValue = attributeConfig.process(prop);
    } else if (typeof attributeConfig.diff === 'function') {
      // An atomic prop with custom diffing. We don't do diffing here.
      newValue = prop;
    }

    if (newValue !== undefined) {
      if (!payload) {
        payload = ({}: {[string]: $FlowFixMe});
      }
      payload[propKey] = newValue;
      continue;
Repository: facebook/react
File: packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js (JavaScript)

Code before:
      );
    }
  }
  return updatePayload;
}

function fastAddProperties(
  updatePayload: null | Object,
  nextProps: Object,
  validAttributes: AttributeConfiguration,
): null | Object {
  let attributeConfig;
  let nextProp;

  for (const propKey in nextProps) {
    nextProp = nextProps[propKey];

    if (nextProp === undefined) {
      continue;
    }

    attributeConfig = validAttributes[propKey];

    if (!attributeConfig) {
      continue;
    }

    let newValue;

    if (typeof nextProp === 'function') {
      newValue = (true: any);
    } else if (typeof attributeConfig !== 'object') {
      newValue = nextProp;
    } else if (typeof attributeConfig.process === 'function') {
      newValue = attributeConfig.process(nextProp);
    } else if (typeof attributeConfig.diff === 'function') {
      newValue = nextProp;
    }

    if (newValue !== undefined) {
      if (!updatePayload) {
        updatePayload = ({}: {[string]: $FlowFixMe});
      }
      updatePayload[propKey] = newValue;
      continue;
    }

    if (isArray(nextProp)) {
      for (let i = 0; i < nextProp.length; i++) {
        updatePayload = fastAddProperties(
          updatePayload,

Review comment (suggestion):
What I was just mentioning about: we could change the default here and stop ignoring any properties that do not have `validAttributes` entry.

Code after:
      );
    }
  }
  return updatePayload;
}

function fastAddProperties(
  payload: null | Object,
  props: Object,
  validAttributes: AttributeConfiguration,
): null | Object {
  let attributeConfig;
  let prop;

  for (const propKey in props) {
    prop = props[propKey];

    if (prop === undefined) {
      continue;
    }

    attributeConfig = ((validAttributes[propKey]: any): AttributeConfiguration);

    if (attributeConfig == null) {
      continue;
    }

    let newValue;

    if (typeof prop === 'function') {
      // A function prop. It represents an event handler. Pass it to native as 'true'.
      newValue = true;
    } else if (typeof attributeConfig !== 'object') {
      // An atomic prop. Doesn't need to be flattened.
      newValue = prop;
    } else if (typeof attributeConfig.process === 'function') {
      // An atomic prop with custom processing.
      newValue = attributeConfig.process(prop);
    } else if (typeof attributeConfig.diff === 'function') {
      // An atomic prop with custom diffing. We don't do diffing here.
      newValue = prop;
    }

    if (newValue !== undefined) {
      if (!payload) {
        payload = ({}: {[string]: $FlowFixMe});
      }
      payload[propKey] = newValue;
      continue;
    }
Repository: facebook/react
File: packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js (JavaScript)

Code before:

    if (nextProp === undefined) {
      continue;
    }

    attributeConfig = validAttributes[propKey];

    if (!attributeConfig) {
      continue;
    }

    let newValue;

    if (typeof nextProp === 'function') {
      newValue = (true: any);
    } else if (typeof attributeConfig !== 'object') {
      newValue = nextProp;
    } else if (typeof attributeConfig.process === 'function') {
      newValue = attributeConfig.process(nextProp);
    } else if (typeof attributeConfig.diff === 'function') {
      newValue = nextProp;
    }

    if (newValue !== undefined) {
      if (!updatePayload) {
        updatePayload = ({}: {[string]: $FlowFixMe});
      }
      updatePayload[propKey] = newValue;
      continue;
    }

    if (isArray(nextProp)) {
      for (let i = 0; i < nextProp.length; i++) {
        updatePayload = fastAddProperties(
          updatePayload,
          nextProp[i],
          ((attributeConfig: any): AttributeConfiguration),
        );
      }
      continue;
    }

    updatePayload = fastAddProperties(
      updatePayload,
      nextProp,
      ((attributeConfig: any): AttributeConfiguration),
    );
  }

  return updatePayload;
}

Review comment (suggestion):
Could this work?

```
    let newValue = typeof nextProp === 'function' ? true : nextProp;
    if (typeof attributeConfig === 'object' && typeof attributeConfig.process === 'function') {
      newValue = attributeConfig.process(nextProp);
    }
```

Code after:

    if (prop === undefined) {
      continue;
    }

    attributeConfig = ((validAttributes[propKey]: any): AttributeConfiguration);

    if (attributeConfig == null) {
      continue;
    }

    let newValue;

    if (typeof prop === 'function') {
      // A function prop. It represents an event handler. Pass it to native as 'true'.
      newValue = true;
    } else if (typeof attributeConfig !== 'object') {
      // An atomic prop. Doesn't need to be flattened.
      newValue = prop;
    } else if (typeof attributeConfig.process === 'function') {
      // An atomic prop with custom processing.
      newValue = attributeConfig.process(prop);
    } else if (typeof attributeConfig.diff === 'function') {
      // An atomic prop with custom diffing. We don't do diffing here.
      newValue = prop;
    }

    if (newValue !== undefined) {
      if (!payload) {
        payload = ({}: {[string]: $FlowFixMe});
      }
      payload[propKey] = newValue;
      continue;
    }

    // Not-atomic prop that needs to be flattened. Likely it's the 'style' prop.

    // It can be an array.
    if (isArray(prop)) {
      for (let i = 0; i < prop.length; i++) {
        payload = fastAddProperties(payload, prop[i], attributeConfig);
      }
      continue;
    }

    // Or it can be an object.
    payload = fastAddProperties(payload, prop, attributeConfig);
  }

  return payload;
}
Repository: facebook/react
File: packages/react-reconciler/src/ReactFiberAsyncDispatcher.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 {AsyncDispatcher, Fiber} from './ReactInternalTypes';
import type {Cache} from './ReactFiberCacheComponent';

import {enableCache} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {CacheContext} from './ReactFiberCacheComponent';

import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {current as currentOwner} from 'react-reconciler/src/ReactCurrentFiber';

function getCacheForType<T>(resourceType: () => T): T {
  if (!enableCache) {
    throw new Error('Not implemented.');
  }
  const cache: Cache = readContext(CacheContext);
  let cacheForType: T | void = (cache.data.get(resourceType): any);
  if (cacheForType === undefined) {
    cacheForType = resourceType();
    cache.data.set(resourceType, cacheForType);
  }
  return cacheForType;
}

export const DefaultAsyncDispatcher: AsyncDispatcher = ({
  getCacheForType,
}: any);

if (__DEV__ || !disableStringRefs) {
  DefaultAsyncDispatcher.getOwner = (): null | Fiber => {
    return currentOwner;
  };
}


Review comment (suggestion):
nit:
```suggestion
import {current as currentOwner} from './ReactCurrentFiber';
```


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 {AsyncDispatcher, Fiber} from './ReactInternalTypes';
import type {Cache} from './ReactFiberCacheComponent';

import {enableCache} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {CacheContext} from './ReactFiberCacheComponent';

import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {current as currentOwner} from './ReactCurrentFiber';

function getCacheForType<T>(resourceType: () => T): T {
  if (!enableCache) {
    throw new Error('Not implemented.');
  }
  const cache: Cache = readContext(CacheContext);
  let cacheForType: T | void = (cache.data.get(resourceType): any);
  if (cacheForType === undefined) {
    cacheForType = resourceType();
    cache.data.set(resourceType, cacheForType);
  }
  return cacheForType;
}

export const DefaultAsyncDispatcher: AsyncDispatcher = ({
  getCacheForType,
}: any);

if (__DEV__ || !disableStringRefs) {
  DefaultAsyncDispatcher.getOwner = (): null | Fiber => {
    return currentOwner;
  };
}
Repository: facebook/react
File: packages/react-reconciler/src/ReactFiberAsyncDispatcher.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 {AsyncDispatcher, Fiber} from './ReactInternalTypes';
import type {Cache} from './ReactFiberCacheComponent';

import {enableCache} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {CacheContext} from './ReactFiberCacheComponent';

import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {current as currentOwner} from 'react-reconciler/src/ReactCurrentFiber';

function getCacheForType<T>(resourceType: () => T): T {
  if (!enableCache) {
    throw new Error('Not implemented.');
  }
  const cache: Cache = readContext(CacheContext);
  let cacheForType: T | void = (cache.data.get(resourceType): any);
  if (cacheForType === undefined) {
    cacheForType = resourceType();
    cache.data.set(resourceType, cacheForType);
  }
  return cacheForType;
}

export const DefaultAsyncDispatcher: AsyncDispatcher = ({
  getCacheForType,
}: any);

if (__DEV__ || !disableStringRefs) {
  DefaultAsyncDispatcher.getOwner = (): null | Fiber => {
    return currentOwner;
  };
}


Review comment (suggestion):
nit:
```suggestion
import {current as currentOwner} from './ReactCurrentFiber';
```

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 {AsyncDispatcher, Fiber} from './ReactInternalTypes';
import type {Cache} from './ReactFiberCacheComponent';

import {enableCache} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {CacheContext} from './ReactFiberCacheComponent';

import {disableStringRefs} from 'shared/ReactFeatureFlags';

import {current as currentOwner} from './ReactCurrentFiber';

function getCacheForType<T>(resourceType: () => T): T {
  if (!enableCache) {
    throw new Error('Not implemented.');
  }
  const cache: Cache = readContext(CacheContext);
  let cacheForType: T | void = (cache.data.get(resourceType): any);
  if (cacheForType === undefined) {
    cacheForType = resourceType();
    cache.data.set(resourceType, cacheForType);
  }
  return cacheForType;
}

export const DefaultAsyncDispatcher: AsyncDispatcher = ({
  getCacheForType,
}: any);

if (__DEV__ || !disableStringRefs) {
  DefaultAsyncDispatcher.getOwner = (): null | Fiber => {
    return currentOwner;
  };
}
Repository: facebook/react
File: packages/react-devtools-shared/src/symbolicateSource.js (JavaScript)

Code before:
            columnNumber, // 1-based
          });

          if (possiblyURL === null) {
            return null;
          }
          try {
            // sourceMapURL=https://...
            void new URL(possiblyURL); // test if it is a valid URL
            const normalizedURL = normalizeUrl(possiblyURL);

            return {sourceURL: normalizedURL, line, column};
          } catch (e) {
            // This is not valid URL
            if (
              // sourceMapURL=/file
              possiblyURL.startsWith('/') ||
              // sourceMapURL=C:\\...
              possiblyURL.slice(1).startsWith(':\\\\')
            ) {
              // This is an absolute path
              return {sourceURL: possiblyURL, line, column};
            }

            // This is a relative path
            // sourceMapURL=x.js.map
            const absoluteSourcePath = new URL(
              possiblyURL,
              sourceMapURL,
            ).toString();
            return {sourceURL: absoluteSourcePath, line, column};
          }
        } catch (e) {
          return null;
        }
      }

      return null;
    }
  }

  return null;
}


Review comment (suggestion):
```suggestion
            // possiblyURL = x.js.map, sourceMapURL = https://react.dev
```

Code after:
            columnNumber, // 1-based
          });

          if (possiblyURL === null) {
            return null;
          }
          try {
            // sourceMapURL = https://react.dev/script.js.map
            void new URL(possiblyURL); // test if it is a valid URL
            const normalizedURL = normalizeUrl(possiblyURL);

            return {sourceURL: normalizedURL, line, column};
          } catch (e) {
            // This is not valid URL
            if (
              // sourceMapURL = /file
              possiblyURL.startsWith('/') ||
              // sourceMapURL = C:\\...
              possiblyURL.slice(1).startsWith(':\\\\')
            ) {
              // This is an absolute path
              return {sourceURL: possiblyURL, line, column};
            }

            // This is a relative path
            // possiblyURL = x.js.map, sourceMapURL = https://react.dev/script.js.map
            const absoluteSourcePath = new URL(
              possiblyURL,
              sourceMapURL,
            ).toString();
            return {sourceURL: absoluteSourcePath, line, column};
          }
        } catch (e) {
          return null;
        }
      }

      return null;
    }
  }

  return null;
}
Repository: facebook/react
File: compiler/packages/babel-plugin-react-compiler/src/Babel/RunReactCompilerBabelPlugin.ts (TypeScript)

Code before:

export const DEFAULT_PLUGINS = ["babel-plugin-fbt", "babel-plugin-fbt-runtime"];
export function runBabelPluginReactCompiler(
  text: string,
  file: string,
  language: "flow" | "typescript",
  options: Partial<PluginOptions> | null,
  includeAst: boolean = false,
): BabelCore.BabelFileResult {
  const ast = BabelParser.parse(text, {
    sourceFilename: file,
    plugins: [language, "jsx"],
    sourceType: "module",
  });
  const result = transformFromAstSync(ast, text, {
    ast: includeAst,
    filename: file,
    highlightCode: false,
    retainLines: true,
    plugins: [
      [BabelPluginReactCompiler, options],
      "babel-plugin-fbt",
      "babel-plugin-fbt-runtime",
    ],
    sourceType: "module",
    configFile: false,
  });
  invariant(
    result?.code != null,
    `Expected BabelPluginReactForget to codegen successfully, got: ${result}`,
  );
  return result;
}


Review comment (suggestion):
You may want to also add `babelrc: false` (to disable `.babelrc` in addition to `babel.config`)

See https://github.com/facebook/react/pull/29229

```suggestion
    configFile: false,
    babelrc: false,
```

Code after:

export const DEFAULT_PLUGINS = ["babel-plugin-fbt", "babel-plugin-fbt-runtime"];
export function runBabelPluginReactCompiler(
  text: string,
  file: string,
  language: "flow" | "typescript",
  options: Partial<PluginOptions> | null,
  includeAst: boolean = false
): BabelCore.BabelFileResult {
  const ast = BabelParser.parse(text, {
    sourceFilename: file,
    plugins: [language, "jsx"],
    sourceType: "module",
  });
  const result = transformFromAstSync(ast, text, {
    ast: includeAst,
    filename: file,
    highlightCode: false,
    retainLines: true,
    plugins: [
      [BabelPluginReactCompiler, options],
      "babel-plugin-fbt",
      "babel-plugin-fbt-runtime",
    ],
    sourceType: "module",
    configFile: false,
    babelrc: false,
  });
  invariant(
    result?.code != null,
    `Expected BabelPluginReactForget to codegen successfully, got: ${result}`
  );
  return result;
}
Repository: facebook/react
File: packages/react-dom/src/shared/ensureCorrectIsomorphicReactVersion.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 reactDOMPackageVersion from 'shared/ReactVersion';
import * as IsomorphicReactPackage from 'react';

export function ensureCorrectIsomorphicReactVersion() {
  const isomorphicReactPackageVersion = IsomorphicReactPackage.version;
  if (isomorphicReactPackageVersion !== reactDOMPackageVersion) {
    // TODO: Do we have a canoncical documentation page for this?
    throw new Error(
      'Incompatible React versions: The "react" and "react-dom" packages must ' +
        'have the exact same version. Instead got:\n' +
        `  - react:      ${isomorphicReactPackageVersion}\n` +
        `  - react-dom:  ${reactDOMPackageVersion}`,
    );
  }
}


Review comment (suggestion):
In a perfect world we'd also detect environment mismatches e.g. importing `react-dom` **with** the `react-server` condition while `react` is being imported **without** the `react-server` condition. Maybe we encode the environment in the version string e.g. `19.0.0-abc-123+react-server`? Though that wouldn't tell you explcitly that the environment mismatched not the version.

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 reactDOMPackageVersion from 'shared/ReactVersion';
import * as IsomorphicReactPackage from 'react';

export function ensureCorrectIsomorphicReactVersion() {
  const isomorphicReactPackageVersion = IsomorphicReactPackage.version;
  if (isomorphicReactPackageVersion !== reactDOMPackageVersion) {
    throw new Error(
      'Incompatible React versions: The "react" and "react-dom" packages must ' +
        'have the exact same version. Instead got:\n' +
        `  - react:      ${isomorphicReactPackageVersion}\n` +
        `  - react-dom:  ${reactDOMPackageVersion}\n` +
        'Learn more: https://react.dev/warnings/version-mismatch',
    );
  }
}