ronantakizawa/codereview-bench huggingface dataset:
Repository: vuejs/vue-next
File: packages/runtime-core/src/apiOptions.ts (TypeScript)

Code before:
          })
    }
  }
  if (methods) {
    for (const key in methods) {
      renderContext[key] = (methods as MethodOptions)[key].bind(ctx)
    }
  }
  if (watchOptions) {
    for (const key in watchOptions) {
      const raw = watchOptions[key]
      const getter = () => ctx[key]
      if (isString(raw)) {
        const handler = renderContext[raw]
        if (isFunction(handler)) {
          watch(getter, handler as any)
        } else if (__DEV__) {
          warn(`Expected a function as watch handler "${raw}"`)
        }
      } else if (isFunction(raw)) {
        watch(getter, raw.bind(ctx))
      } else if (isObject(raw)) {
        // TODO 2.x compat
        watch(getter, raw.handler.bind(ctx), raw)
      } else if (__DEV__) {
        warn(`Invalid watch options "${key}"`)
      }
    }
  }
  if (provideOptions) {
    const provides = isFunction(provideOptions)
      ? provideOptions.call(ctx)
      : provideOptions
    for (const key in provides) {
      provide(key, provides[key])
    }
  }
  if (injectOptions) {
    if (isArray(injectOptions)) {
      for (let i = 0; i < injectOptions.length; i++) {
        const key = injectOptions[i]
        renderContext[key] = inject(key)
      }
    } else {
      for (const key in injectOptions) {
        const opt = injectOptions[key]
        if (isObject(opt)) {
          renderContext[key] = inject(opt.from, opt.default)
        } else {
          renderContext[key] = inject(opt)
        }

Review comment (suggestion):
```suggestion
        warn(`Invalid watch option: "${key}"`)
```

Code after:
          })
    }
  }
  if (methods) {
    for (const key in methods) {
      renderContext[key] = (methods as MethodOptions)[key].bind(ctx)
    }
  }
  if (watchOptions) {
    for (const key in watchOptions) {
      const raw = watchOptions[key]
      const getter = () => ctx[key]
      if (isString(raw)) {
        const handler = renderContext[raw]
        if (isFunction(handler)) {
          watch(getter, handler as any)
        } else if (__DEV__) {
          warn(`Invalid watch handler specified by key "${raw}"`, handler)
        }
      } else if (isFunction(raw)) {
        watch(getter, raw.bind(ctx))
      } else if (isObject(raw)) {
        // TODO 2.x compat
        watch(getter, raw.handler.bind(ctx), raw)
      } else if (__DEV__) {
        warn(`Invalid watch option: "${key}"`)
      }
    }
  }
  if (provideOptions) {
    const provides = isFunction(provideOptions)
      ? provideOptions.call(ctx)
      : provideOptions
    for (const key in provides) {
      provide(key, provides[key])
    }
  }
  if (injectOptions) {
    if (isArray(injectOptions)) {
      for (let i = 0; i < injectOptions.length; i++) {
        const key = injectOptions[i]
        renderContext[key] = inject(key)
      }
    } else {
      for (const key in injectOptions) {
        const opt = injectOptions[key]
        if (isObject(opt)) {
          renderContext[key] = inject(opt.from, opt.default)
        } else {
          renderContext[key] = inject(opt)
        }
Repository: vuejs/vue-next
File: packages/runtime-core/__tests__/componentProxy.spec.ts (TypeScript)

Code before:
  describe('warnings', () => {
    mockWarn()
    let app: any, appProps: any

    beforeEach(() => {
      const component = {
        render() {}
      }
      const root = nodeOps.createElement('div')
      app = createApp().mount(component, root, appProps)
    })

    test('Attempting to mutate public property', () => {
      try {
        // @ts-ignore
        app.$props = null
      } catch {
        expect(
          'Attempting to mutate public property "$props". ' +
            'Properties starting with $ are reserved and readonly.'
        ).toHaveBeenWarned()
      }
    })

    test('Attempting to mutate prop', () => {
      appProps = { foo: 'foo' }
      try {
        // @ts-ignore
        app.foo = null
      } catch {
        expect(
          'Attempting to mutate prop "foo". Props are readonly.'
        ).toHaveBeenWarned()
      }
    })
  })
})


Review comment (refactor):
Out of scope assignment here makes the tests a bit harder to read in isolation. I'd refactor it into something like a `createTestInstance()` function to be called in each spec.

Code after:
    mockWarn()

    test('Attempting to mutate public property', () => {
      const app = createTestInstance()

      try {
        app.$props = { foo: 'bar' }
      } catch {
        expect(
          'Attempting to mutate public property "$props". ' +
            'Properties starting with $ are reserved and readonly.'
        ).toHaveBeenWarned()
      }
    })

    test('Attempting to mutate prop', () => {
      const app = createTestInstance({ foo: 'foo' })

      try {
        app.foo = 'bar'
      } catch {
        expect(
          'Attempting to mutate prop "foo". Props are readonly.'
        ).toHaveBeenWarned()
      }
    })
  })
})
Repository: vuejs/vue-next
File: packages/dts-test/ref.test-d.ts (TypeScript)

