ronantakizawa/codereview-bench huggingface dataset:
Repository: expo/expo
File: packages/expo/src/winter/fetch/fetch.types.ts (TypeScript)

Code before:
/**
 * A fetch RequestInit compatible structure.
 */
export interface FetchRequestInit {
  body?: BodyInit | null;
  credentials?: RequestCredentials; // same-origin is not supported
  headers?: HeadersInit;
  method?: string;
  signal?: AbortSignal | null;
  redirect?: RequestRedirect;

  // Not supported fields
  integrity?: string;
  keepalive?: boolean;
  mode?: RequestMode;
  referrer?: string;
  window?: any;
}

/**
 * A fetch Request compatible structure.
 */
export interface FetchRequestLike {
  readonly url: string;
  readonly body: BodyInit | null;
  readonly method: string;

  // Not always supported, marked as optional
  readonly credentials?: RequestCredentials;
  readonly signal?: AbortSignal;
  readonly redirect?: RequestRedirect;
}


Review comment (suggestion):
[🏗️ **design**] This interface is missing `headers` — but the fetch implementation accesses `input.headers` when the input is detected as Request-like. Add:
```ts
readonly headers?: HeadersInit;
```
to keep the type and the runtime behavior aligned.

Code after:
/**
 * A fetch RequestInit compatible structure.
 */
export interface FetchRequestInit {
  body?: BodyInit | null;
  credentials?: RequestCredentials; // same-origin is not supported
  headers?: HeadersInit;
  method?: string;
  signal?: AbortSignal | null;
  redirect?: RequestRedirect;

  // Not supported fields
  integrity?: string;
  keepalive?: boolean;
  mode?: RequestMode;
  referrer?: string;
  window?: any;
}

/**
 * A fetch Request compatible structure.
 */
export interface FetchRequestLike {
  readonly url: string;
  readonly body: BodyInit | null;
  readonly method: string;
  readonly headers: Headers;

  // Not always supported, marked as optional
  readonly credentials?: RequestCredentials;
  readonly signal?: AbortSignal;
  readonly redirect?: RequestRedirect;
}
Repository: expo/expo
File: packages/@expo/cli/src/utils/nodeEnv.ts (TypeScript)

Code before:
import * as env from '@expo/env';
import path from 'node:path';

import { events, shouldReduceLogs } from '../events';

type EnvOutput = Record<string, string | undefined>;

// TODO(@kitten): We assign this here to run server-side code bundled by metro
// It's not isolated into a worker thread yet
declare namespace globalThis {
  let __DEV__: boolean | undefined;
}

// prettier-ignore
export const event = events('env', (t) => [
  t.event<'mode', {
    nodeEnv: string;
    babelEnv: string;
    mode: 'development' | 'production';
  }>(),
  t.event<'load', {
    mode: string;
    files: string[];
    env: Record<string, string | undefined>;
  }>(),
]);

/**
 * Set the environment to production or development
 * lots of tools use this to determine if they should run in a dev mode.
 */
export function setNodeEnv(mode: 'development' | 'production') {
  process.env.NODE_ENV = process.env.NODE_ENV || mode;
  process.env.BABEL_ENV = process.env.BABEL_ENV || process.env.NODE_ENV;
  globalThis.__DEV__ = process.env.NODE_ENV !== 'production';

  event('mode', {
    nodeEnv: process.env.NODE_ENV,
    babelEnv: process.env.BABEL_ENV,
    mode,
  });
}

