ronantakizawa/codereview-bench huggingface dataset:
Repository: ianstormtaylor/slate
File: packages/slate/src/interfaces/scrubber.ts (TypeScript)

Code before:
 *
 *    import { Scrubber } from 'slate';
 *    Scrubber.setScrubber((key, val) => {
 *      if (key === 'text') return '...scrubbed...'
 *      return val
 *    });
 *
 */
export const Scrubber: ScrubberInterface = {
  setScrubber(scrubber: Scrubber | undefined): void {
    _scrubber = scrubber
  },

  stringify(value: any): string {
    return JSON.stringify(value, _scrubber)
  },

  /**
   * Helper function to create a Scrubber that scrubs the contents of any field
   * in `fieldNames` with a randomly generated string of the same length.
   *
   * For example:
   *
   *    Scrubber.setScrubber(Scrubber.textRandomizer(['text']));
   */
  textRandomizer(fieldNames: string[]): Scrubber {
    return (key, value) => {
      if (fieldNames.includes(key)) {
        if (typeof value === 'string') {
          return value
            .split('')
            .map(generateRandomCharacter)
            .join('')
        } else {
          return '... scrubbed ...'
        }
      }

      return value
    }
  },
}

const generateRandomCharacter = (): string => {
  const chars =
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLKMNOPQRSTUVWXYZ1234567890'
  return chars.charAt(Math.floor(Math.random() * chars.length))
}


Review comment (refactor):
While I appreciate the addition of a helper function, the way it's implemented as an object property it can't be tree shaken when unused.

I don't think you need the `ScrubberInterface` and this sub-object at all; if you unwrap it and just `export const` the three functions inside it should be an equivalent interface.

Code after:
export type Scrubber = (key: string, value: unknown) => unknown

export interface ScrubberInterface {
  setScrubber(scrubber: Scrubber | undefined): void
  stringify(value: any): string
}

let _scrubber: Scrubber | undefined = undefined

/**
 * This interface implements a stringify() function, which is used by Slate
 * internally when generating exceptions containing end user data. Developers
 * using Slate may call Scrubber.setScrubber() to alter the behavior of this
 * stringify() function.
 *
 * For example, to prevent the cleartext logging of 'text' fields within Nodes:
 *
 *    import { Scrubber } from 'slate';
 *    Scrubber.setScrubber((key, val) => {
 *      if (key === 'text') return '...scrubbed...'
 *      return val
 *    });
 *
 */
export const Scrubber: ScrubberInterface = {
  setScrubber(scrubber: Scrubber | undefined): void {
    _scrubber = scrubber
  },

  stringify(value: any): string {
    return JSON.stringify(value, _scrubber)
  },
}
Repository: ianstormtaylor/slate
File: packages/slate-react/src/components/editable.tsx (TypeScript)

Code before:
}

/**
 * The props that get passed to renderPlaceholder
 */
