ronantakizawa/codereview-bench huggingface dataset:
Repository: expo/expo
File: packages/expo-ui/src/swift-ui/Section/index.tsx (TypeScript)

Code before:
import { requireNativeView } from 'expo';

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

export type SectionProps = {
  /**
   * On iOS, section titles are usually capitalized for consistency with platform conventions.
   */
  title?: string;
  /**
   * Sets the section footer text.
   * @description If `expanded` is `true`, the footer will not be shown.
   */
  footer?: string;
  children: React.ReactNode;
  /**
   * Enables or disables collapsible behavior for the section.
   * @default false
   * @availability Available only when the list style is set to `sidebar`.
   */
  collapsible?: boolean;
} & CommonViewModifierProps;

const SectionNativeView: React.ComponentType<SectionProps> = requireNativeView(
  'ExpoUI',
  'SectionView'
);

/**
 * Section component uses the native [Section](https://developer.apple.com/documentation/swiftui/section) component.
 * It has no intrinsic dimensions, so it needs explicit height or flex set to display content (like ScrollView).
 * @platform ios
 */
export function Section(props: SectionProps) {
  const { modifiers, ...restProps } = props;
  return (
    <SectionNativeView

Review comment (suggestion):
we can remove the `expanded` from description as we don't control the state from JS yet

suggestion description: If section is expanded, the footer will not be shown.




Code after:
import { requireNativeView } from 'expo';

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

export type SectionProps = {
  /**
   * On iOS, section titles are usually capitalized for consistency with platform conventions.
   */
  title?: string;
  /**
   * Sets a custom footer for the section.
   * @description If section is expanded, the footer will not be shown.
   */
  footer?: React.ReactNode;
  /**
   * Sets a custom header for the section.
   * @description This will replace the default section header with your own content.
   */
  header?: React.ReactNode;
  children: React.ReactNode;
  /**
   * Enables or disables collapsible behavior for the section.
   * @default false
   * @availability Available only when the list style is set to `sidebar`.
   */
  collapsible?: boolean;
} & CommonViewModifierProps;

const SectionNativeView: React.ComponentType<SectionProps> = requireNativeView(
  'ExpoUI',
  'SectionView'
);

const SectionHeader: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionHeader');

const SectionFooter: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionFooter');
Repository: expo/expo
File: packages/expo-ui/ios/SectionView.swift (Swift)

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

import SwiftUI
import ExpoModulesCore

final class SectionProps: ExpoSwiftUI.ViewProps, CommonViewModifierProps {
  @Field var fixedSize: Bool?
  @Field var frame: FrameOptions?
  @Field var padding: PaddingOptions?
  @Field var testID: String?
  @Field var modifiers: ModifierArray?

  @Field var title: String?
  @Field var footer: String?
  @Field var collapsible: Bool = false
  var onStateChange = EventDispatcher()
}

internal struct SectionView: ExpoSwiftUI.View {
  @ObservedObject var props: SectionProps
  @State private var isExpanded: Bool = true

  var body: some View {
    Group {
      if #available(iOS 17.0, macOS 14.0, tvOS 17.0, *), props.collapsible {
        collapsibleSection
      } else {
        regularSection
      }
    }
    .modifier(CommonViewModifiers(props: props))
  }

  private var regularSection: some View {
    Section(
      header: Text(props.title ?? "").textCase(nil),
      footer: Text(props.footer ?? "").textCase(nil)
    ) {
      Children()
    }
  }

  @available(iOS 17.0, macOS 14.0, tvOS 17.0, *)
  private var collapsibleSection: some View {
    Section(isExpanded: $isExpanded) {
      Children()
    } header: {
      Text(props.title ?? "").textCase(nil)
    }
    .onChange(of: isExpanded) { newValue in
      props.onStateChange(["isExpanded": newValue])
    }
  }
}


Review comment (suggestion):
```suggestion
  var body: some View {
    if #available(iOS 17.0, macOS 14.0, tvOS 17.0, *), props.collapsible {
      collapsibleSection
        .modifier(CommonViewModifiers(props: props))
    } else {
      regularSection
        .modifier(CommonViewModifiers(props: props))
    }
  }
```

Code after:
    Children()
  }
}

