ronantakizawa/codereview-bench huggingface dataset:
Repository: electron-userland/electron-builder
File: packages/builder-util-runtime/src/httpExecutor.ts (TypeScript)

Code before:
          continue
        }
        throw e
      }
    }
  }
}

export interface DownloadCallOptions {
  responseHandler: ((response: IncomingMessage, callback: (error: Error | null) => void) => void) | null
  onCancel: (callback: () => void) => void
  callback: (error: Error | null) => void

  options: DownloadOptions

  destination: string | null
}

function parseUrl(url: string, options: RequestOptions): URL {
  if (url.startsWith("http://") || url.startsWith("https://")) {
    // Absolute URL
    return new URL(url)
  } else {
    // Relative URL - construct base URL from original options
    const protocol = options.protocol || "https:"
    const hostname = options.hostname || "localhost"
    const port = options.port ? `:${options.port}` : ""
    const baseUrl = `${protocol}//${hostname}${port}`
    return new URL(url, baseUrl)
  }
}

export function configureRequestOptionsFromUrl(url: string, options: RequestOptions) {
  const result = configureRequestOptions(options)
  const parsedUrl = parseUrl(url, options)
  configureRequestUrl(parsedUrl, result)
  return result
}

export function configureRequestUrl(url: URL, options: RequestOptions): void {
  options.protocol = url.protocol
  options.hostname = url.hostname
  if (url.port) {
    options.port = url.port
  } else if (options.port) {
    delete options.port
  }
  options.path = url.pathname + url.search
}

export class DigestTransform extends Transform {

Review comment (security):
Why do we have localhost as the fallback? That seems like a security vulnerability waiting to happen.

Code after:
          continue
        }
        throw e
      }
    }
  }
}

export interface DownloadCallOptions {
  responseHandler: ((response: IncomingMessage, callback: (error: Error | null) => void) => void) | null
  onCancel: (callback: () => void) => void
  callback: (error: Error | null) => void

  options: DownloadOptions

  destination: string | null
}

function parseUrl(url: string, options: RequestOptions): URL {
  try {
    // Would throw exception if url is not absolute
    return new URL(url)
  } catch {
    // Relative URL - construct base URL from original options
    const hostname = options.hostname
    const protocol = options.protocol || "https:"
    const port = options.port ? `:${options.port}` : ""
    const baseUrl = `${protocol}//${hostname}${port}`
    return new URL(url, baseUrl)
  }
}

export function configureRequestOptionsFromUrl(url: string, options: RequestOptions) {
  const result = configureRequestOptions(options)
  const parsedUrl = parseUrl(url, options)
  configureRequestUrl(parsedUrl, result)
  return result
}

export function configureRequestUrl(url: URL, options: RequestOptions): void {
  options.protocol = url.protocol
  options.hostname = url.hostname
  if (url.port) {
    options.port = url.port
  } else if (options.port) {
    delete options.port
  }
  options.path = url.pathname + url.search
}

export class DigestTransform extends Transform {
Repository: electron-userland/electron-builder
File: packages/app-builder-lib/src/node-module-collector/packageManager.ts (TypeScript)

Code before:
  ]

  const pms = Object.values(PM).filter(pm => pm !== PM.YARN_BERRY)
  for (const checker of priorityChecklist) {
    for (const pm of pms) {
      if (checker(pm)) {
        return pm
      }
    }
  }
  return null
}

export function detectPackageManagerByLockfile(cwd: string): PM | null {
  const has = (file: string) => fs.existsSync(path.join(cwd, file))

  const yarn = has("yarn.lock")
  const pnpm = has("pnpm-lock.yaml")
  const npm = has("package-lock.json")
  const bun = has("bun.lock") || has("bun.lockb")

  const detected: PM[] = []
  if (yarn) detected.push(PM.YARN)
  if (pnpm) detected.push(PM.PNPM)
  if (npm) detected.push(PM.NPM)
  if (bun) detected.push(PM.BUN)

  if (detected.length === 1) {
    return detected[0]
  }

  return null
}