Code before:

describe('reactive in shallow ref', () => {
  const x = shallowRef({
    a: reactive({
      b: ref(0)
    })
  })

  expectType<number>(x.value.a.b)
})

describe('toRef <-> toValue', () => {
  function foo(
    a: MaybeRef<string>,
    b: () => string,
    c: MaybeRefOrGetter<string>,
    d: ComputedRef<string>
  ) {
    const r = toRef(a)
    expectType<Ref<string>>(r)
    // writable
    r.value = 'foo'

    const rb = toRef(b)
    expectType<Readonly<Ref<string>>>(rb)
    // @ts-expect-error ref created from getter shuld be readonly
    rb.value = 'foo'

    const rc = toRef(c)
    expectType<Readonly<Ref<string> | Ref<string>>>(rc)
    // @ts-expect-error ref created from MaybeReadonlyRef shuld be readonly
    rc.value = 'foo'

    const rd = toRef(d)
    expectType<ComputedRef<string>>(rd)
    // @ts-expect-error ref created from computed ref shuld be readonly
    rd.value = 'foo'

    expectType<string>(toValue(a))
    expectType<string>(toValue(b))
    expectType<string>(toValue(c))
    expectType<string>(toValue(d))

    return {
      r: toValue(r),
      rb: toValue(rb),
      rc: toValue(rc),
      rd: toValue(rd)
    }
  }


Review comment (suggestion):
```suggestion
    // @ts-expect-error ref created from getter should be readonly
```

Code after:

describe('reactive in shallow ref', () => {
  const x = shallowRef({
    a: reactive({
      b: ref(0)
    })
  })

  expectType<number>(x.value.a.b)
})

describe('toRef <-> toValue', () => {
  function foo(
    a: MaybeRef<string>,
    b: () => string,
    c: MaybeRefOrGetter<string>,
    d: ComputedRef<string>
  ) {
    const r = toRef(a)
    expectType<Ref<string>>(r)
    // writable
    r.value = 'foo'

    const rb = toRef(b)
    expectType<Readonly<Ref<string>>>(rb)
    // @ts-expect-error ref created from getter should be readonly
    rb.value = 'foo'

    const rc = toRef(c)
    expectType<Readonly<Ref<string> | Ref<string>>>(rc)
    // @ts-expect-error ref created from MaybeReadonlyRef should be readonly
    rc.value = 'foo'

    const rd = toRef(d)
    expectType<ComputedRef<string>>(rd)
    // @ts-expect-error ref created from computed ref should be readonly
    rd.value = 'foo'

    expectType<string>(toValue(a))
    expectType<string>(toValue(b))
    expectType<string>(toValue(c))
    expectType<string>(toValue(d))

    return {
      r: toValue(r),
      rb: toValue(rb),
      rc: toValue(rc),
      rd: toValue(rd)
    }
  }
Repository: vuejs/vue-next
File: packages/reactivity/src/ref.ts (TypeScript)

Code before:
    private readonly _object: T,
    private readonly _key: K,
    private readonly _defaultValue?: T[K]
  ) {}

  get value() {
    const val = this._object[this._key]
    return val === undefined ? (this._defaultValue as T[K]) : val
  }

  set value(newVal) {
    this._object[this._key] = newVal
  }

  get dep(): Dep | undefined {
    return getDepFromReactive(toRaw(this._object), this._key)
  }
}

class GetterRefImpl<T> {
  public readonly __v_isRef = true
  constructor(private readonly _getter: () => T) {}
  get value() {
    return this._getter()
  }
}

export type ToRef<T> = IfAny<T, Ref<T>, [T] extends [Ref] ? T : Ref<T>>

/**
 * Used to normalize values / refs / getters into refs.
 *
 * @example
 * ```js
 * // returns existing refs as-is
 * toRef(existingRef)
 *
 * // creates a ref that calls the getter on .value access
 * toRef(() => props.foo)
 *
 * // creates normal refs from non-function values
 * // requivalent to ref(1)
 * toRef(1)
 * ```
 *
 * Can also be used to create a ref for a property on a source reactive object.
 * The created ref is synced with its source property: mutating the source
 * property will update the ref, and vice-versa.
 *
 * @example
 * ```js

Review comment (suggestion):
Should this also have `__v_isReadonly` set to `true`?

```js
isReadonly(toRef(() => {})) // should be true?
```

Code after:
    return val === undefined ? (this._defaultValue as T[K]) : val
  }

  set value(newVal) {
    this._object[this._key] = newVal
  }

  get dep(): Dep | undefined {
    return getDepFromReactive(toRaw(this._object), this._key)
  }
}

class GetterRefImpl<T> {
  public readonly __v_isRef = true
  public readonly __v_isReadonly = true
  constructor(private readonly _getter: () => T) {}
  get value() {
    return this._getter()
  }
}

export type ToRef<T> = IfAny<T, Ref<T>, [T] extends [Ref] ? T : Ref<T>>

/**
 * Used to normalize values / refs / getters into refs.
 *
 * @example
 * ```js
 * // returns existing refs as-is
 * toRef(existingRef)
 *
 * // creates a ref that calls the getter on .value access
 * toRef(() => props.foo)
 *
 * // creates normal refs from non-function values
 * // equivalent to ref(1)
 * toRef(1)
 * ```
 *
 * Can also be used to create a ref for a property on a source reactive object.
 * The created ref is synced with its source property: mutating the source
 * property will update the ref, and vice-versa.
 *
 * @example
 * ```js
 * const state = reactive({
 *   foo: 1,
 *   bar: 2
 * })
 *
 * const fooRef = toRef(state, 'foo')
Repository: vuejs/vue-next
File: packages/reactivity/src/ref.ts (TypeScript)

Code before:
export function toRef<T>(
  value: T
): T extends () => infer R
  ? Readonly<Ref<R>>
  : T extends Ref
  ? T
  : Ref<UnwrapRef<T>>
export function toRef<T extends object, K extends keyof T>(
  object: T,
  key: K
): ToRef<T[K]>
export function toRef<T extends object, K extends keyof T>(
  object: T,
  key: K,
  defaultValue: T[K]
): ToRef<Exclude<T[K], undefined>>
export function toRef(
  source: Record<string, any> | MaybeRef,
  key?: string,
  defaultValue?: unknown
): Ref {
  if (isRef(source)) {
    return source
  } else if (isFunction(source)) {
    return new GetterRefImpl(source as () => unknown) as any
  } else if (isObject(source) && isString(key)) {
    return propertyToRef(source, key, defaultValue)
  } else {
    return ref(source)
  }
}

function propertyToRef(source: object, key: string, defaultValue?: unknown) {
  const val = (source as any)[key]
  return isRef(val)
    ? val
    : (new ObjectRefImpl(
        source as Record<string, any>,
        key,
        defaultValue
      ) as any)
}

// corner case when use narrows type
// Ex. type RelativePath = string & { __brand: unknown }
// RelativePath extends object -> true
type BaseTypes = string | number | boolean

/**
 * This is a special exported interface for other packages to declare
 * additional types that should bail out for ref unwrapping. For example

Review comment (suggestion):
I think this will potentially cause a breaking change for arrays:

```js
const arr = reactive(['a', 'b'])
const c = toRef(arr, 1)
```

Previously this would have worked, but now `c` will be the whole array.

`Symbol` keys for objects too.

Code after:
  : Ref<UnwrapRef<T>>
export function toRef<T extends object, K extends keyof T>(
  object: T,
  key: K
): ToRef<T[K]>
export function toRef<T extends object, K extends keyof T>(
  object: T,
  key: K,
  defaultValue: T[K]
): ToRef<Exclude<T[K], undefined>>
export function toRef(
  source: Record<string, any> | MaybeRef,
  key?: string,
  defaultValue?: unknown
): Ref {
  if (isRef(source)) {
    return source
  } else if (isFunction(source)) {
    return new GetterRefImpl(source as () => unknown) as any
  } else if (isObject(source) && arguments.length > 1) {
    return propertyToRef(source, key!, defaultValue)
  } else {
    return ref(source)
  }
}

function propertyToRef(source: object, key: string, defaultValue?: unknown) {
  const val = (source as any)[key]
  return isRef(val)
    ? val
    : (new ObjectRefImpl(
        source as Record<string, any>,
        key,
        defaultValue
      ) as any)
}

// corner case when use narrows type
// Ex. type RelativePath = string & { __brand: unknown }
// RelativePath extends object -> true
type BaseTypes = string | number | boolean

/**
 * This is a special exported interface for other packages to declare
 * additional types that should bail out for ref unwrapping. For example
 * \@vue/runtime-dom can declare it like so in its d.ts:
 *
 * ``` ts
 * declare module '@vue/reactivity' {
 *   export interface RefUnwrapBailTypes {
 *     runtimeDOMBailTypes: Node | Window
Repository: vuejs/vue-next
File: packages/runtime-core/src/components/BaseTransition.ts (TypeScript)

Code before:
            earlyRemove,
            delayedLeave,
          ) => {
            const leavingVNodesCache = getLeavingNodesForType(
              state,
              oldInnerChild!,
            )
            leavingVNodesCache[String(oldInnerChild!.key)] = oldInnerChild!
            // early removal callback
            el[leaveCbKey] = () => {
              earlyRemove()
              el[leaveCbKey] = undefined
              delete enterHooks.delayedLeave
              oldInnerChild = undefined
            }
            enterHooks.delayedLeave = () => {
              delayedLeave()
              delete enterHooks.delayedLeave
              oldInnerChild = undefined
            }
          }
        } else {
          oldInnerChild = undefined
        }
      } else if (oldInnerChild) {
        oldInnerChild = undefined
      }

      return child
    }
  },
}

if (__COMPAT__) {
  BaseTransitionImpl.__isBuiltIn = true
}

function findNonCommentChild(children: VNode[]): VNode {
  let child: VNode = children[0]
  if (children.length > 1) {
    let hasFound = false
    // locate first non-comment child
    for (const c of children) {
      if (c.type !== Comment) {
        if (__DEV__ && hasFound) {
          // warn more than one non-comment child
          warn(
            '<transition> can only be used on a single element or component. ' +
              'Use <transition-group> for lists.',
          )
          break

Review comment (suggestion):
```suggestion
      oldInnerChild = undefined
```

Code after:
            delayedLeave,
          ) => {
            const leavingVNodesCache = getLeavingNodesForType(
              state,
              oldInnerChild!,
            )
            leavingVNodesCache[String(oldInnerChild!.key)] = oldInnerChild!
            // early removal callback
            el[leaveCbKey] = () => {
              earlyRemove()
              el[leaveCbKey] = undefined
              delete enterHooks.delayedLeave
              oldInnerChild = undefined
            }
            enterHooks.delayedLeave = () => {
              delayedLeave()
              delete enterHooks.delayedLeave
              oldInnerChild = undefined
            }
          }
        } else {
          oldInnerChild = undefined
        }
      } else if (oldInnerChild) {
        oldInnerChild = undefined
      }

      return child
    }
  },
}

if (__COMPAT__) {
  BaseTransitionImpl.__isBuiltIn = true
}

function findNonCommentChild(children: VNode[]): VNode {
  let child: VNode = children[0]
  if (children.length > 1) {
    let hasFound = false
    // locate first non-comment child
    for (const c of children) {
      if (c.type !== Comment) {
        if (__DEV__ && hasFound) {
          // warn more than one non-comment child
          warn(
            '<transition> can only be used on a single element or component. ' +
              'Use <transition-group> for lists.',
          )
          break
        }
Repository: vuejs/vue-next
File: packages/compiler-ssr/src/transforms/ssrInjectFallthroughAttrs.ts (TypeScript)

Code before:
import {
  ElementTypes,
  type NodeTransform,
  NodeTypes,
  type ParentNode,
  type RootNode,
  type TemplateChildNode,
  createSimpleExpression,
  findDir,
  locStub,
} from '@vue/compiler-dom'

const filterChild = (node: ParentNode) =>
  node.children.filter(
    n => n.type !== NodeTypes.COMMENT && n.type !== NodeTypes.TEXT,
  )

const hasSingleChild = (node: ParentNode): boolean =>
  filterChild(node).length === 1

export const ssrInjectFallthroughAttrs: NodeTransform = (node, context) => {
  // _attrs is provided as a function argument.
  // mark it as a known identifier so that it doesn't get prefixed by
  // transformExpression.
  if (node.type === NodeTypes.ROOT) {
    context.identifiers._attrs = 1
  }

  if (
    node.type === NodeTypes.ELEMENT &&
    node.tagType === ElementTypes.COMPONENT &&
    (node.tag === 'transition' ||
      node.tag === 'Transition' ||
      node.tag === 'KeepAlive' ||
      node.tag === 'keep-alive')
  ) {
    const rootChildren = filterChild(context.root)
    if (rootChildren.length === 1 && rootChildren[0] === node) {
      if (hasSingleChild(node)) {
        injectFallthroughAttrs(node.children[0])

Review comment (bug):
Doesn't this also need to check that the text node only contains whitespace?

e.g. In this playground the attrs are incorrectly inherited in SSR mode because the text node is ignored:

- [Playground](https://deploy-preview-12304--vue-sfc-playground.netlify.app/#__SSR__eNp9UctqwzAQ/JVFFHIJ9qE9BbfQhhxa6IOmR12MvXGUypKQ1q6D8b9XkuvUgZCbdh5idqdnj8YkbYNsxTJXWGEIHFJjHrgStdGW4PW43gtZws7qGhZJ+jcH04KrLB1dXu8HwtrInNBPANnkdHSUeM9ZoaW2K7BYcgap12TpzMCWjFyh1U5UycFp5RP14Zvgq42QaN8NCa0cZyuITOByKfXPS8TINric8GKPxfcF/OC6gHH2YdGhbZGzE0e5rZBGerN9w86/T2Sty0Z69RXyE52WTcg4yp4aVfrYM11M+xzvKlT15TYdoXLTUiFoUA5Rz5m/8PrK6v9xb5O76ONq8FecFRRaPetkq2sECuFDQaVoIwowFtX3cJMTWQeD/8sL0lFx3tPwC5sTuwo=)

Code after:
import {
  ElementTypes,
  type NodeTransform,
  NodeTypes,
  type ParentNode,
  type RootNode,
  type TemplateChildNode,
  createSimpleExpression,
  findDir,
  isCommentOrWhitespace,
  locStub,
} from '@vue/compiler-dom'

const filterChild = (node: ParentNode) =>
  node.children.filter(n => !isCommentOrWhitespace(n))

const hasSingleChild = (node: ParentNode): boolean =>
  filterChild(node).length === 1

export const ssrInjectFallthroughAttrs: NodeTransform = (node, context) => {
  // _attrs is provided as a function argument.
  // mark it as a known identifier so that it doesn't get prefixed by
  // transformExpression.
  if (node.type === NodeTypes.ROOT) {
    context.identifiers._attrs = 1
  }

  if (
    node.type === NodeTypes.ELEMENT &&
    node.tagType === ElementTypes.COMPONENT &&
    (node.tag === 'transition' ||
      node.tag === 'Transition' ||
      node.tag === 'KeepAlive' ||
      node.tag === 'keep-alive')
  ) {
    const rootChildren = filterChild(context.root)
    if (rootChildren.length === 1 && rootChildren[0] === node) {
      if (hasSingleChild(node)) {
        injectFallthroughAttrs(node.children[0])
      }
Repository: vuejs/vue-next
File: packages/reactivity/__tests__/computed.spec.ts (TypeScript)

Code before:
            {
              onClick: () => {
                this.counter++
              },
            },
            'Step',
          ),
          h('p', this.message),
        ]
      },
    })
    const root = nodeOps.createElement('div')
    render(h(Comp), root)
    expect(serializeInner(root)).toBe(`<button>Step</button><p>Step 1</p>`)
    triggerEvent(root.children[1] as TestElement, 'click')
    await nextTick()
    expect(serializeInner(root)).toBe(`<button>Step</button><p>Step 2</p>`)
  })

  test('manual trigger computed', () => {
    const cValue = computed(() => 1)
    triggerRef(cValue)
    expect(cValue.value).toBe(1)
  })

  test('should not recompute when condition changes if computed does not track reactive data', async () => {
    const spy = vi.fn()
    const lookup = computed(() => {
      spy()
      return {
        g: 'foo',
      }
    })

    const unit = ref('')
    const display = computed(() => {
      if (!unit.value) return 'empty'
      // @ts-expect-error
      return lookup.value[unit.value]
    })

    function toggle() {
      unit.value = unit.value === 'g' ? '' : 'g'
    }

    expect(display.value).toBe('empty')
    expect(spy).toHaveBeenCalledTimes(0)

    toggle()
    expect(display.value).toBe('foo')
    expect(spy).toHaveBeenCalledTimes(1)

Review comment (suggestion):
This could be simplified as:

```ts
  test('', () => {
    const spy = vi.fn()
    const c1 = computed(() => spy())

    c1.value
    ref(0).value++ // update globalVersion
    c1.value

    expect(spy).toBeCalledTimes(1)
  })
  ```

Code after:
            {
              onClick: () => {
                this.counter++
              },
            },
            'Step',
          ),
          h('p', this.message),
        ]
      },
    })
    const root = nodeOps.createElement('div')
    render(h(Comp), root)
    expect(serializeInner(root)).toBe(`<button>Step</button><p>Step 1</p>`)
    triggerEvent(root.children[1] as TestElement, 'click')
    await nextTick()
    expect(serializeInner(root)).toBe(`<button>Step</button><p>Step 2</p>`)
  })

  test('manual trigger computed', () => {
    const cValue = computed(() => 1)
    triggerRef(cValue)
    expect(cValue.value).toBe(1)
  })

  test('should not recompute if computed does not track reactive data', async () => {
    const spy = vi.fn()
    const c1 = computed(() => spy())

    c1.value
    ref(0).value++ // update globalVersion
    c1.value

    expect(spy).toBeCalledTimes(1)
  })

  test('computed should remain live after losing all subscribers', () => {
    const state = reactive({ a: 1 })
    const p = computed(() => state.a + 1)
    const { effect: e } = effect(() => p.value)
    e.stop()

    expect(p.value).toBe(2)
    state.a++
    expect(p.value).toBe(3)
  })

  // #11995
  test('computed dep cleanup should not cause property dep to be deleted', () => {
    const toggle = ref(true)
    const state = reactive({ a: 1 })