internal final class SectionContentProps: ExpoSwiftUI.ViewProps {}
internal struct SectionContent: ExpoSwiftUI.View {
  @ObservedObject var props: SectionContentProps

  var body: some View {
    Children()
  }
}

internal struct SectionView: ExpoSwiftUI.View {
  @ObservedObject var props: SectionProps
  @State private var isExpanded: Bool = true

  var body: some View {
    if #available(iOS 17.0, macOS 14.0, tvOS 17.0, *), props.collapsible {
      collapsibleSection
        .modifier(CommonViewModifiers(props: props))
    } else {
      regularSection
        .modifier(CommonViewModifiers(props: props))
    }
  }

  @ViewBuilder
  private var contentChildren: some View {
    if let content = props.children?
      .compactMap({ $0.childView as? SectionContent })
      .first
    {
      content
    }
  }

  @ViewBuilder
  private var headerView: some View {
    if let header = props.children?
      .compactMap({ $0.childView as? SectionHeader })
      .first
    {
      header
    } else if let title = props.title, !title.isEmpty {
      Text(title).textCase(nil)
    }
  }

  @ViewBuilder
  private var footerView: some View {
Repository: expo/expo
File: packages/expo-ui/src/swift-ui/Section/index.tsx (TypeScript)

Code before:

const SectionNativeView: React.ComponentType<SectionProps> = requireNativeView(
  'ExpoUI',
  'SectionView'
);

const SectionHeader: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionHeader');

const SectionFooter: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionFooter');

const SectionContent: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionContent');

/**
 * Section component uses the native [Section](https://developer.apple.com/documentation/swiftui/section) component.
 * It has no intrinsic dimensions, so it needs explicit height or flex set to display content (like ScrollView).
 * @platform ios
 */
export function Section(props: SectionProps) {
  const { modifiers, header, footer, children, ...restProps } = props;
  return (
    <SectionNativeView
      modifiers={modifiers}
      {...(modifiers ? createViewModifierEventListener(modifiers) : undefined)}
      {...restProps}>
      {header && <SectionHeader>{header}</SectionHeader>}
      <SectionFooter>{footer}</SectionFooter>
      <SectionContent>{children}</SectionContent>
    </SectionNativeView>
  );
}


Review comment (suggestion):
```suggestion
{footer && <SectionFooter>{footer}</SectionFooter>}
```

Code after:

const SectionNativeView: React.ComponentType<SectionProps> = requireNativeView(
  'ExpoUI',
  'SectionView'
);

const SectionHeader: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionHeader');

const SectionFooter: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionFooter');

const SectionContent: React.ComponentType<object> = requireNativeView('ExpoUI', 'SectionContent');

/**
 * Section component uses the native [Section](https://developer.apple.com/documentation/swiftui/section) component.
 * It has no intrinsic dimensions, so it needs explicit height or flex set to display content (like ScrollView).
 */
export function Section(props: SectionProps) {
  const { modifiers, header, footer, children, ...restProps } = props;
  return (
    <SectionNativeView
      modifiers={modifiers}
      {...(modifiers ? createViewModifierEventListener(modifiers) : undefined)}
      {...restProps}>
      {header && <SectionHeader>{header}</SectionHeader>}
      {footer && <SectionFooter>{footer}</SectionFooter>}
      <SectionContent>{children}</SectionContent>
    </SectionNativeView>
  );
}
Repository: expo/expo
File: packages/expo-image/src/Image.types.ts (TypeScript)

Code before:

  /**
   * If provided, the image will be automatically resized to not exceed this height in pixels, preserving its aspect ratio.
   */
  maxHeight?: number;

  /**
   * Function to call when the image has failed to load. In addition to the error, it also provides a function that retries loading the image.
   */
  onError?(error: Error, retry: () => void): void;
};