export function detectYarnBerry() {
  // yarn --version
  const version = execSync("yarn --version").toString().trim()
  if (parseInt(version.split(".")[0]) > 1) return PM.YARN_BERRY
  return PM.YARN
}


Review comment (suggestion):
Please consider prettier settings for mult-line `if` statements. This can all be simplified to the below since `has` is already pretty self-explanatory.
```
const detected: PM[] = []
if (has("yarn.lock")) {
  detected.push(PM.YARN)
}
if (has("pnpm-lock.yaml")) {
  detected.push(PM.PNPM)
}
if (has("package-lock.json")) {
  detected.push(PM.NPM)
}
if (has("bun.lock") || has("bun.lockb")) {
  detected.push(PM.BUN)
}
```

Code after:
  ]

  const pms = Object.values(PM).filter(pm => pm !== PM.YARN_BERRY)
  for (const checker of priorityChecklist) {
    for (const pm of pms) {
      if (checker(pm)) {
        return pm
      }
    }
  }
  return null
}

export function detectPackageManagerByLockfile(cwd: string): PM | null {
  const has = (file: string) => fs.existsSync(path.join(cwd, file))

  const detected: PM[] = []
  if (has("yarn.lock")) {
    detected.push(PM.YARN)
  }
  if (has("pnpm-lock.yaml")) {
    detected.push(PM.PNPM)
  }
  if (has("package-lock.json")) {
    detected.push(PM.NPM)
  }
  if (has("bun.lock") || has("bun.lockb")) {
    detected.push(PM.BUN)
  }

  if (detected.length === 1) {
    return detected[0]
  }

  return null
}

export function detectYarnBerry() {
  // yarn --version
  const version = execSync("yarn --version").toString().trim()
  if (parseInt(version.split(".")[0]) > 1) return PM.YARN_BERRY
  return PM.YARN
}
Repository: element-plus/element-plus
File: packages/components/segmented/src/segmented.vue (Vue)

Code before:
const activeElement = useActiveElement()

const state = reactive({
  isInit: false,
  width: 0,
  height: 0,
  translateX: 0,
  translateY: 0,
  focusVisible: false,
})

const handleChange = (item: Option, evt: Event) => {
  const value = getValue(item)
  emit(UPDATE_MODEL_EVENT, value)
  emit(CHANGE_EVENT, value)

  if (!getSelected(item)) {
    evt.target && ((evt.target as HTMLInputElement).checked = false)
    if (!segmentedRef.value) return
    const selectedItemInput = segmentedRef.value.querySelector(
      '.is-selected input'
    ) as HTMLInputElement
    if (selectedItemInput) {
      selectedItemInput.checked = true
    }
  }
}

const getValue = (item: Option) => {
  return isObject(item) ? item.value : item
}

const getLabel = (item: Option) => {
  return isObject(item) ? item.label : item
}

const getDisabled = (item: Option | undefined) => {
  return !!(_disabled.value || (isObject(item) ? item.disabled : false))
}

const getSelected = (item: Option) => {
  return props.modelValue === getValue(item)
}

const getOption = (value: any) => {
  return props.options.find((item) => getValue(item) === value)
}

