ronantakizawa/codereview-bench huggingface dataset:
Repository: expo/expo
File: packages/expo-ui/ios/Menu/MenuView.swift (Swift)

Code before:
// Copyright 2025-present 650 Industries. All rights reserved.

import SwiftUI
import ExpoModulesCore

struct MenuView: ExpoSwiftUI.View {
  @ObservedObject var props: MenuProps

  var body: some View {
    if #available(iOS 14.0, tvOS 17.0, *) {
      let labelContent = props.children?
        .compactMap { $0.childView as? MenuLabel }
        .first

      if props.hasPrimaryAction {
        // With primaryAction, tap triggers callback and long-press shows menu
        if let systemImage = props.systemImage, let label = props.label {
          // primary action initializers require a LocalizedStringKey
          Menu(LocalizedStringKey(label), systemImage: systemImage) { Children() } primaryAction: { props.onPrimaryAction() }
        } else if let labelContent {
          Menu { Children() } label: { labelContent } primaryAction: { props.onPrimaryAction() }
        } else if let label = props.label {
          Menu(LocalizedStringKey(label)) { Children() } primaryAction: { props.onPrimaryAction() }
        }
      } else {
        // Without primaryAction, tap shows menu
        if let systemImage = props.systemImage, let label = props.label {
          Menu(label, systemImage: systemImage) { Children() }
        } else if let labelContent {
          Menu { Children() } label: { labelContent }
        } else if let label = props.label {
          Menu(label) { Children() }
        }
      }
    }
  }
}


Review comment (suggestion):
```suggestion
```

reduce some trivial comments

Code after:
// Copyright 2025-present 650 Industries. All rights reserved.

import SwiftUI
import ExpoModulesCore

internal struct MenuView: ExpoSwiftUI.View {
  @ObservedObject var props: MenuProps

  var body: some View {
    if #available(iOS 14.0, tvOS 17.0, *) {
      let labelContent = props.children?
        .compactMap { $0.childView as? MenuLabel }
        .first

      if props.hasPrimaryAction {
        // With primaryAction, tap triggers callback and long-press shows menu
        if let systemImage = props.systemImage, let label = props.label {
          Menu(LocalizedStringKey(label), systemImage: systemImage) { Children() } primaryAction: { props.onPrimaryAction() }
        } else if let labelContent {
          Menu { Children() } label: { labelContent } primaryAction: { props.onPrimaryAction() }
        } else if let label = props.label {
          Menu(LocalizedStringKey(label)) { Children() } primaryAction: { props.onPrimaryAction() }
        }
      } else {
        // Without primaryAction, tap shows menu
        if let systemImage = props.systemImage, let label = props.label {
          Menu(label, systemImage: systemImage) { Children() }
        } else if let labelContent {
          Menu { Children() } label: { labelContent }
        } else if let label = props.label {
          Menu(label) { Children() }
        }
      }
    }
  }
}
Repository: expo/expo
File: packages/expo-video/src/VideoPlayer.types.ts (TypeScript)

Code before:
  id?: string;

  /**
   * Language of the subtitle track. For example, `en`, `pl`, `de`.
   */
  language: string;

  /**
   * Label of the subtitle track in the language of the device.
   */
  label: string;
};

/**
 * Specifies a VideoTrack loaded from a [`VideoSource`](#videosource).
 */
export type VideoTrack = {
  /**
   * The id of the video track.
   *
   * > This field is platform-specific and may return different depending on the operating system.
   */
  id: string;

  /**
   * The url of the VideoTrack for HLS video sources, null for other source types.
   */
  url: string | null;

  /**
   * Size of the video track.
   */
  size: VideoSize;

  /**
   * MimeType of the video track or null if unknown.
   */
  mimeType: string | null;

  /**
   * Indicates whether the video track format is supported by the device.
   *
   * @platform android
   */
  isSupported: boolean;

  /**
   * Specifies the bitrate in bits per second. This is the peak bitrate if known, or else the average bitrate if known, or else null.
   *
   * @deprecated Use `peakBitrate` or `averageBitrate` instead.
   */

Review comment (suggestion):
```suggestion
   * The URL of the `VideoTrack` for HLS video sources. `null` for other source types.
```

Code after:
  /**
   * A string used by `expo-video` to identify the subtitle track.
   *
   * @platform android
   */
  id?: string;

  /**
   * Language of the subtitle track. For example, `en`, `pl`, `de`.
   */
  language: string;

  /**
   * Label of the subtitle track in the language of the device.
   */
  label: string;
};

