ronantakizawa/codereview-bench huggingface dataset:
Repository: element-plus/element-plus
File: packages/components/table/src/table-column/render-helper.ts (TypeScript)

Code before:
      }
    }
  }
  const setColumnRenders = (column: TableColumnCtx<T>) => {
    // renderHeader 属性不推荐使用。
    //@ts-expect-error
    if (props.renderHeader) {
      debugWarn(
        'TableColumn',
        'Comparing to render-header, scoped-slot header is easier to use. We recommend users to use scoped-slot header.'
      )
    } else if (column.type !== 'selection') {
      column.renderHeader = (scope) => {
        // help render
        instance.columnConfig.value['label']

        if (slots.header) {
          const slotResult = slots.header(scope)
          // Manual valid check to support v-if fallback
          // and bypass renderSlot to support HMR
          if (ensureValidVNode(slotResult)) {
            return h(Fragment, slotResult)
          }
        }

        return h('span', column.label)
      }
    }

    if (slots['filter-icon']) {
      column.renderFilterIcon = (scope) => {
        return renderSlot(slots, 'filter-icon', scope)
      }
    }

    if (slots.expand) {
      column.renderExpand = (scope) => {
        return renderSlot(slots, 'expand', scope)
      }
    }

    let originRenderCell = column.renderCell
    // TODO: 这里的实现调整
    if (column.type === 'expand') {
      // 对于展开行，renderCell 不允许配置的。在上一步中已经设置过，这里需要简单封装一下。
      column.renderCell = (data) =>
        h(
          'div',
          {
            class: 'cell',
          },

Review comment (suggestion):
```suggestion
        return column.label
```
We should keep the tag structure consistent with the one we rendered earlier.

Code after:
        item.vParent = instance
      }
    }
  }
  const setColumnRenders = (column: TableColumnCtx<T>) => {
    // renderHeader 属性不推荐使用。
    //@ts-expect-error
    if (props.renderHeader) {
      debugWarn(
        'TableColumn',
        'Comparing to render-header, scoped-slot header is easier to use. We recommend users to use scoped-slot header.'
      )
    } else if (column.type !== 'selection') {
      column.renderHeader = (scope) => {
        // help render
        instance.columnConfig.value['label']

        if (slots.header) {
          const slotResult = slots.header(scope)
          // Manual valid check to support v-if fallback
          // and bypass renderSlot to support HMR
          if (ensureValidVNode(slotResult)) {
            return h(Fragment, slotResult)
          }
        }

        return createTextVNode(column.label)
      }
    }

    if (slots['filter-icon']) {
      column.renderFilterIcon = (scope) => {
        return renderSlot(slots, 'filter-icon', scope)
      }
    }

    if (slots.expand) {
      column.renderExpand = (scope) => {
        return renderSlot(slots, 'expand', scope)
      }
    }

    let originRenderCell = column.renderCell
    // TODO: 这里的实现调整
    if (column.type === 'expand') {
      // 对于展开行，renderCell 不允许配置的。在上一步中已经设置过，这里需要简单封装一下。
      column.renderCell = (data) =>
        h(
          'div',
          {
            class: 'cell',
Repository: element-plus/element-plus
File: packages/components/splitter/src/splitter.ts (TypeScript)

Code before:

import type { ExtractPropTypes, __ExtractPublicPropTypes } from 'vue'
import type Splitter from './splitter.vue'

export const splitterProps = buildProps({
  layout: {
    type: String,
    default: 'horizontal',
    values: ['horizontal', 'vertical'] as const,
  },
  lazy: Boolean,
} as const)

export type SplitterProps = ExtractPropTypes<typeof splitterProps>
export type SplitterPropsPublic = __ExtractPublicPropTypes<typeof splitterProps>
export type SplitterInstance = InstanceType<typeof Splitter> & unknown

/* eslint-disable @typescript-eslint/no-unused-vars */
export const splitterEmits = {
  resizeStart: (index: number, sizes: number[]) => true,
  resize: (index: number, sizes: number[]) => true,
  resizeEnd: (index: number, sizes: number[]) => true,
  collapse: (index: number, type: 'start' | 'end', sizes: number[]) => true,
}

export type SplitterEmits = typeof splitterEmits


Review comment (suggestion):
```suggestion
/* eslint-enable @typescript-eslint/no-unused-vars */

```

Code after:

import type { ExtractPropTypes, __ExtractPublicPropTypes } from 'vue'
import type Splitter from './splitter.vue'

export const splitterProps = buildProps({
  layout: {
    type: String,
    default: 'horizontal',
    values: ['horizontal', 'vertical'] as const,
  },
  lazy: Boolean,
} as const)

export type SplitterProps = ExtractPropTypes<typeof splitterProps>
export type SplitterPropsPublic = __ExtractPublicPropTypes<typeof splitterProps>
export type SplitterInstance = InstanceType<typeof Splitter> & unknown

/* eslint-disable @typescript-eslint/no-unused-vars */
export const splitterEmits = {
  resizeStart: (index: number, sizes: number[]) => true,
  resize: (index: number, sizes: number[]) => true,
  resizeEnd: (index: number, sizes: number[]) => true,
  collapse: (index: number, type: 'start' | 'end', sizes: number[]) => true,
}
/* eslint-enable @typescript-eslint/no-unused-vars */

export type SplitterEmits = typeof splitterEmits
Repository: element-plus/element-plus
File: docs/examples/timeline/mode.vue (Vue)

Code before:
<template>
  <el-radio-group v-model="mode">
    <el-radio-button label="left" value="left" />
    <el-radio-button label="alternate" value="alternate" />
    <el-radio-button label="right" value="right" />
  </el-radio-group>

  <el-timeline class="mt-4" style="max-width: 600px" :mode="mode">
    <el-timeline-item
      v-for="(activity, index) in activities"
      :key="index"
      :timestamp="activity.timestamp"
    >
      {{ activity.content }}
    </el-timeline-item>
  </el-timeline>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

const activities = [
  {
    content: 'Event start',
    timestamp: '2018-04-15',
  },
  {
    content: 'Approved',
    timestamp: '2018-04-13',
  },
  {
    content: 'Success',
    timestamp: '2018-04-11',

Review comment (suggestion):
```suggestion
  <el-timeline class="mt-4" :mode="mode">
```

This style can be removed to provide a clearer reference.

Code after:
<template>
  <el-radio-group v-model="mode">
    <el-radio-button label="start" value="start" />
    <el-radio-button label="alternate" value="alternate" />
    <el-radio-button label="alternate-reverse" value="alternate-reverse" />
    <el-radio-button label="end" value="end" />
  </el-radio-group>

  <el-timeline class="mt-4" :mode="mode">
    <el-timeline-item
      v-for="(activity, index) in activities"
      :key="index"
      :timestamp="activity.timestamp"
    >
      {{ activity.content }}
    </el-timeline-item>
  </el-timeline>
</template>

<script lang="ts" setup>
import { ref } from 'vue'

import type { TimelineProps } from 'element-plus'

const activities = [
  {
    content: 'Event start',
    timestamp: '2018-04-15',
  },
  {
    content: 'Approved',
    timestamp: '2018-04-13',
  },
Repository: element-plus/element-plus
File: packages/components/table-v2/src/table-grid.tsx (TypeScript)

Code before:
      header$?.scrollToLeft(leftOrOptions)
      scrollLeft.value = leftOrOptions
      body$?.scrollTo({
        scrollLeft: leftOrOptions,
        scrollTop: top,
      })
    }
  }

  function scrollToTop(scrollTop: number) {
    unref(bodyRef)?.scrollTo({
      scrollTop,
    })
  }

  function scrollToRow(row: number, strategy: ScrollStrategy) {
    const body = unref(bodyRef)
    if (!body) return

    const prevScrollLeft = scrollLeft.value

    body.scrollToItem(row, 1, strategy)

    scrollTo({
      scrollLeft: prevScrollLeft,
    })
  }

  function forceUpdate() {
    unref(bodyRef)?.$forceUpdate()
    unref(headerRef)?.$forceUpdate()
  }

  watch(
    () => props.bodyWidth,
    () => {
      if (isNumber(props.estimatedRowHeight))
        bodyRef.value?.resetAfter({ columnIndex: 0 }, false)
    }
  )

  return {
    bodyRef,
    forceUpdate,
    fixedRowHeight,
    gridHeight,
    hasHeader,
    headerHeight,
    headerRef,
    totalHeight,


Review comment (suggestion):
I'm thinking of a solution where `columnIndex` can be approximately known in advance. With `props.bodyWith` and `props.columns` it can probably work 🤔 .

Code after:
      header$?.scrollToLeft(leftOrOptions)
      scrollLeft.value = leftOrOptions
      body$?.scrollTo({
        scrollLeft: leftOrOptions,
        scrollTop: top,
      })
    }
  }

  function scrollToTop(scrollTop: number) {
    unref(bodyRef)?.scrollTo({
      scrollTop,
    })
  }

  function scrollToRow(row: number, strategy: ScrollStrategy) {
    const body = unref(bodyRef)
    if (!body) return

    const prevScrollLeft = scrollLeft.value

    body.scrollToItem(row, 0, strategy)

    if (prevScrollLeft) {
      scrollTo({
        scrollLeft: prevScrollLeft,
      })
    }
  }

  function forceUpdate() {
    unref(bodyRef)?.$forceUpdate()
    unref(headerRef)?.$forceUpdate()
  }

  watch(
    () => props.bodyWidth,
    () => {
      if (isNumber(props.estimatedRowHeight))
        bodyRef.value?.resetAfter({ columnIndex: 0 }, false)
    }
  )

  return {
    bodyRef,
    forceUpdate,
    fixedRowHeight,
    gridHeight,
    hasHeader,
    headerHeight,
    headerRef,
Repository: element-plus/element-plus
File: packages/components/table-v2/src/table-grid.tsx (TypeScript)

Code before:
      body$?.scrollTo(leftOrOptions)
    } else {
      header$?.scrollToLeft(leftOrOptions)
      scrollLeft.value = leftOrOptions
      body$?.scrollTo({
        scrollLeft: leftOrOptions,
        scrollTop: top,
      })
    }
  }

  function scrollToTop(scrollTop: number) {
    unref(bodyRef)?.scrollTo({
      scrollTop,
    })
  }

  function scrollToRow(row: number, strategy: ScrollStrategy) {
    const body = unref(bodyRef)
    if (!body) return

    const prevScrollLeft = scrollLeft.value

    body.scrollToItem(row, 1, strategy)

    scrollTo({
      scrollLeft: prevScrollLeft,
    })
  }

  function forceUpdate() {
    unref(bodyRef)?.$forceUpdate()
    unref(headerRef)?.$forceUpdate()
  }

  watch(
    () => props.bodyWidth,
    () => {
      if (isNumber(props.estimatedRowHeight))
        bodyRef.value?.resetAfter({ columnIndex: 0 }, false)
    }
  )

  return {
    bodyRef,
    forceUpdate,
    fixedRowHeight,
    gridHeight,
    hasHeader,
    headerHeight,
    headerRef,

Review comment (suggestion):
```suggestion
  body.scrollToItem(row, 0, strategy)
  if (scrollLeft.value) {
    scrollTo({ scrollLeft: scrollLeft.value })
  }
```

Code after:
      body$?.scrollTo(leftOrOptions)
    } else {
      header$?.scrollToLeft(leftOrOptions)
      scrollLeft.value = leftOrOptions
      body$?.scrollTo({
        scrollLeft: leftOrOptions,
        scrollTop: top,
      })
    }
  }

  function scrollToTop(scrollTop: number) {
    unref(bodyRef)?.scrollTo({
      scrollTop,
    })
  }

  function scrollToRow(row: number, strategy: ScrollStrategy) {
    const body = unref(bodyRef)
    if (!body) return

    const prevScrollLeft = scrollLeft.value

    body.scrollToItem(row, 0, strategy)

    if (prevScrollLeft) {
      scrollTo({
        scrollLeft: prevScrollLeft,
      })
    }
  }

  function forceUpdate() {
    unref(bodyRef)?.$forceUpdate()
    unref(headerRef)?.$forceUpdate()
  }

  watch(
    () => props.bodyWidth,
    () => {
      if (isNumber(props.estimatedRowHeight))
        bodyRef.value?.resetAfter({ columnIndex: 0 }, false)
    }
  )

  return {
    bodyRef,
    forceUpdate,
    fixedRowHeight,
    gridHeight,
    hasHeader,
Repository: element-plus/element-plus
File: packages/components/form/src/form.vue (Vue)

Code before:
  fields.push(field)
}

const removeField: FormContext['removeField'] = (field) => {
  if (field.prop) {
    fields.splice(fields.indexOf(field), 1)
  }
}

const setInitialValues: FormContext['setInitialValues'] = (initModel) => {
  if (!props.model) {
    debugWarn(COMPONENT_NAME, 'model is required for setInitialValues to work.')
    return
  }
  if (!initModel) {
    debugWarn(
      COMPONENT_NAME,
      'initModel is required for setInitialValues to work.'
    )
    return
  }
  fields.forEach((field) => {
    if (field.prop) {
      const initValue = getProp(initModel, field.prop).value
      // Only update fields that exist in initModel to avoid resetting undefined fields to undefined
      if (initValue !== undefined) {
        field.setInitialValue(initValue)
      }
    }
  })
}

const resetFields: FormContext['resetFields'] = (properties = []) => {
  if (!props.model) {
    debugWarn(COMPONENT_NAME, 'model is required for resetFields to work.')
    return
  }
  filterFields(fields, properties).forEach((field) => field.resetField())
}

const clearValidate: FormContext['clearValidate'] = (props = []) => {
  filterFields(fields, props).forEach((field) => field.clearValidate())
}

const isValidatable = computed(() => {
  const hasModel = !!props.model
  if (!hasModel) {
    debugWarn(COMPONENT_NAME, 'model is required for validate to work.')
  }
  return hasModel
})

Review comment (suggestion):
Is there also a scenario where the value of a certain field needs to be set to `undefined`? 🤔 

Code after:
const addField: FormContext['addField'] = (field) => {
  fields.push(field)
}

const removeField: FormContext['removeField'] = (field) => {
  if (field.prop) {
    fields.splice(fields.indexOf(field), 1)
  }
}

const setInitialValues: FormContext['setInitialValues'] = (
  initModel: Partial<typeof props.model>
) => {
  if (!props.model) {
    debugWarn(COMPONENT_NAME, 'model is required for setInitialValues to work.')
    return
  }
  if (!initModel) {
    debugWarn(
      COMPONENT_NAME,
      'initModel is required for setInitialValues to work.'
    )
    return
  }
  fields.forEach((field) => {
    if (field.prop) {
      // Check if the property path actually exists in initModel
      // This allows setting undefined/null values while skipping non-existent properties
      if (has(initModel, field.prop)) {
        const initValue = getProp(initModel, field.prop).value
        field.setInitialValue(initValue)
      }
    }
  })
}

const resetFields: FormContext['resetFields'] = (properties = []) => {
  if (!props.model) {
    debugWarn(COMPONENT_NAME, 'model is required for resetFields to work.')
    return
  }
  filterFields(fields, properties).forEach((field) => field.resetField())
}

const clearValidate: FormContext['clearValidate'] = (props = []) => {
  filterFields(fields, props).forEach((field) => field.clearValidate())
}

const isValidatable = computed(() => {
  const hasModel = !!props.model
  if (!hasModel) {
Repository: element-plus/element-plus
File: packages/components/form/src/form.vue (Vue)

Code before:
   */
  validateField,
  /**
   * @description Reset specified fields and remove validation result.
   */
  resetFields,
  /**
   * @description Clear validation message for specified fields.
   */
  clearValidate,
  /**
   * @description Scroll to the specified fields.
   */
  scrollToField,
  /**
   * @description Get a field context.
   */
  getField,
  /**
   * @description All fields context.
   */
  fields,
  /**
   * @description Set initial values for form fields. These values will be used when calling resetFields(). Useful for forms populated with API data after mount.
   * @param initModel Partial or complete model containing initial values to set
   */
  setInitialValues,
})
</script>


Review comment (suggestion):
You can avoid `@param` section, and please make the comment as the same of the documentation description for el-form and el-form-item.

Code after:
   */
  validateField,
  /**
   * @description Reset specified fields and remove validation result.
   */
  resetFields,
  /**
   * @description Clear validation message for specified fields.
   */
  clearValidate,
  /**
   * @description Scroll to the specified fields.
   */
  scrollToField,
  /**
   * @description Get a field context.
   */
  getField,
  /**
   * @description All fields context.
   */
  fields,
  /**
   * @description Set initial values for form fields. When `resetFields` is called, fields will reset to these values. Only fields present in `initModel` will be updated.
   */
  setInitialValues,
})
</script>
Repository: element-plus/element-plus
File: packages/components/date-picker/src/date-picker.tsx (TypeScript)

Code before:

    expose(refProps)

    const onModelValueUpdated = (val: SingleOrRange<DateModelType> | null) => {
      emit(UPDATE_MODEL_EVENT, val)
    }

    return () => {
      // since props always have all defined keys on it, {format, ...props} will always overwrite format
      // pick props.format or provide default value here before spreading
      const format =
        props.format ??
        (DEFAULT_FORMATS_DATEPICKER[props.type] || DEFAULT_FORMATS_DATE)

      return (
        <CommonPicker
          {...props}
          format={format}
          type={props.type}
          ref={commonPicker}
          onUpdate:modelValue={onModelValueUpdated}
        >
          {{
            default: (scopedProps: /**FIXME: remove any type */ any) => (
              <ElDatePickerPanel
                disabled={disabled.value}
                border={false}
                {...scopedProps}
              >
                {slots}
              </ElDatePickerPanel>
            ),
            'range-separator': slots['range-separator'],
          }}
        </CommonPicker>
      )
    }
  },
})