interface LoadEnvFilesOptions {
  force?: boolean;
  silent?: boolean;
  mode?: string;

Review comment (bug):
```
    mode: string | undefined;
```

`patch-project` unexpectedly triggers `nodeEnv.ts` type check with different globals causing mode type to fail.

https://github.com/expo/expo/actions/runs/22307655125/job/64530936684

Code after:
import * as env from '@expo/env';
import path from 'node:path';

import { events, shouldReduceLogs } from '../events';

type EnvOutput = Record<string, string | undefined>;

// TODO(@kitten): We assign this here to run server-side code bundled by metro
// It's not isolated into a worker thread yet
declare namespace globalThis {
  let __DEV__: boolean | undefined;
}

// prettier-ignore
export const event = events('env', (t) => [
  t.event<'mode', {
    nodeEnv: string;
    babelEnv: string;
    mode: 'development' | 'production';
  }>(),
  t.event<'load', {
    mode: string | undefined;
    files: string[];
    env: Record<string, string | undefined>;
  }>(),
]);

/**
 * Set the environment to production or development
 * lots of tools use this to determine if they should run in a dev mode.
 */
export function setNodeEnv(mode: 'development' | 'production') {
  process.env.NODE_ENV = process.env.NODE_ENV || mode;
  process.env.BABEL_ENV = process.env.BABEL_ENV || process.env.NODE_ENV;
  globalThis.__DEV__ = process.env.NODE_ENV !== 'production';

  event('mode', {
    nodeEnv: process.env.NODE_ENV,
    babelEnv: process.env.BABEL_ENV,
    mode,
  });
}

interface LoadEnvFilesOptions {
  force?: boolean;
  silent?: boolean;
  mode?: string;
Repository: expo/expo
File: packages/expo-image/android/src/main/java/expo/modules/image/okhttp/ExpoImageOkHttpClientGlideModule.kt (Kotlin)

Code before:
    return glideUrl.toString()
  }
}

private object SharedCookieJar : CookieJar {
  override fun saveFromResponse(url: HttpUrl, cookies: List<Cookie>) {
    val cookieManager = getCookieManager() ?: return
    val urlString = url.toString()
    for (cookie in cookies) {
      cookieManager.setCookie(urlString, cookie.toString())
    }
    cookieManager.flush()
  }

  override fun loadForRequest(url: HttpUrl): List<Cookie> {
    val cookieManager = getCookieManager() ?: return emptyList()
    val cookieString = cookieManager.getCookie(url.toString()) ?: return emptyList()
    return cookieString.split(";").mapNotNull { Cookie.parse(url, it.trim()) }
  }

  private fun getCookieManager(): CookieManager? = try {
    CookieManager.getInstance()
  } catch (e: Exception) {
    null
  }
}

@GlideModule
class ExpoImageOkHttpClientGlideModule : LibraryGlideModule() {
  override fun registerComponents(context: Context, glide: Glide, registry: Registry) {
    val client = OkHttpClient.Builder()
      .cookieJar(SharedCookieJar)
      .build()
    // We don't use the `GlideUrl` directly but we want to replace the default okhttp loader anyway
    // to make sure that the app will use only one client.
    registry.replace(GlideUrl::class.java, InputStream::class.java, OkHttpUrlLoader.Factory(client))
    registry.prepend(GlideUrlWrapper::class.java, InputStream::class.java, GlideUrlWrapperLoader.Factory(client))
  }
}


Review comment (suggestion):
```suggestion
  private fun getCookieManager(): CookieManager? = runCatching {
    CookieManager.getInstance() }.getOrNull()
  }
```

Code after:
    return glideUrl.toString()
  }
}

private object SharedCookieJar : CookieJar {
  override fun saveFromResponse(url: HttpUrl, cookies: List<Cookie>) {
    val cookieManager = getCookieManager() ?: return
    val urlString = url.toString()
    for (cookie in cookies) {
      cookieManager.setCookie(urlString, cookie.toString())
    }
    cookieManager.flush()
  }

  override fun loadForRequest(url: HttpUrl): List<Cookie> {
    val cookieManager = getCookieManager() ?: return emptyList()
    val cookieString = cookieManager.getCookie(url.toString()) ?: return emptyList()
    return cookieString.split(";").mapNotNull { Cookie.parse(url, it.trim()) }
  }

  private fun getCookieManager(): CookieManager? = runCatching {
    CookieManager.getInstance()
  }.getOrNull()
}

@GlideModule
class ExpoImageOkHttpClientGlideModule : LibraryGlideModule() {
  override fun registerComponents(context: Context, glide: Glide, registry: Registry) {
    val client = OkHttpClient.Builder()
      .cookieJar(SharedCookieJar)
      .build()
    // We don't use the `GlideUrl` directly but we want to replace the default okhttp loader anyway
    // to make sure that the app will use only one client.
    registry.replace(GlideUrl::class.java, InputStream::class.java, OkHttpUrlLoader.Factory(client))
    registry.prepend(GlideUrlWrapper::class.java, InputStream::class.java, GlideUrlWrapperLoader.Factory(client))
  }
}
Repository: expo/expo
File: docs/constants/structured-data.ts (TypeScript)