/**
 * Specifies a VideoTrack loaded from a [`VideoSource`](#videosource).
 */
export type VideoTrack = {
  /**
   * The id of the video track.
   *
   * > This field is platform-specific and may return different depending on the operating system.
   */
  id: string;

  /**
   * The URL of the `VideoTrack` for HLS video sources. `null` for other source types.
   */
  url: string | null;

  /**
   * Size of the video track.
   */
  size: VideoSize;

  /**
   * MimeType of the video track or null if unknown.
   */
  mimeType: string | null;

  /**
   * Indicates whether the video track format is supported by the device.
   *
   * @platform android
   */
  isSupported: boolean;
Repository: expo/expo
File: packages/expo-glass-effect/src/GlassView.types.ts (TypeScript)

Code before:
import { type RefObject } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassStyle = 'clear' | 'regular';

export type GlassViewProps = {
  /**
   * Glass effect style to apply to the view.
   * @default 'regular'
   */
  glassEffectStyle?: GlassStyle;
  /**
   * Tint color to apply to the glass effect.
   */
  tintColor?: string;
  /**
   * Whether the glass effect should be interactive.
   * @default false
   */
  isInteractive?: boolean;
  ref?: RefObject<View | null>;
} & ViewProps;


Review comment (suggestion):
```suggestion
  ref?: Ref<View>;
```

Ref also includes callback ref type

Code after:
import { type Ref } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassStyle = 'clear' | 'regular';

export type GlassViewProps = {
  /**
   * Glass effect style to apply to the view.
   * @default 'regular'
   */
  glassEffectStyle?: GlassStyle;
  /**
   * Tint color to apply to the glass effect.
   */
  tintColor?: string;
  /**
   * Whether the glass effect should be interactive.
   * @default false
   */
  isInteractive?: boolean;
  ref?: Ref<View>;
} & ViewProps;
Repository: expo/expo
File: packages/expo-glass-effect/src/GlassView.types.ts (TypeScript)

Code before:
import { type RefObject } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassStyle = 'clear' | 'regular';

export type GlassViewProps = {
  /**
   * Glass effect style to apply to the view.
   * @default 'regular'
   */
  glassEffectStyle?: GlassStyle;
  /**
   * Tint color to apply to the glass effect.
   */
  tintColor?: string;
  /**
   * Whether the glass effect should be interactive.
   * @default false
   */
  isInteractive?: boolean;
  ref?: RefObject<View | null>;
} & ViewProps;


Review comment (suggestion):
```suggestion
import { type Ref } from 'react';
```

Code after:
import { type Ref } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassStyle = 'clear' | 'regular';

export type GlassViewProps = {
  /**
   * Glass effect style to apply to the view.
   * @default 'regular'
   */
  glassEffectStyle?: GlassStyle;
  /**
   * Tint color to apply to the glass effect.
   */
  tintColor?: string;
  /**
   * Whether the glass effect should be interactive.
   * @default false
   */
  isInteractive?: boolean;
  ref?: Ref<View>;
} & ViewProps;
Repository: expo/expo
File: packages/expo-glass-effect/src/GlassContainer.types.ts (TypeScript)

Code before:
import { type RefObject } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassContainerProps = {
  /**
   * The distance at which glass elements start affecting each other.
   * Controls when glass elements begin to merge together.
   * @default undefined
   */
  spacing?: number;
  ref?: RefObject<View | null>;
} & ViewProps;


Review comment (suggestion):
```suggestion
import { type Ref } from 'react';
```

Code after:
import { type Ref } from 'react';
import { View, type ViewProps } from 'react-native';

export type GlassContainerProps = {
  /**
   * The distance at which glass elements start affecting each other.
   * Controls when glass elements begin to merge together.
   * @default undefined
   */
  spacing?: number;
  ref?: Ref<View>;
} & ViewProps;
Repository: expo/expo
File: packages/expo-modules-autolinking/src/types.ts (TypeScript)

Code before:

  /**
   * The path to the project directory. Should contain the `build.gradle{.kts}` file.
   * It's relative to the module root directory.
   */
  path?: string;

  /**
   * Information about the available publication of an Android AAR file
   */
  publication?: AndroidPublication;

  /**
   * The path to the script that determines whether the publication should be used.
   * Evaluate in the context of the `settings.gradle` file.
   * Won't be run if the publication is not defined.
   */
  shouldUsePublicationScriptPath?: string;

  /**
   * List of modules provided by the package.
   */
  modules?: (string | RawAndroidModuleConfig)[];

  /**
   * Full qualified names of Android services provided by the package.
   */
  services?: string[];

  /**
   * Prebuilded AAR projects.
   */
  gradleAarProjects?: AndroidGradleAarProjectDescriptor[];

  gradlePath?: string;
};