/**
 * An object containing options for the [`configureCache`](#configurecacheconfig) function.
 * See [SDImageCacheConfig](https://sdwebimage.github.io/documentation/sdwebimage/sdimagecacheconfig) for more information.
 * @platform ios
 */
export type ImageCacheConfig = {
  /**
   * The maximum size of the disk cache, in bytes.
   * Defaults to 0, which means there is no cache size limit.
   */
  maxDiskSize?: number;

  /**
   * The maximum “total cost” of the in-memory image cache. The cost function is the bytes size held in memory.
   */
  maxMemoryCost?: number;

  /**
   * The maximum number of objects the in-memory image cache should hold.
   * Defaults to 0, which means there is no memory count limit.
   */
  maxMemoryCount?: number;
};


Review comment (performance):
```suggestion
   * The maximum "total cost" of the in-memory image cache. The cost function is the bytes size held in memory.
```

- We don't use curly quotes in docs.
- Is there a default value for this property? If yes, it'd be nice to mention that here.

Code after:

  /**
   * If provided, the image will be automatically resized to not exceed this height in pixels, preserving its aspect ratio.
   */
  maxHeight?: number;

  /**
   * Function to call when the image has failed to load. In addition to the error, it also provides a function that retries loading the image.
   */
  onError?(error: Error, retry: () => void): void;
};

/**
 * An object containing options for the [`configureCache`](#configurecacheconfig) function.
 * See [`SDImageCacheConfig`](https://sdwebimage.github.io/documentation/sdwebimage/sdimagecacheconfig) for more information.
 * @platform ios
 */
export type ImageCacheConfig = {
  /**
   * The maximum size of the disk cache, in bytes.
   * Defaults to 0, which means there is no cache size limit.
   */
  maxDiskSize?: number;

  /**
   * The maximum "total cost" of the in-memory image cache. The cost function is the bytes size held in memory,
   * not simply the pixel count. For example, a typical ARGB8888 image uses 4 bytes (32 bits) per pixel.
   * Defaults to 0, which means there is no memory cost limit.
   */
  maxMemoryCost?: number;

  /**
   * The maximum number of objects the in-memory image cache should hold.
   * Defaults to 0, which means there is no memory count limit.
   */
  maxMemoryCount?: number;
};
Repository: expo/expo
File: packages/expo-localization/plugin/__tests__/withExpoLocalization-test.ts (TypeScript)

Code before:
  });

  it('should handle underscore separators', () => {
    expect(convertBcp47ToAndroidLocale('en_US')).toBe('en-rUS');
    expect(convertBcp47ToAndroidLocale('fr_CA')).toBe('fr-rCA');
    expect(convertBcp47ToAndroidLocale('pt_BR')).toBe('pt-rBR');
  });

  it('should normalize language codes to lowercase and region codes to uppercase', () => {
    expect(convertBcp47ToAndroidLocale('EN-us')).toBe('en-rUS');
    expect(convertBcp47ToAndroidLocale('Fr-ca')).toBe('fr-rCA');
    expect(convertBcp47ToAndroidLocale('PT-br')).toBe('pt-rBR');
  });

  it('should return already formatted Android locales unchanged', () => {
    expect(convertBcp47ToAndroidLocale('en-rUS')).toBe('en-rUS');
    expect(convertBcp47ToAndroidLocale('pt-rBR')).toBe('pt-rBR');
    expect(convertBcp47ToAndroidLocale('zh-rCN')).toBe('zh-rCN');
  });

  it('should handle numeric region codes (UN M.49)', () => {
    expect(convertBcp47ToAndroidLocale('es-419')).toBe('es-r419');
  });

  it('should handle edge cases with only script codes (no region)', () => {
    // When there's only a script code and no region, should return just the language
    expect(convertBcp47ToAndroidLocale('zh-Hans')).toBe('zh');
    expect(convertBcp47ToAndroidLocale('sr-Latn')).toBe('sr');
  });
});