Code before:
  }

  return {
    '@context': 'https://schema.org',
    '@type': 'BreadcrumbList',
    itemListElement: items.map((item, index) => ({
      '@type': 'ListItem',
      position: index + 1,
      name: item.name,
      ...(item.url ? { item: item.url } : {}),
    })),
  };
}

export const websiteSchema = {
  '@context': 'https://schema.org',
  '@type': 'WebSite',
  name: 'Expo Documentation',
  url: 'https://docs.expo.dev',
  publisher: {
    '@type': 'Organization',
    name: 'Expo',
    url: 'https://expo.dev',
    logo: {
      '@type': 'ImageObject',
      url: 'https://raw.githubusercontent.com/expo/logos/main/png/logo-type-a.png',
    },
    sameAs: [
      'https://github.com/expo',
      'https://x.com/expo',
      'https://bsky.app/profile/expo.dev',
      'https://www.linkedin.com/company/expo-dev/',
      'https://www.youtube.com/@expodevelopers',
    ],
  },
};


Review comment (suggestion):
Is there anywhere from our CDN we could pull this logo? I think `githubusercontent` can get rate limited.

Code after:
  }

  return {
    '@context': 'https://schema.org',
    '@type': 'BreadcrumbList',
    itemListElement: items.map((item, index) => ({
      '@type': 'ListItem',
      position: index + 1,
      name: item.name,
      ...(item.url ? { item: item.url } : {}),
    })),
  };
}

export const websiteSchema = {
  '@context': 'https://schema.org',
  '@type': 'WebSite',
  name: 'Expo Documentation',
  url: 'https://docs.expo.dev',
  publisher: {
    '@type': 'Organization',
    name: 'Expo',
    url: 'https://expo.dev',
    logo: {
      '@type': 'ImageObject',
      url: 'https://docs.expo.dev/static/images/expo-logo.svg',
    },
    sameAs: [
      'https://github.com/expo',
      'https://x.com/expo',
      'https://bsky.app/profile/expo.dev',
      'https://www.linkedin.com/company/expo-dev/',
      'https://www.youtube.com/@expodevelopers',
    ],
  },
};
Repository: expo/expo
File: packages/expo-brownfield/cli/src/utils/ios.ts (TypeScript)

Code before:
  }
};

// TODO(pmleczek): Add support for prebuilt RN frameworks in future PR
export const generatePackageMetadataFile = async (config: IosConfig, packagePath: string) => {
  if (config.output === 'frameworks') {
    return;
  }

  const xcframeworks = [
    { name: config.scheme, targets: [config.scheme] },
    { name: 'hermesvm', targets: ['hermesvm'] },
  ];

  const contents = `// swift-tools-version:5.9
import PackageDescription

let package = Package(
    name: "${config.output.packageName}",
    platforms: [.iOS(.v15)],
    products: [${xcframeworks
      .map(
        (xcf) => `
        .library(
          name: "${xcf.name}",
          targets: ["${xcf.targets.join('", "')}"],
        ),
      `
      )
      .join('\n')}],
    targets: [${xcframeworks
      .map(
        (xcf) => `
        .binaryTarget(
          name: "${xcf.name}",
          path: "xcframeworks/${xcf.name}.xcframework",
        ),
      `
      )
      .join('\n')}]
);
`;

  await fs.promises.writeFile(path.join(packagePath, 'Package.swift'), contents);
};

export const makeArtifactsDirectory = (config: IosConfig) => {
  try {
    if (!fs.existsSync(config.artifacts)) {
      fs.mkdirSync(config.artifacts, { recursive: true });
    }

Review comment (suggestion):
We should probably read this from app.config instead of hardcoding it, but we can address it in a follow up PR

Code after:
    }

    CLIError.handle('ios-workspace-not-found');
  } catch (error) {
    const errorMessage = error instanceof Error ? error.message : '';
    CLIError.handle('ios-workspace-unknown-error', errorMessage);
  }
};