export type RenderPlaceholderProps = {
  children: any
  attributes: {
    'data-slate-placeholder': boolean
    dir?: 'rtl'
    contentEditable: boolean
    ref: React.RefObject<any>
    style: React.CSSProperties
  }
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => (
  // COMPAT: Artificially add a line-break to the end on the placeholder element
  // to prevent android IMEs to pick up it's content in autocorrect and to auto-capitalize the first letter
  <span {...attributes}>
    {children}
    {IS_ANDROID && <br />}
  </span>
)

/**
 * A default memoized decorate function.
 */

export const defaultDecorate: (entry: NodeEntry) => Range[] = () => []

/**
 * A default implement to scroll dom range into view.
 */

const defaultScrollSelectionIntoView = (
  editor: ReactEditor,
  domRange: DOMRange
) => {
  // This was affecting the selection of multiple blocks and dragging behavior,
  // so enabled only if the selection has been collapsed.
  if (
    !editor.selection ||
    (editor.selection && Range.isCollapsed(editor.selection))

Review comment (suggestion):
```suggestion
  // to prevent Android IMEs to pick up its content in autocorrect and to auto-capitalize the first letter
```

Code after:
}

/**
 * The props that get passed to renderPlaceholder
 */
export type RenderPlaceholderProps = {
  children: any
  attributes: {
    'data-slate-placeholder': boolean
    dir?: 'rtl'
    contentEditable: boolean
    ref: React.RefObject<any>
    style: React.CSSProperties
  }
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => (
  // COMPAT: Artificially add a line-break to the end on the placeholder element
  // to prevent Android IMEs to pick up its content in autocorrect and to auto-capitalize the first letter
  <span {...attributes}>
    {children}
    {IS_ANDROID && <br />}
  </span>
)

/**
 * A default memoized decorate function.
 */

export const defaultDecorate: (entry: NodeEntry) => Range[] = () => []

/**
 * A default implement to scroll dom range into view.
 */

const defaultScrollSelectionIntoView = (
  editor: ReactEditor,
  domRange: DOMRange
) => {
  // This was affecting the selection of multiple blocks and dragging behavior,
  // so enabled only if the selection has been collapsed.
  if (
    !editor.selection ||
    (editor.selection && Range.isCollapsed(editor.selection))
Repository: ianstormtaylor/slate
File: packages/slate-react/src/hooks/android-input-manager/use-android-input-manager.ts (TypeScript)

Code before:
import { EDITOR_TO_SCHEDULE_FLUSH } from '../../utils/weak-maps'
import {
  createAndroidInputManager,
  CreateAndroidInputManagerOptions,
} from './android-input-manager'
import { useIsMounted } from '../use-is-mounted'
import { useMutationObserver } from '../use-mutation-observer'

type UseAndroidInputManagerOptions = {
  node: RefObject<HTMLElement>
} & Omit<
  CreateAndroidInputManagerOptions,
  'editor' | 'onUserInput' | 'receivedUserInput'
>

const MUTATION_OBSERVER_CONFIG: MutationObserverInit = {
  subtree: true,
  childList: true,
  characterData: true,
}

export function useAndroidInputManager({
  node,
  ...options
}: UseAndroidInputManagerOptions) {
  /* eslint-disable react-hooks/rules-of-hooks */
  // react hooks should not be called conditionally
  // However IS_ANDROID is a constant and so they will be called consistently
  if (!IS_ANDROID) {
    return null
  }

  const editor = useSlateStatic()
  const isMounted = useIsMounted()

  const [inputManager] = useState(() =>
    createAndroidInputManager({
      editor,
      ...options,
    })
  )

  useMutationObserver(
    node,
    inputManager.handleDomMutations,
    MUTATION_OBSERVER_CONFIG
  )

  EDITOR_TO_SCHEDULE_FLUSH.set(editor, inputManager.scheduleFlush)
  if (isMounted) {
    inputManager.flush()

Review comment (suggestion):
Another option here would be to move the IS_ANDROID check outside of the function, like

```tsx
export const useAndroidInputManager = !IS_ANDROID ? () => null : ({
  node,
  ...options
}: UseAndroidInputManagerOptions) => {
```

Code after:
import { EDITOR_TO_SCHEDULE_FLUSH } from '../../utils/weak-maps'
import {
  createAndroidInputManager,
  CreateAndroidInputManagerOptions,
} from './android-input-manager'
import { useIsMounted } from '../use-is-mounted'
import { useMutationObserver } from '../use-mutation-observer'

type UseAndroidInputManagerOptions = {
  node: RefObject<HTMLElement>
} & Omit<
  CreateAndroidInputManagerOptions,
  'editor' | 'onUserInput' | 'receivedUserInput'
>

const MUTATION_OBSERVER_CONFIG: MutationObserverInit = {
  subtree: true,
  childList: true,
  characterData: true,
}

export const useAndroidInputManager = !IS_ANDROID
  ? () => null
  : ({ node, ...options }: UseAndroidInputManagerOptions) => {
      if (!IS_ANDROID) {
        return null
      }

      const editor = useSlateStatic()
      const isMounted = useIsMounted()

      const [inputManager] = useState(() =>
        createAndroidInputManager({
          editor,
          ...options,
        })
      )

      useMutationObserver(
        node,
        inputManager.handleDomMutations,
        MUTATION_OBSERVER_CONFIG
      )

      EDITOR_TO_SCHEDULE_FLUSH.set(editor, inputManager.scheduleFlush)
      if (isMounted) {
        inputManager.flush()
      }

      return inputManager
    }
Repository: ianstormtaylor/slate
File: packages/slate-react/test/index.spec.tsx (TypeScript)

Code before:
    act(() => {
      create(
        <Slate
          editor={editor}
          initialValue={initialValue}
          onSelectorChange={onSelectorChange}
        >
          <Editable />
        </Slate>,
        { createNodeMock }
      )
    })
    await act(async () =>
      Transforms.select(editor, { path: [0, 0], offset: 2 })
    )
    expect(onSelectorChange).toHaveBeenCalled()
  })

  test('calls onValueChange when editor children change', async () => {
    const editor = withReact(createEditor())
    const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
    const onValueChange = jest.fn()
    act(() => {
      create(
        <Slate
          editor={editor}
          initialValue={initialValue}
          onValueChange={onValueChange}
        >
          <Editable />
        </Slate>,
        { createNodeMock }
      )
    })
    await act(async () => Transforms.insertText(editor, 'Hello word!'))
    expect(onValueChange).toHaveBeenCalled()
  })
})


Review comment (suggestion):
Could you add 
```ts
expect(onChange).toHaveBeenCalled()
expect(onValueChange).not.toHaveBeenCalled()
```

Code after:
    const onValueChange = jest.fn()
    const onSelectionChange = jest.fn()

    act(() => {
      create(
        <Slate
          editor={editor}
          initialValue={initialValue}
          onChange={onChange}
          onValueChange={onValueChange}
          onSelectionChange={onSelectionChange}
        >
          <Editable />
        </Slate>,
        { createNodeMock }
      )
    })

    await act(async () =>
      Transforms.select(editor, { path: [0, 0], offset: 2 })
    )

    expect(onSelectionChange).toHaveBeenCalled()
    expect(onChange).toHaveBeenCalled()
    expect(onValueChange).not.toHaveBeenCalled()
  })

  test('calls onValueChange when editor children change', async () => {
    const editor = withReact(createEditor())
    const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
    const onChange = jest.fn()
    const onValueChange = jest.fn()
    const onSelectionChange = jest.fn()

    act(() => {
      create(
        <Slate
          editor={editor}
          initialValue={initialValue}
          onChange={onChange}
          onValueChange={onValueChange}
          onSelectionChange={onSelectionChange}
        >
          <Editable />
        </Slate>,
        { createNodeMock }
      )
    })

    await act(async () => Transforms.insertText(editor, 'Hello word!'))
Repository: ianstormtaylor/slate
File: packages/slate/src/transforms-node/merge-nodes.ts (TypeScript)

Code before:
    } else {
      throw new Error(
        `Cannot merge the node at path [${path}] with the previous sibling because it is not the same kind: ${Scrubber.stringify(
          node
        )} ${Scrubber.stringify(prevNode)}`
      )
    }

    // If the node isn't already the next sibling of the previous node, move
    // it so that it is before merging.
    if (!isPreviousSibling) {
      Transforms.moveNodes(editor, { at: path, to: newPath, voids })
    }

    // If there was going to be an empty ancestor of the node that was merged,
    // we remove it from the tree.
    if (emptyRef) {
      Transforms.removeNodes(editor, { at: emptyRef.current!, voids })
    }

    // If the target node that we're merging with is empty, remove it instead
    // of merging the two. This is a common rich text editor behavior to
    // prevent losing formatting when deleting entire nodes when you have a
    // hanging selection.
    // if prevNode is first child in parent,don't remove it.
    if (editor.shouldMergeNodesRemovePrevNode(prev, current)) {
      Transforms.removeNodes(editor, { at: prevPath, voids })
    } else {
      editor.apply({
        type: 'merge_node',
        path: newPath,
        position,
        properties,
      })
    }

    if (emptyRef) {
      emptyRef.unref()
    }
  })
}