Review comment (suggestion):
I think simply passing through the `disabled` prop is sufficient, since each type of panel component already uses `useFormDisabled` internally. The current handling here seems a bit redundant.
What do you think? @btea @jeff-fe 

See
https://github.com/element-plus/element-plus/blob/d215c1c1455ce3932fb50b32813b7a98d3f73169/packages/components/date-picker-panel/src/date-picker-panel.tsx#L72-L75

https://github.com/element-plus/element-plus/blob/dev/packages/components/date-picker-panel/src/panel-utils.ts

```suggestion
                disabled={props.disabled}
```

Code after:

    const onModelValueUpdated = (val: SingleOrRange<DateModelType> | null) => {
      emit(UPDATE_MODEL_EVENT, val)
    }

    return () => {
      // since props always have all defined keys on it, {format, ...props} will always overwrite format
      // pick props.format or provide default value here before spreading
      const format =
        props.format ??
        (DEFAULT_FORMATS_DATEPICKER[props.type] || DEFAULT_FORMATS_DATE)

      return (
        <CommonPicker
          {...props}
          format={format}
          type={props.type}
          ref={commonPicker}
          onUpdate:modelValue={onModelValueUpdated}
        >
          {{
            default: (scopedProps: /**FIXME: remove any type */ any) => (
              <ElDatePickerPanel
                disabled={props.disabled}
                border={false}
                {...scopedProps}
              >
                {slots}
              </ElDatePickerPanel>
            ),
            'range-separator': slots['range-separator'],
          }}
        </CommonPicker>
      )
    }
  },
})
Repository: element-plus/element-plus
File: packages/components/button/__tests__/button.test.tsx (TypeScript)