const getItemCls = (item: Option) => {
  return [
    ns.e('item'),

Review comment (bug):
It seems to follow the radio component on change.
https://github.com/element-plus/element-plus/blob/dev/packages/components/radio/src/use-radio.ts#L37

reason: By default clicking on a radio element will set checked=true without any restriction. Am i wrong ? @ykcory

I think it can be simplified:
```suggestion
  ;(evt.target as HTMLInputElement).checked = value === props.modelValue
```

Code after:
const segmentedRef = ref<HTMLElement | null>(null)
const activeElement = useActiveElement()

const state = reactive({
  isInit: false,
  width: 0,
  height: 0,
  translateX: 0,
  translateY: 0,
  focusVisible: false,
})

const handleChange = (evt: Event, item: Option) => {
  const value = getValue(item)
  emit(UPDATE_MODEL_EVENT, value)
  emit(CHANGE_EVENT, value)
  ;(evt.target as HTMLInputElement).checked = value === props.modelValue
}

const aliasProps = computed(() => ({ ...defaultProps, ...props.props }))

//FIXME: remove this when vue >=3.3
const intoAny = (item: any) => item

const getValue = (item: Option) => {
  return isObject(item) ? item[aliasProps.value.value] : item
}

const getLabel = (item: Option) => {
  return isObject(item) ? item[aliasProps.value.label] : item
}

const getDisabled = (item: Option | undefined) => {
  return !!(
    _disabled.value ||
    (isObject(item) ? item[aliasProps.value.disabled] : false)
  )
}

const getSelected = (item: Option) => {
  return props.modelValue === getValue(item)
}

const getOption = (value: any) => {
  return props.options.find((item) => getValue(item) === value)
}

const getItemCls = (item: Option) => {
  return [
    ns.e('item'),
    ns.is('selected', getSelected(item)),
Repository: element-plus/element-plus
File: packages/components/segmented/src/segmented.vue (Vue)

Code before:

const props = defineProps(segmentedProps)
const emit = defineEmits(segmentedEmits)

const ns = useNamespace('segmented')
const segmentedId = useId()
const segmentedSize = useFormSize()
const _disabled = useFormDisabled()
const { formItem } = useFormItem()
const { inputId, isLabeledByFormItem } = useFormItemInputId(props, {
  formItemContext: formItem,
})

const segmentedRef = ref<HTMLElement | null>(null)
const activeElement = useActiveElement()

const state = reactive({
  isInit: false,
  width: 0,
  height: 0,
  translateX: 0,
  translateY: 0,
  focusVisible: false,
})

const handleChange = (item: Option, evt: Event) => {
  const value = getValue(item)
  emit(UPDATE_MODEL_EVENT, value)
  emit(CHANGE_EVENT, value)

  if (!getSelected(item)) {
    evt.target && ((evt.target as HTMLInputElement).checked = false)
    if (!segmentedRef.value) return
    const selectedItemInput = segmentedRef.value.querySelector(
      '.is-selected input'
    ) as HTMLInputElement
    if (selectedItemInput) {
      selectedItemInput.checked = true
    }
  }
}

const getValue = (item: Option) => {
  return isObject(item) ? item.value : item
}

const getLabel = (item: Option) => {
  return isObject(item) ? item.label : item
}

const getDisabled = (item: Option | undefined) => {

Review comment (suggestion):
```suggestion
const handleChange = (evt: Event, item: Option) => {
```

Code after:
})

const props = defineProps(segmentedProps)
const emit = defineEmits(segmentedEmits)

const ns = useNamespace('segmented')
const segmentedId = useId()
const segmentedSize = useFormSize()
const _disabled = useFormDisabled()
const { formItem } = useFormItem()
const { inputId, isLabeledByFormItem } = useFormItemInputId(props, {
  formItemContext: formItem,
})

const segmentedRef = ref<HTMLElement | null>(null)
const activeElement = useActiveElement()

const state = reactive({
  isInit: false,
  width: 0,
  height: 0,
  translateX: 0,
  translateY: 0,
  focusVisible: false,
})

const handleChange = (evt: Event, item: Option) => {
  const value = getValue(item)
  emit(UPDATE_MODEL_EVENT, value)
  emit(CHANGE_EVENT, value)
  ;(evt.target as HTMLInputElement).checked = value === props.modelValue
}

const aliasProps = computed(() => ({ ...defaultProps, ...props.props }))

//FIXME: remove this when vue >=3.3
const intoAny = (item: any) => item

const getValue = (item: Option) => {
  return isObject(item) ? item[aliasProps.value.value] : item
}

const getLabel = (item: Option) => {
  return isObject(item) ? item[aliasProps.value.label] : item
}

const getDisabled = (item: Option | undefined) => {
  return !!(
    _disabled.value ||
    (isObject(item) ? item[aliasProps.value.disabled] : false)
  )
Repository: element-plus/element-plus
File: packages/components/autocomplete/src/autocomplete.vue (Vue)

Code before:
const handleKeyEnter = async () => {
  if (
    suggestionVisible.value &&
    highlightedIndex.value >= 0 &&
    highlightedIndex.value < suggestions.value.length
  ) {
    handleSelect(suggestions.value[highlightedIndex.value])
  } else {
    if (props.selectWhenUnmatched) {
      emit('select', { value: props.modelValue })
      suggestions.value = []
      highlightedIndex.value = -1
    }
    activated.value = true
    debouncedGetData(String(props.modelValue))
  }
}

const handleKeyEscape = (evt: Event) => {
  if (suggestionVisible.value) {
    evt.preventDefault()
    evt.stopPropagation()
    close()
  }
}

const close = () => {
  activated.value = false
}

const focus = () => {
  inputRef.value?.focus()
}

const blur = () => {
  inputRef.value?.blur()
}

const handleSelect = async (item: any) => {
  emit(INPUT_EVENT, item[props.valueKey])
  emit(UPDATE_MODEL_EVENT, item[props.valueKey])
  emit('select', item)
  suggestions.value = []
  highlightedIndex.value = -1
}

const highlight = (index: number) => {
  if (!suggestionVisible.value || loading.value) return

  if (index < 0) {
    highlightedIndex.value = -1

Review comment (suggestion):
LGTM.

I am considering whether to avoid triggering dropdown panel  when `isComposing` is `true`. When entering a Chinese character that does not exist in a list, using the Enter key to select the text will cause the dropdown panel to flash. WDYT

Code after:
      ignoreFocusEvent = true
      return
    }
    activated.value && close()
    emit('blur', evt)
  })
}

const handleClear = () => {
  activated.value = false
  emit(UPDATE_MODEL_EVENT, '')
  emit('clear')
}

const handleKeyEnter = async () => {
  if (inputRef.value?.isComposing) {
    return
  }

  if (
    suggestionVisible.value &&
    highlightedIndex.value >= 0 &&
    highlightedIndex.value < suggestions.value.length
  ) {
    handleSelect(suggestions.value[highlightedIndex.value])
  } else {
    if (props.selectWhenUnmatched) {
      emit('select', { value: props.modelValue })
      suggestions.value = []
      highlightedIndex.value = -1
    }
    activated.value = true
    debouncedGetData(String(props.modelValue))
  }
}

const handleKeyEscape = (evt: Event) => {
  if (suggestionVisible.value) {
    evt.preventDefault()
    evt.stopPropagation()
    close()
  }
}

const close = () => {
  activated.value = false
}

const focus = () => {
  inputRef.value?.focus()
}
Repository: element-plus/element-plus
File: packages/components/steps/src/item.vue (Vue)

Code before:
  uid: number
  currentStatus: string
  setIndex: (val: number) => void
  calcProgress: (status: string) => void
}

export interface IStepsInject {
  props: IStepsProps
  steps: Ref<StepItemState[]>
  addStep: (item: StepItemState) => void
  removeStep: (uid: number) => void
}

defineOptions({
  name: 'ElStep',
})

const props = defineProps(stepProps)
const ns = useNamespace('step')
const index = ref(-1)
const lineStyle = ref({})
const internalStatus = ref('')
const parent = inject('ElSteps') as IStepsInject
const currentInstance = getCurrentInstance()
const stepDiff = ref(0)
const beforeActive = ref(0)

onMounted(() => {
  watch(
    [
      () => parent.props.active,
      () => parent.props.processStatus,
      () => parent.props.finishStatus,
    ],
    ([active], [oldActive]) => {
      beforeActive.value = oldActive || 0
      stepDiff.value = active - beforeActive.value

      updateStatus(active)
    },
    { immediate: true }
  )
})

onBeforeUnmount(() => {
  parent.removeStep(stepItemState.uid)
})

const currentStatus = computed(() => {
  return props.status || internalStatus.value
})

Review comment (suggestion):
```suggestion
let stepDiff = 0
let beforeActive = 0
```

It should be possible to dispense with ref here

Code after:
  props: StepsProps
  steps: Ref<StepItemState[]>
  addStep: (item: StepItemState) => void
  removeStep: (item: StepItemState) => void
}