Review comment (suggestion):
Since we've got this on the Editor interface, should we use that here?

```suggestion
    if (Editor.shouldMergeNodesRemovePrevNode(editor, prev, current)) {
```

Code after:
    } else {
      throw new Error(
        `Cannot merge the node at path [${path}] with the previous sibling because it is not the same kind: ${Scrubber.stringify(
          node
        )} ${Scrubber.stringify(prevNode)}`
      )
    }

    // If the node isn't already the next sibling of the previous node, move
    // it so that it is before merging.
    if (!isPreviousSibling) {
      Transforms.moveNodes(editor, { at: path, to: newPath, voids })
    }

    // If there was going to be an empty ancestor of the node that was merged,
    // we remove it from the tree.
    if (emptyRef) {
      Transforms.removeNodes(editor, { at: emptyRef.current!, voids })
    }

    if (Editor.shouldMergeNodesRemovePrevNode(editor, prev, current)) {
      Transforms.removeNodes(editor, { at: prevPath, voids })
    } else {
      editor.apply({
        type: 'merge_node',
        path: newPath,
        position,
        properties,
      })
    }

    if (emptyRef) {
      emptyRef.unref()
    }
  })
}
Repository: ianstormtaylor/slate
File: packages/slate-dom/src/plugin/with-dom.ts (TypeScript)