// TODO(pmleczek): Add support for prebuilt RN frameworks in future PR
export const generatePackageMetadataFile = async (config: IosConfig, packagePath: string) => {
  if (config.output === 'frameworks') {
    return;
  }

  const xcframeworks = [
    { name: config.scheme, targets: [config.scheme] },
    { name: 'hermesvm', targets: ['hermesvm'] },
  ];

  const contents = `// swift-tools-version:5.9
import PackageDescription

let package = Package(
    name: "${config.output.packageName}",
    platforms: [.iOS(.v15)],
    products: [${xcframeworks
      .map(
        (xcf) => `
        .library(
          name: "${xcf.name}",
          targets: ["${xcf.targets.join('", "')}"],
        ),
      `
      )
      .join('\n')}],
    targets: [${xcframeworks
      .map(
        (xcf) => `
        .binaryTarget(
          name: "${xcf.name}",
          path: "xcframeworks/${xcf.name}.xcframework",
        ),
      `
      )
      .join('\n')}]
);
`;

  await fs.promises.writeFile(path.join(packagePath, 'Package.swift'), contents);
};
Repository: expo/expo
File: packages/expo-notifications/ios/ExpoNotifications/Permissions/PermissionsModule.swift (Swift)

Code before:
      appContext?.permissions?.register([
        requester
      ])
    }

    AsyncFunction("getPermissionsAsync") { (promise: Promise) in
      appContext?
        .permissions?
        .getPermissionUsingRequesterClass(
          ExpoNotificationsPermissionsRequester.self,
          resolve: promise.resolver,
          reject: promise.legacyRejecter
        )
    }

    AsyncFunction("requestPermissionsAsync") { (requestedPermissions: NotificationPermissionRecord, promise: Promise) in
      let defaultAuthorizationOptions: UNAuthorizationOptions = [.alert, .badge, .sound]
      let options = requestedPermissions.numberOfOptionsRequested() > 0
        ? requestedPermissions.authorizationOptionValue()
        : defaultAuthorizationOptions
      requester.setAuthorizationOptions(options)

      // Call requestAuthorization directly to ensure new options are always
      // forwarded to the OS, even if notifications were previously granted.
      // iOS safely handles repeated calls to requestAuthorization(options:).
      // expo go notifications permissions are not scoped
      requester.requestAuthorizationOptions(options, resolver: promise.resolver, rejecter: promise.legacyRejecter)
    }
  }
}


Review comment (security):
```suggestion
      // Call `requestAuthorization` directly to ensure new options are always
      // forwarded to the OS, even if notifications were previously granted.
      // iOS safely handles repeated calls to `requestAuthorization(options:)`.
      // Expo Go notifications permissions are not scoped
```

Code after:
      appContext?.permissions?.register([
        requester
      ])
    }

    AsyncFunction("getPermissionsAsync") { (promise: Promise) in
      appContext?
        .permissions?
        .getPermissionUsingRequesterClass(
          ExpoNotificationsPermissionsRequester.self,
          resolve: promise.resolver,
          reject: promise.legacyRejecter
        )
    }

    AsyncFunction("requestPermissionsAsync") { (requestedPermissions: NotificationPermissionRecord, promise: Promise) in
      let defaultAuthorizationOptions: UNAuthorizationOptions = [.alert, .badge, .sound]
      let options = requestedPermissions.numberOfOptionsRequested() > 0
        ? requestedPermissions.authorizationOptionValue()
        : defaultAuthorizationOptions
      requester.setAuthorizationOptions(options)

      // Call `requestAuthorization` directly to ensure new options are always
      // forwarded to the OS, even if notifications were previously granted.
      // iOS safely handles repeated calls to `requestAuthorization(options:)`.
      // Expo Go notifications permissions are not scoped
      requester.requestAuthorizationOptions(options, resolver: promise.resolver, rejecter: promise.legacyRejecter)
    }
  }
}
Repository: expo/expo
File: packages/expo-crypto/src/ExpoCrypto.web.ts (TypeScript)

Code before:
import { CodedError, TypedArray } from 'expo-modules-core';

import { CryptoDigestAlgorithm, CryptoEncoding, CryptoDigestOptions } from './Crypto.types';

const getCrypto = (): Crypto => globalThis.crypto ?? window.crypto ?? (window as any).msCrypto;