Review comment (suggestion):
The script cannot be dropped because it can be used to differentiate between locale resources. `zh-Hans` vs `zh-Hant` to select between Simplified and Traditional Chinese strings is a prominent example.

Code after:
import { convertBcp47ToResourceQualifier } from '../src/withExpoLocalization';

describe('converts locales to BCP-47 format', () => {
  it('should convert simple language codes to BCP-47 format', () => {
    expect(convertBcp47ToResourceQualifier('en')).toBe('b+en');
    expect(convertBcp47ToResourceQualifier('fr')).toBe('b+fr');
    expect(convertBcp47ToResourceQualifier('de')).toBe('b+de');
    expect(convertBcp47ToResourceQualifier('es')).toBe('b+es');
    expect(convertBcp47ToResourceQualifier('ja')).toBe('b+ja');
    expect(convertBcp47ToResourceQualifier('zh')).toBe('b+zh');
  });

  it('should convert language-region codes to BCP-47 format', () => {
    expect(convertBcp47ToResourceQualifier('en-US')).toBe('b+en+US');
    expect(convertBcp47ToResourceQualifier('en-GB')).toBe('b+en+GB');
    expect(convertBcp47ToResourceQualifier('zh-Hans')).toBe('b+zh+Hans');
    expect(convertBcp47ToResourceQualifier('zh-Hant')).toBe('b+zh+Hant');
    expect(convertBcp47ToResourceQualifier('es-419')).toBe('b+es+419');
    expect(convertBcp47ToResourceQualifier('zh-Hant-TW')).toBe('b+zh+Hant+TW');
  });
});
Repository: expo/expo
File: packages/expo-ui/ios/ChartView.swift (Swift)

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

import SwiftUI
import ExpoModulesCore
import Charts

enum ChartType: String, Enumerable {
  case line
  case point
  case bar
  case area
  case pie
  case rectangle
}

struct ChartDataPoint: Record {
  @Field var xString: String?
  @Field var xNumber: Double?
  @Field var y: Double
  @Field var color: Color?

  var xValue: String {
    if let s = xString {
      return s
    }
    if let n = xNumber {
      return String(n)
    }
    return ""
  }

  var xNumericValue: Double? {
    return xNumber
  }
}

enum PointStyle: String, Enumerable {
  case circle
  case square
  case diamond
}

struct LineChartStyle: Record {

Review comment (suggestion):
We can use `Either` here. `Either` requires to be optional `Either<String, Double>?` and is supported. Using `x?.get()` we can get the actual value. We can also remove the JS transform map then.

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

import SwiftUI
import ExpoModulesCore
import Charts

enum ChartType: String, Enumerable {
  case line
  case point
  case bar
  case area
  case pie
  case rectangle
}

struct ChartDataPoint: Record {
  @Field var x: Either<String, Double>?
  @Field var y: Double
  @Field var color: Color?

  var xValue: String {
    if let s: String = x?.get() {
      return s
    }
    if let n: Double = x?.get() {
      return String(n)
    }
    return ""
  }