export type RawAndroidConfig = {
  projects?: WithRequired<RawAndroidProjectConfig, 'name' | 'path'>[];
  /**
   * Gradle plugins.
   */
  gradlePlugins?: AndroidGradlePluginDescriptor[];

  /**
   * Gradle projects containing AAR files.
   */
} & RawAndroidProjectConfig;

/**
 * Represents a raw config from `expo-module.json`.

Review comment (suggestion):
```suggestion
   * Full qualified names of Android services (`expo.modules.kotlin.services.Service`) provided by the package.
```

the service name is a little confusing. i was thinking that is `android.app.Service`. so maybe explicitly saying that's the Service from expo modules

Code after:

  /**
   * The path to the project directory. Should contain the `build.gradle{.kts}` file.
   * It's relative to the module root directory.
   */
  path?: string;

  /**
   * Information about the available publication of an Android AAR file
   */
  publication?: AndroidPublication;

  /**
   * The path to the script that determines whether the publication should be used.
   * Evaluate in the context of the `settings.gradle` file.
   * Won't be run if the publication is not defined.
   */
  shouldUsePublicationScriptPath?: string;

  /**
   * List of modules provided by the package.
   */
  modules?: (string | RawAndroidModuleConfig)[];

  /**
   * Full qualified names of Android services (`expo.modules.kotlin.services.Service`) provided by the package.
   */
  services?: string[];

  /**
   * Prebuilded AAR projects.
   */
  gradleAarProjects?: AndroidGradleAarProjectDescriptor[];

  gradlePath?: string;
};

export type RawAndroidConfig = {
  projects?: WithRequired<RawAndroidProjectConfig, 'name' | 'path'>[];
  /**
   * Gradle plugins.
   */
  gradlePlugins?: AndroidGradlePluginDescriptor[];

  /**
   * Gradle projects containing AAR files.
   */
} & RawAndroidProjectConfig;

/**
 * Represents a raw config from `expo-module.json`.
Repository: expo/expo
File: packages/expo-router/src/toolbar/elements.tsx (TypeScript)

Code before:
    searchBarProps.placement !== 'integrated' && searchBarProps.placement !== 'integratedButton'
      ? 'integrated'
      : searchBarProps.placement;
  return (
    <RouterToolbarItem
      hidesSharedBackground={hidesSharedBackground}
      identifier={id}
      sharesBackground={sharesBackground}
      type="searchBar">
      <Stack.Screen options={{ headerSearchBarOptions: { ...searchBarProps, placement } }} />
    </RouterToolbarItem>
  );
};

/**
 * Props for the ToolbarView component.
 *
 * @platform ios
 */
export interface ToolbarViewProps {
  /**
   * React elements to render inside the toolbar view.
   */
  children: React.ReactNode;
  /**
   * Whether the view should be hidden.
   *
   * @default false
   */
  hidden?: boolean;
  /**
   * Whether to hide the shared background when `sharesBackground` is enabled.
   *
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/hidessharedbackground) for more information.
   *
   * @platform iOS 18+
   */
  hidesSharedBackground?: boolean;
  /**
   * Whether to separate the background of this item from other header items.
   *
   * This prop reverses the native behavior of `sharesBackground`.
   *
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/sharesbackground) for more information.
   *
   * @default false
   *
   * @platform iOS 26+
   */
  separateBackground?: boolean;
}

Review comment (suggestion):
So on older versions you need to use stack searchbar but it can have placement that puts it in the footer location? Maybe these should be unified somehow...

Code after:
   */
  sharesBackground?: boolean;
}

/**
 * Declares the position of a search bar within the toolbar.
 * It should only be used as a child of `Toolbar`.
 *
 * > **Note**: On iOS 26+, this component specifies where in the toolbar the search bar
 * > (configured via `Stack.SearchBar`) should appear. On iOS 18 and earlier, the search bar
 * > will be shown in the header instead.
 *
 * > **Important**: You must use `Stack.SearchBar` to configure and display the actual
 * > search bar. This component only declares its position in the toolbar.
 *
 * @example
 * ```tsx
 * <Stack.SearchBar placeholder="Search..." />
 * <Toolbar>
 *   <Toolbar.SearchBarPreferredSlot />
 *   <Toolbar.Spacer />
 *   <Toolbar.Button icon="mic" />
 * </Toolbar>
 * ```
 *
 * @platform ios 26+
 */