defineOptions({
  name: 'ElStep',
})

const props = defineProps(stepProps)
const ns = useNamespace('step')
const index = ref(-1)
const lineStyle = ref({})
const internalStatus = ref('')
const parent = inject(STEPS_INJECTION_KEY) as IStepsInject
const currentInstance = getCurrentInstance()!
let stepDiff = 0
let beforeActive = 0

onMounted(() => {
  watch(
    [
      () => parent.props.active,
      () => parent.props.processStatus,
      () => parent.props.finishStatus,
    ],
    ([active], [oldActive]) => {
      beforeActive = oldActive || 0
      stepDiff = active - beforeActive

      updateStatus(active)
    },
    { immediate: true }
  )
})

const currentStatus = computed(() => {
  return props.status || internalStatus.value
})

const prevInternalStatus = computed(() => {
  const prevStep = parent.steps.value[index.value - 1]
  return prevStep ? prevStep.internalStatus.value : 'wait'
})

const isCenter = computed(() => {
  return parent.props.alignCenter
})

const isVertical = computed(() => {
Repository: element-plus/element-plus
File: packages/components/tree/src/tree-node.vue (Vue)

Code before:
    ElCollapseTransition,
    ElCheckbox,
    NodeContent,
    ElIcon,
    Loading,
  },
  props: {
    node: {
      type: Node,
      default: () => ({}),
    },
    props: {
      type: Object as PropType<TreeOptionProps>,
      default: () => ({}),
    },
    accordion: Boolean,
    renderContent: Function,
    renderAfterExpand: Boolean,
    showCheckbox: {
      type: Boolean,
      default: false,
    },
    selectFocus: {
      type: Function,
      default: null,
    },
  },
  emits: ['node-expand'],
  setup(props, ctx) {
    const ns = useNamespace('tree')
    const { broadcastExpanded } = useNodeExpandEventBroadcast(props)
    const tree = inject<RootTreeType>('RootTree')!
    const expanded = ref(false)
    const childNodeRendered = ref(false)
    const oldChecked = ref<boolean>()
    const oldIndeterminate = ref<boolean>()
    const node$ = ref<HTMLElement>()
    const dragEvents = inject(dragEventsKey)!
    const instance = getCurrentInstance()

    provide('NodeInstance', instance)
    if (!tree) {
      debugWarn('Tree', "Can not find node's tree.")
    }

    if (props.node.expanded) {
      expanded.value = true
      childNodeRendered.value = true
    }

    const childrenKey = tree.props.props['children'] || 'children'

Review comment (suggestion):
The new prop doesn't seem to belong in the tree, try using the `node-expand/node-collapse` events in the TreeSelect component instead.

Code after:
  name: 'ElTreeNode',
  components: {
    ElCollapseTransition,
    ElCheckbox,
    NodeContent,
    ElIcon,
    Loading,
  },
  props: {
    node: {
      type: Node,
      default: () => ({}),
    },
    props: {
      type: Object as PropType<TreeOptionProps>,
      default: () => ({}),
    },
    accordion: Boolean,
    renderContent: Function,
    renderAfterExpand: Boolean,
    showCheckbox: Boolean,
  },
  emits: ['node-expand'],
  setup(props, ctx) {
    const ns = useNamespace('tree')
    const { broadcastExpanded } = useNodeExpandEventBroadcast(props)
    const tree = inject<RootTreeType>(ROOT_TREE_INJECTION_KEY)!
    const expanded = ref(false)
    const childNodeRendered = ref(false)
    const oldChecked = ref<boolean>()
    const oldIndeterminate = ref<boolean>()
    const node$ = ref<HTMLElement>()
    const dragEvents = inject(dragEventsKey)!
    const instance = getCurrentInstance()

    provide(NODE_INSTANCE_INJECTION_KEY, instance)
    if (!tree) {
      debugWarn('Tree', "Can not find node's tree.")
    }

    if (props.node.expanded) {
      expanded.value = true
      childNodeRendered.value = true
    }

    const childrenKey = tree.props.props['children'] || 'children'
    watch(
      () => {
        const children = props.node.data?.[childrenKey]
        return children && [...children]
      },
Repository: element-plus/element-plus
File: packages/components/tree-select/src/select.ts (TypeScript)

Code before:
) => {
  const ns = useNamespace('tree-select')

  // update tree data when use filterMethod/remoteMethod
  watch(
    () => props.data,
    () => {
      if (props.filterable) {
        nextTick(() => {
          // let tree node expand only, same with tree filter
          tree.value?.filter(select.value?.states.inputValue)
        })
      }
    },
    { flush: 'post' }
  )

  const focusLastNode = (listNode) => {
    const lastNode = listNode.at(-1)
    if (lastNode.expanded && lastNode.childNodes.at(-1)) {
      focusLastNode([lastNode.childNodes.at(-1)])
    } else {
      const el = tree.value.el$?.querySelector(
        `[data-key="${listNode.at(-1).key}"]`
      )
      el?.focus()
      return
    }
  }

  onMounted(() => {
    useEventListener(
      () => select.value?.$el,
      'keydown',
      async (evt) => {
        const { dropdownMenuVisible } = select.value!
        if (
          (EVENT_CODE.down === evt.key || EVENT_CODE.up === evt.key) &&
          dropdownMenuVisible
        ) {
          await nextTick()
          // wait navigateOption to finish
          setTimeout(() => {
            if (EVENT_CODE.up === evt.key) {
              const listNode = tree.value.store.root.childNodes
              focusLastNode(listNode)
              return
            }
            // el-select-dropdown__item => el-tree-node__content => el-tree-node__content
            select.value.optionsArray[
              select.value.states.hoveringIndex

Review comment (suggestion):
```suggestion
      el?.focus({ preventScroll: true })
```

Code after:
  }
) => {
  const ns = useNamespace('tree-select')

  // update tree data when use filterMethod/remoteMethod
  watch(
    () => props.data,
    () => {
      if (props.filterable) {
        nextTick(() => {
          // let tree node expand only, same with tree filter
          tree.value?.filter(select.value?.states.inputValue)
        })
      }
    },
    { flush: 'post' }
  )

  const focusLastNode = (listNode) => {
    const lastNode = listNode.at(-1)
    if (lastNode.expanded && lastNode.childNodes.at(-1)) {
      focusLastNode([lastNode.childNodes.at(-1)])
    } else {
      const el = tree.value.el$?.querySelector(
        `[data-key="${listNode.at(-1).key}"]`
      )
      el?.focus({ preventScroll: true })
      return
    }
  }

  onMounted(() => {
    useEventListener(
      () => select.value?.$el,
      'keydown',
      async (evt) => {
        const code = getEventCode(evt)
        const { dropdownMenuVisible } = select.value!
        if (
          [EVENT_CODE.down, EVENT_CODE.up].includes(code) &&
          dropdownMenuVisible
        ) {
          await nextTick()
          // wait navigateOption to finish
          setTimeout(() => {
            if (EVENT_CODE.up === code) {
              const listNode = tree.value.store.root.childNodes
              focusLastNode(listNode)
              return
            }
            // el-select-dropdown__item => el-tree-node__content => el-tree-node__content