Code before:
    expect(wrapper.find('.el-button span').text()).toBe('中文')
    expect(wrapper.find('.el-button span').classes()).toContain(
      'el-button__text--expand'
    )
  })

  it('should use props of form', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form size="large" disabled>
          <Button
            v-slots={{
              default: () => AXIOM,
            }}
          />
        </Form>
      ),
    })
    const btn = wrapper.findComponent(Button)
    expect(btn.classes()).toContain('el-button--large')
    expect(btn.classes()).toContain('is-disabled')
    await btn.trigger('click')
    expect(btn.emitted('click')).toBeUndefined()
  })

  it('form disabled but button is not disabled', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form disabled>
          <Button
            disabled={false}
            v-slots={{
              default: () => AXIOM,
            }}
          />
        </Form>
      ),
    })
    const btn = wrapper.findComponent(Button)
    expect(btn.classes()).not.toContain('is-disabled')
    await btn.trigger('click')
    expect(btn.emitted('click')).toBeDefined()
  })

  it('should use size of form-item', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form size="large" disabled>
          <Form.FormItem size="small">
            <Button
              v-slots={{

Review comment (suggestion):
```suggestion
  it('The disabled state of a component has higher priority than that of a form', async () => {
```
This description makes things clearer.

Code after:
    expect(wrapper.find('.el-button span').text()).toBe('中文')
    expect(wrapper.find('.el-button span').classes()).toContain(
      'el-button__text--expand'
    )
  })

  it('should use props of form', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form size="large" disabled>
          <Button
            v-slots={{
              default: () => AXIOM,
            }}
          />
        </Form>
      ),
    })
    const btn = wrapper.findComponent(Button)
    expect(btn.classes()).toContain('el-button--large')
    expect(btn.classes()).toContain('is-disabled')
    await btn.trigger('click')
    expect(btn.emitted('click')).toBeUndefined()
  })

  it('The disabled state of a component has higher priority than that of a form', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form disabled>
          <Button
            disabled={false}
            v-slots={{
              default: () => AXIOM,
            }}
          />
        </Form>
      ),
    })
    const btn = wrapper.findComponent(Button)
    expect(btn.classes()).not.toContain('is-disabled')
    await btn.trigger('click')
    expect(btn.emitted('click')).toBeDefined()
  })

  it('should use size of form-item', async () => {
    const wrapper = mount({
      setup: () => () => (
        <Form size="large" disabled>
          <Form.FormItem size="small">
            <Button
              v-slots={{