export const ToolbarSearchBarPreferredSlot = ({
  hidesSharedBackground,
  hidden,
  sharesBackground,
}: ToolbarSearchBarPreferredSlotProps) => {
  const id = useId();
  if (process.env.EXPO_OS !== 'ios' || parseInt(String(Platform.Version).split('.')[0], 10) < 26) {
    return null;
  }
  if (hidden) {
    return null;
  }
  return (
    <RouterToolbarItem
      hidesSharedBackground={hidesSharedBackground}
      identifier={id}
      sharesBackground={sharesBackground}
      type="searchBar"
    />
  );
};

/**
 * Props for the ToolbarView component.
Repository: expo/expo
File: packages/expo-router/src/toolbar/elements.tsx (TypeScript)

Code before:
  /**
   * Whether to hide the shared background.
   *
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/hidessharedbackground) for more information.
   *
   * @platform iOS 26+
   */
  hidesSharedBackground?: boolean;

  // TODO: support ImageSourcePropType icons in addition to SFSymbols
  /**
   * The name of the SF Symbol to display as the button icon.
   * For a list of available symbols, see [SF Symbols](https://developer.apple.com/sf-symbols/).
   */
  icon?: SFSymbol;

  /**
   * Custom image loaded using expo-image's `useImage` hook.
   * Takes priority over `icon` (SF Symbol) when both are provided.
   *
   * @example
   * ```tsx
   * import { useImage } from 'expo-image';
   * import { Toolbar } from 'expo-router/unstable-toolbar';
   *
   * const customIcon = useImage('https://picsum.photos/128', {
   *   maxWidth: 32,
   *   maxHeight: 32,
   * });
   *
   * <Toolbar.Button image={customIcon} onPress={() => {}} />
   * ```
   */
  image?: ImageRef | null;

  /**
   * Callback function when the button is pressed.
   */
  onPress?: () => void;

  /**
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/possibletitles) for more information.
   */
  possibleTitles?: string[];

  /**
   * Whether the button is in a selected state
   *
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/isselected) for more information.
   */
  selected?: boolean;

Review comment (suggestion):
Maybe something a bit more durable like github.com/expo.png or more practical like svg:

```suggestion
   * const customIcon = useImage('https://simpleicons.org/icons/expo.svg', {
```

Code after:
   * ```
   */
  children?: ReactNode;
  disabled?: boolean;

  /**
   * Whether the button should be hidden.
   *
   * @default false
   */
  hidden?: boolean;

  /**
   * Whether to hide the shared background.
   *
   * @see [Official Apple documentation](https://developer.apple.com/documentation/uikit/uibarbuttonitem/hidessharedbackground) for more information.
   *
   * @platform iOS 26+
   */
  hidesSharedBackground?: boolean;

  // TODO: support ImageSourcePropType icons in addition to SFSymbols
  /**
   * The name of the SF Symbol to display as the button icon.
   * For a list of available symbols, see [SF Symbols](https://developer.apple.com/sf-symbols/).
   */
  icon?: SFSymbol;

  /**
   * Custom image loaded using `useImage()` hook from `expo-image`.
   * Takes priority over `icon` (SF Symbol) when both are provided.
   *
   * @example
   * ```tsx
   * import { useImage } from 'expo-image';
   * import { Toolbar } from 'expo-router/unstable-toolbar';
   *
   * const customIcon = useImage('https://simpleicons.org/icons/expo.svg', {
   *   maxWidth: 24,
   *   maxHeight: 24,
   * });
   *
   * <Toolbar.Button image={customIcon} onPress={() => {}} />
   * ```
   */
  image?: ImageRef | null;

  /**
   * Callback function when the button is pressed.
   */
  onPress?: () => void;
Repository: expo/expo
File: packages/expo-modules-core/ios/Core/Records/FormattedRecord.swift (Swift)

Code before:
/**
 Class that binds a formatter with a record.
 It can be converted to JS, but it can't be converted from a JS value.
 */
struct FormattedRecord<RecordType: Record>: Convertible {
  internal final class FormattedRecordCannotBeUsedAsParameterException: Exception, @unchecked Sendable {
    override var reason: String {
      "FormattedRecord cann't be used as paremeter"
    }
  }

  internal let record: RecordType
  internal let formatter: Formatter<RecordType>

  func toDictionary(appContext: AppContext? = nil) -> Record.Dict {
    return fieldsOf(record).reduce(into: Record.Dict()) { result, field in
      if let key = field.key {
        let action = formatter.selectors.first { $0.name == key }?.action

        if let action = action {
          let formattedValue = action(record, field.get())
          if !(formattedValue is Skip) {
            result[key] = Conversions.convertFunctionResult(formattedValue, appContext: appContext)
          }
        } else {
          result[key] = Conversions.convertFunctionResult(field.get(), appContext: appContext)
        }
      }
    }
  }

  static func convertResult(_ result: Any, appContext: AppContext) throws -> Any {
    if let value = result as? FormattedRecord {

Review comment (suggestion):
```suggestion
      "FormattedRecord cannot be used as a parameter"