  var xNumericValue: Double? {
    return x?.get()
  }
}

enum PointStyle: String, Enumerable {
  case circle
  case square
  case diamond
}

struct LineChartStyle: Record {
  @Field var dashArray: [Double]?
Repository: expo/expo
File: apps/native-component-list/src/screens/Contacts@Next/ContactsNextFormScreen.tsx (TypeScript)

Code before:
import React, { useEffect, useState } from 'react';
import { View, StyleSheet, Text, ScrollView } from 'react-native';
import Button from '../../components/Button';
import { Contact } from 'expo-contacts/next';

export default function ContactApiScreen() {
  const [contact, setContact] = useState<Contact | null>(null);
  const [status, setStatus] = useState<string>('');
  const [loading, setLoading] = useState(false);

  useEffect(() => {
    Contact.requestPermissionsAsync();
  }, []);

  const execute = async (fn: () => Promise<any>) => {
    setLoading(true);
    try {
      const result = await fn();
      setStatus(JSON.stringify(result ?? null, null, 2));
    } catch (error: any) {
      console.error(error);
      setStatus(JSON.stringify({ error: error.message }, null, 2));
    } finally {
      setLoading(false);
    }
  };

  const handleCreate = () => {
    execute(async () => {
      const newContact = await Contact.create({
        givenName: 'Init',
        familyName: 'Contact',
      });
      setContact(newContact);
      return newContact;

Review comment (suggestion):
```suggestion
  const [loading, setLoading] = useState(false);
```

not a huge deal, given this is a tiny example screen, but considering there's a `status` state, it feels natural it'd also handle the `loading` state at the same time

Code after:
import { Contact } from 'expo-contacts/next';
import React, { useEffect, useState } from 'react';
import { View, StyleSheet, Text, ScrollView } from 'react-native';

import Button from '../../components/Button';

export default function ContactApiScreen() {
  const [contact, setContact] = useState<Contact | null>(null);
  const [result, setResult] = useState<string>('');
  const [loading, setLoading] = useState(false);

  useEffect(() => {
    Contact.requestPermissionsAsync();
  }, []);

  const execute = async (fn: () => Promise<any>) => {
    setLoading(true);
    try {
      const result = await fn();
      setResult(JSON.stringify(result ?? null, null, 2));
    } catch (error: any) {
      console.error(error);
      setResult(JSON.stringify({ error: error.message }, null, 2));
    } finally {
      setLoading(false);
    }
  };

  const handleCreate = () => {
    execute(async () => {
      const newContact = await Contact.create({
        givenName: 'Init',
        familyName: 'Contact',
      });
      setContact(newContact);
Repository: expo/expo
File: packages/expo-ui/src/swift-ui/Menu/types.ts (TypeScript)

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

import { type CommonViewModifierProps } from '../types';

/**
 * Props for the `Menu` component.
 */
export type MenuProps = {
  /**
   * The label for the menu trigger. Can be a string for simple text labels,
   * or a ReactNode for custom label content.
   */
  label: string | ReactNode;
  /**
   * An SF Symbol name to display alongside the label.
   * Only used when `label` is a string.
   */
  systemImage?: string;
  /**
   * A callback that is invoked when the user taps the menu label.
   * When provided, a single tap triggers this action, while a long-press shows the menu.
   * When not provided, a single tap shows the menu.
   */
  onPrimaryAction?: () => void;
  /**
   * The content of the menu - the items shown when the menu is opened.
   * Can contain `Button`, `Switch`, `Picker`, `Section`, `Divider`, or nested `Menu` components.
   */
  children: ReactNode;
} & CommonViewModifierProps;


Review comment (suggestion):
```suggestion
   * The menu's content items, which are shown when the menu is opened.
```

Let's improve the readability of this sentence.

Code after:
import { ReactNode } from 'react';

import { type CommonViewModifierProps } from '../types';

/**
 * Props for the `Menu` component.
 */
export type MenuProps = {
  /**
   * The label for the menu trigger. Can be a string for simple text labels,
   * or a ReactNode for custom label content.
   */
  label: string | ReactNode;
  /**
   * An SF Symbol name to display alongside the label.
   * Only used when `label` is a string.
   */
  systemImage?: string;
  /**
   * A callback that is invoked when the user taps the menu label.
   * When provided, a single tap triggers this action, while a long-press shows the menu.
   * When not provided, a single tap shows the menu.
   */
  onPrimaryAction?: () => void;
  /**
   * The menu's content items, which are shown when the menu is opened.
   * Can contain `Button`, `Switch`, `Picker`, `Section`, `Divider`, or nested `Menu` components.
   */
  children: ReactNode;
} & CommonViewModifierProps;
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 {

Review comment (suggestion):
```suggestion
internal struct MenuView: ExpoSwiftUI.View {
```

explitly internal or just make it public (as we now opened some public views)

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() }