Code before:
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return
    }

    const [start, end] = Range.edges(selection)
    const startVoid = Editor.void(e, { at: start.path })

Review comment (suggestion):
```suggestion
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED to better reflect reality, see #5792 
        IS_NODE_MAP_DIRTY.set(e, true)
```

Code after:
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED
        // to better reflect reality, see #5792
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return
    }
Repository: ianstormtaylor/slate
File: packages/slate-dom/src/plugin/with-dom.ts (TypeScript)

Code before:
        } else {
          matches.push(...getMatches(e, Path.parent(op.newPath)))
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED to better reflect reality, see #5792 
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return
    }

Review comment (suggestion):
```suggestion
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED 
        // to better reflect reality, see #5792 
```

Code after:
        } else {
          matches.push(...getMatches(e, Path.parent(op.newPath)))
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED
        // to better reflect reality, see #5792
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return
Repository: ianstormtaylor/slate
File: packages/slate-dom/src/plugin/with-dom.ts (TypeScript)

Code before:
        } else {
          matches.push(...getMatches(e, Path.parent(op.newPath)))
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED 
        // to better reflect reality, see #5792 
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return

Review comment (suggestion):
```suggestion
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED
```

Code after:
        } else {
          matches.push(...getMatches(e, Path.parent(op.newPath)))
          changedPath = op.path
        }

        const changedNode = Node.get(editor, Path.parent(changedPath))
        const changedNodeKey = DOMEditor.findKey(e, changedNode)
        const changedPathRef = Editor.pathRef(e, Path.parent(changedPath))
        pathRefMatches.push([changedPathRef, changedNodeKey])

        break
      }
    }

    apply(op)

    switch (op.type) {
      case 'insert_node':
      case 'remove_node':
      case 'merge_node':
      case 'move_node':
      case 'split_node':
      case 'insert_text':
      case 'remove_text':
      case 'set_selection': {
        // FIXME: Rename to something like IS_DOM_EDITOR_DESYNCED
        // to better reflect reality, see #5792
        IS_NODE_MAP_DIRTY.set(e, true)
      }
    }

    for (const [path, key] of matches) {
      const [node] = Editor.node(e, path)
      NODE_TO_KEY.set(node, key)
    }

    for (const [pathRef, key] of pathRefMatches) {
      if (pathRef.current) {
        const [node] = Editor.node(e, pathRef.current)
        NODE_TO_KEY.set(node, key)
      }

      pathRef.unref()
    }
  }

  e.setFragmentData = (data: Pick<DataTransfer, 'getData' | 'setData'>) => {
    const { selection } = e

    if (!selection) {
      return