```

Code after:
/**
 Class that binds a formatter with a record.
 It can be converted to JS, but it can't be converted from a JS value.
 */
public struct FormattedRecord<RecordType: Record>: Convertible {
  internal final class FormattedRecordCannotBeUsedAsParameterException: Exception, @unchecked Sendable {
    override var reason: String {
      "FormattedRecord cannot be used as a parameter"
    }
  }

  internal let record: RecordType
  internal let formatter: Formatter<RecordType>

  func toDictionary(appContext: AppContext? = nil) -> Record.Dict {
    return fieldsOf(record).reduce(into: Record.Dict()) { result, field in
      if let key = field.key {
        if let action = formatter.selectors.first(where: { $0.name == key })?.action {
          let formattedValue = action(record, field.get())
          if !(formattedValue is Skip) {
            result[key] = Conversions.convertFunctionResult(formattedValue, appContext: appContext)
          }
        } else {
          result[key] = Conversions.convertFunctionResult(field.get(), appContext: appContext)
        }
      }
    }
  }

  public static func convertResult(_ result: Any, appContext: AppContext) throws -> Any {
    if let value = result as? FormattedRecord {
      return value.toDictionary(appContext: appContext)
    }
Repository: expo/expo
File: packages/expo-modules-core/ios/Core/Records/Formatter.swift (Swift)

Code before:
/**
 A marker object that allows us to skip calculating a property.
 */
enum Skip { case instance }

class PropertySelector<RecordType: Record> {
  class ActionBuilder<InputType> {
    typealias NextBuilder<T> = PropertySelector.ActionBuilder<T>

    private let propertySelector: PropertySelector<RecordType>

    init (selector: PropertySelector<RecordType>) {
      propertySelector = selector
    }

    @discardableResult func map<ResultType>(
      _ mapper: @escaping (RecordType, InputType) -> ResultType
    ) -> NextBuilder<ResultType> {
      return chainAction { record, value in mapper(record, value as! InputType ) }
    }

    @discardableResult func map<ResultType>(
      _ mapper: @escaping ( InputType) -> ResultType
    ) -> NextBuilder<ResultType> {
      return chainAction { _, value in mapper(value as! InputType ) }
    }

    @discardableResult func skip() -> NextBuilder<InputType> {
      return chainSkipAction(shouldSkip: { _, _ in true })
    }

    @discardableResult func skip(
      _ valueSelector: @escaping (RecordType, InputType) -> Bool
    ) -> NextBuilder<InputType> {
      return chainSkipAction(shouldSkip: valueSelector)
    }


Review comment (suggestion):
```suggestion
    init(selector: PropertySelector<RecordType>) {
```

Code after:
/**
 A marker object that allows us to skip calculating a property.
 */
enum Skip {
  case instance
}

public class PropertySelector<RecordType: Record> {
  public class ActionBuilder<InputType> {
    public typealias NextBuilder<T> = PropertySelector.ActionBuilder<T>

    private let propertySelector: PropertySelector<RecordType>

    internal init(selector: PropertySelector<RecordType>) {
      propertySelector = selector
    }

    @discardableResult
    public func map<ResultType>(
      _ mapper: @escaping (RecordType, InputType) -> ResultType
    ) -> NextBuilder<ResultType> {
      return chainAction { record, value in mapper(record, value as! InputType ) }
    }

    @discardableResult
    public func map<ResultType>(
      _ mapper: @escaping ( InputType) -> ResultType
    ) -> NextBuilder<ResultType> {
      return chainAction { _, value in mapper(value as! InputType ) }
    }

    @discardableResult
    public func skip() -> NextBuilder<InputType> {
      return chainSkipAction(shouldSkip: { _, _ in true })
    }

    @discardableResult