export default {
  async digestStringAsync(
    algorithm: CryptoDigestAlgorithm,
    data: string,
    options: CryptoDigestOptions
  ): Promise<string> {
    const crypto = getCrypto();
    if (!crypto.subtle) {
      throw new CodedError(
        'ERR_CRYPTO_UNAVAILABLE',
        'Access to the WebCrypto API is restricted to secure origins (localhost/https).'
      );
    }
    const encoder = new TextEncoder();
    const buffer = encoder.encode(data);
    const hashedData = await crypto.subtle.digest(algorithm, buffer);
    if (options.encoding === CryptoEncoding.HEX) {
      return hexString(hashedData);
    } else if (options.encoding === CryptoEncoding.BASE64) {
      return btoa(String.fromCharCode(...new Uint8Array(hashedData)));
    }
    throw new CodedError('ERR_CRYPTO_DIGEST', 'Invalid encoding type provided.');
  },
  getRandomBytes(length: number): Uint8Array {

Review comment (suggestion):
```suggestion
const getCrypto = (): Crypto => {
  if (typeof globalThis.crypto !== 'undefined') {
    return globalThis.crypto;
  } else if (typeof window !== 'undefined') {
    return window.crypto ?? (window as any).msCrypto;
  } else {
    return crypto;
  }
};
```

There's other ways to write this, this isn't necessarily the most elegant way, since we can also just do `typeof crypto !== 'undefined' ? crypto : ...` but `getCrypto` has no guard, so we should rely on the `ReferenceError` that `return crypto` should throw.

Mainly, my concern here is that we should avoid `?? window...` when it could be undefined, for completeness to align with the Babel transformation we have for this

Code after:
import { CodedError, TypedArray } from 'expo-modules-core';

import { CryptoDigestAlgorithm, CryptoEncoding, CryptoDigestOptions } from './Crypto.types';

const getCrypto = (): Crypto => {
  if (typeof globalThis.crypto !== 'undefined') {
    return globalThis.crypto;
  } else if (typeof window !== 'undefined') {
    return window.crypto ?? (window as any).msCrypto;
  } else {
    return crypto;
  }
};

export default {
  async digestStringAsync(
    algorithm: CryptoDigestAlgorithm,
    data: string,
    options: CryptoDigestOptions
  ): Promise<string> {
    const crypto = getCrypto();
    if (!crypto.subtle) {
      throw new CodedError(
        'ERR_CRYPTO_UNAVAILABLE',
        'Access to the WebCrypto API is restricted to secure origins (localhost/https).'
      );
    }
    const encoder = new TextEncoder();
    const buffer = encoder.encode(data);
    const hashedData = await crypto.subtle.digest(algorithm, buffer);
Repository: expo/expo
File: packages/expo-router/ios/Tests/RNScreensTabCompatTests.swift (Swift)

Code before:
private class MockTabHostWithBadController: UIView {
  @objc var controller: NSObject? = NSObject()
}

/// Mock view with a reactViewController() method that returns a UIViewController.
private class MockTabScreenWithReactVC: UIView {
  @objc var tabKey: String?
  private var _reactViewController: UIViewController?

  func configure(reactViewController: UIViewController) {
    _reactViewController = reactViewController
  }

  @objc override func reactViewController() -> UIViewController? {
    return _reactViewController
  }
}

// MARK: - Unit tests

@Suite("RNScreensTabCompat unit tests")
struct RNScreensTabCompatUnitTests {

  // MARK: isTabScreen

  @Test func isTabScreen_detectsMockTabScreen() {
    let tabScreen = MockTabScreenView()
    #expect(RNScreensTabCompat.isTabScreen(tabScreen))
  }

  @Test func isTabScreen_rejectsPlainUIView() {
    let plainView = UIView()
    #expect(!RNScreensTabCompat.isTabScreen(plainView))
  }

  @Test func isTabScreen_rejectsMockTabHost() {
    let tabHost = MockTabHostView()
    #expect(!RNScreensTabCompat.isTabScreen(tabHost))
  }

  // MARK: tabKey

  @Test func tabKey_readsValue() {
    let tabScreen = MockTabScreenView()
    tabScreen.tabKey = "home"
    #expect(RNScreensTabCompat.tabKey(from: tabScreen) == "home")
  }

  @Test func tabKey_returnsNilForNilTabKey() {
    let tabScreen = MockTabScreenView()
    tabScreen.tabKey = nil

Review comment (suggestion):
Looks like Claude was not able to detect our convention 🙈 

```suggestion
  @Test 
  func `detects mock tab screen`() {
```

Code after:
private class MockTabHostWithBadController: UIView {
  @objc var controller: NSObject? = NSObject()
}

/// Mock view with a reactViewController() method that returns a UIViewController.
private class MockTabScreenWithReactVC: UIView {
  @objc var tabKey: String?
  private var _reactViewController: UIViewController?

  func configure(reactViewController: UIViewController) {
    _reactViewController = reactViewController
  }

  @objc override func reactViewController() -> UIViewController? {
    return _reactViewController
  }
}

// MARK: - Unit tests

@Suite("RNScreensTabCompat unit tests")
struct RNScreensTabCompatUnitTests {

  @Suite("isTabScreen")
  struct IsTabScreen {
    @Test
    func `detects mock tab screen`() {
      let tabScreen = MockTabScreenView()
      #expect(RNScreensTabCompat.isTabScreen(tabScreen))
    }

    @Test
    func `rejects plain UIView`() {
      let plainView = UIView()
      #expect(!RNScreensTabCompat.isTabScreen(plainView))
    }

    @Test
    func `rejects mock tab host`() {
      let tabHost = MockTabHostView()
      #expect(!RNScreensTabCompat.isTabScreen(tabHost))
    }
  }

  @Suite("tabKey")
  struct TabKey {
    @Test
    func `reads value`() {
      let tabScreen = MockTabScreenView()
      tabScreen.tabKey = "home"
      #expect(RNScreensTabCompat.tabKey(from: tabScreen) == "home")
Repository: expo/expo
File: packages/expo-ui/src/swift-ui/ControlGroup/index.tsx (TypeScript)

Code before:
import { requireNativeView } from 'expo';
import { type ReactNode, type ComponentType } from 'react';
import { type SFSymbol } from 'sf-symbols-typescript';

import { createViewModifierEventListener } from '../modifiers/utils';
import { type CommonViewModifierProps } from '../types';

export interface ControlGroupProps extends CommonViewModifierProps {
  /**
   * The label for the control group. Can be a string for simple text labels,
   * or a Label component for custom label content. When omitted, the control group
   * has no label.
   * @platform iOS
   * @platform tvOS 17.0+
   */
  label?: string | ReactNode;
  /**
   * An SF Symbol name to display alongside the label.
   * Only used when `label` is a string.
   * @platform iOS 16.0+
   * @platform tvOS 17.0+
   */
  systemImage?: SFSymbol;
  /**
   * The control group's content.
   * Can contain `Button`, `Toggle`, `Picker`, or other interactive controls.
   * @platform iOS
   * @platform tvOS 17.0+
   */
  children: ReactNode;
}

type NativeControlGroupProps = Omit<ControlGroupProps, 'label'> & {
  label?: string;
};


Review comment (suggestion):
```suggestion
   * or a `Label` component for custom label content. When omitted, the control group
```

Missing inline syntax highlight for the component name.

Code after:
import { requireNativeView } from 'expo';
import { type ReactNode, type ComponentType } from 'react';
import { type SFSymbol } from 'sf-symbols-typescript';

import { createViewModifierEventListener } from '../modifiers/utils';
import { type CommonViewModifierProps } from '../types';

export interface ControlGroupProps extends CommonViewModifierProps {
  /**
   * The label for the control group. Can be a string for simple text labels,
   * or a `Label` component for custom label content. When omitted, the control group
   * has no label.
   * @platform iOS 16.0+
   * @platform tvOS 17.0+
   */
  label?: string | ReactNode;
  /**
   * An SF Symbol name to display alongside the label.
   * Only used when `label` is a string.
   * @platform iOS 16.0+
   * @platform tvOS 17.0+
   */
  systemImage?: SFSymbol;
  /**
   * The control group's content.
   * Can contain `Button`, `Toggle`, `Picker`, or other interactive controls.
   * @platform iOS
   * @platform tvOS 17.0+
   */
  children: ReactNode;
}

type NativeControlGroupProps = Omit<ControlGroupProps, 'label'> & {
  label?: string;
};