ronantakizawa/codereview-bench huggingface dataset:
Repository: ianstormtaylor/slate
File: packages/slate-react/src/hooks/use-children.tsx (TypeScript)

Code before:
    }
  }

  if (Element.isElement(child)) {
    return (
      <ElementComponent
        decorations={ds}
        element={child}
        renderElement={renderElement}
        renderPlaceholder={renderPlaceholder}
        renderLeaf={renderLeaf}
        selection={sel}
      />
    )
  } else {
    return (
      <TextComponent
        decorations={ds}
        isLast={isLast}
        parent={parent}
        renderPlaceholder={renderPlaceholder}
        renderLeaf={renderLeaf}
        text={child}
      />
    )
  }
}

const useChildren = (props: {
  decorations: Range[]
  node: Ancestor
  renderElement?: (props: RenderElementProps) => JSX.Element
  renderPlaceholder: (props: RenderPlaceholderProps) => JSX.Element
  renderLeaf?: (props: RenderLeafProps) => JSX.Element
  selection: Range | null
}) => {
  const {
    decorations,
    node,
    renderElement,
    renderPlaceholder,
    renderLeaf,
    selection,
  } = props
  const editor = useSlateStatic()
  const path = ReactEditor.findPath(editor, node)
  const children = []
  const isLeafBlock =
    Element.isElement(node) &&
    !editor.isInline(node) &&
    Editor.hasInlines(editor, node)

Review comment (suggestion):
`else` is unnecessary IMO
```suggestion
  return (
    <TextComponent
      decorations={ds}
      isLast={isLast}
      parent={parent}
      renderPlaceholder={renderPlaceholder}
      renderLeaf={renderLeaf}
      text={child}
    />
  )
```

Code after:
    }
  }

  if (Element.isElement(child)) {
    return (
      <ElementComponent
        decorations={ds}
        element={child}
        renderElement={renderElement}
        renderPlaceholder={renderPlaceholder}
        renderLeaf={renderLeaf}
        selection={sel}
      />
    )
  }
  return (
    <TextComponent
      decorations={ds}
      isLast={isLast}
      parent={parent}
      renderPlaceholder={renderPlaceholder}
      renderLeaf={renderLeaf}
      text={child}
    />
  )
}

const useChildren = (props: {
  decorations: Range[]
  node: Ancestor
  renderElement?: (props: RenderElementProps) => JSX.Element
  renderPlaceholder: (props: RenderPlaceholderProps) => JSX.Element
  renderLeaf?: (props: RenderLeafProps) => JSX.Element
  selection: Range | null
}) => {
  const {
    decorations,
    node,
    renderElement,
    renderPlaceholder,
    renderLeaf,
    selection,
  } = props
  const editor = useSlateStatic()
  const path = ReactEditor.findPath(editor, node)
  const children = []
  const isLeafBlock =
    Element.isElement(node) &&
    !editor.isInline(node) &&
    Editor.hasInlines(editor, node)
Repository: ianstormtaylor/slate
File: packages/slate-react/src/plugin/react-editor.ts (TypeScript)

Code before:
    }
  },

  /**
   * Focus the editor.
   */

  focus(editor: ReactEditor): void {
    const el = ReactEditor.toDOMNode(editor, editor)
    const root = ReactEditor.findDocumentOrShadowRoot(editor)
    IS_FOCUSED.set(editor, true)

    if (root !== undefined && root.activeElement !== el) {
      el.focus({ preventScroll: true })
    }
  },

  /**
   * Deselect the editor.
   */

  deselect(editor: ReactEditor): void {
    const el = ReactEditor.toDOMNode(editor, editor)
    const { selection } = editor
    const root = ReactEditor.findDocumentOrShadowRoot(editor)
    const domSelection = root ? root.getSelection() : null

    if (domSelection && domSelection.rangeCount > 0) {
      domSelection.removeAllRanges()
    }

    if (selection) {
      Transforms.deselect(editor)
    }
  },

  /**
   * Check if a DOM node is within the editor.
   */

  hasDOMNode(
    editor: ReactEditor,
    target: DOMNode,
    options: { editable?: boolean } = {}
  ): boolean {
    const { editable = false } = options
    const editorEl = ReactEditor.toDOMNode(editor, editor)
    let targetEl

    // COMPAT: In Firefox, reading `target.nodeType` will throw an error if
    // target is originating from an internal "restricted" element (e.g. a

Review comment (suggestion):
```suggestion
    const domSelection = root?.getSelection() ?? null
```

Code after:
    const root = ReactEditor.findDocumentOrShadowRoot(editor)
    IS_FOCUSED.set(editor, true)

    if (root.activeElement !== el) {
      el.focus({ preventScroll: true })
    }
  },

  /**
   * Deselect the editor.
   */

  deselect(editor: ReactEditor): void {
    const el = ReactEditor.toDOMNode(editor, editor)
    const { selection } = editor
    const root = ReactEditor.findDocumentOrShadowRoot(editor)
    const domSelection = root.getSelection()

    if (domSelection && domSelection.rangeCount > 0) {
      domSelection.removeAllRanges()
    }

    if (selection) {
      Transforms.deselect(editor)
    }
  },

  /**
   * Check if a DOM node is within the editor.
   */

  hasDOMNode(
    editor: ReactEditor,
    target: DOMNode,
    options: { editable?: boolean } = {}
  ): boolean {
    const { editable = false } = options
    const editorEl = ReactEditor.toDOMNode(editor, editor)
    let targetEl

    // COMPAT: In Firefox, reading `target.nodeType` will throw an error if
    // target is originating from an internal "restricted" element (e.g. a
    // stepper arrow on a number input). (2018/05/04)
    // https://github.com/ianstormtaylor/slate/issues/1819
    try {
      targetEl = (isDOMElement(target)
        ? target
        : target.parentElement) as HTMLElement
    } catch (err) {
      if (
        !err.message.includes('Permission denied to access property "nodeType"')
Repository: ianstormtaylor/slate
File: packages/slate-react/src/plugin/react-editor.ts (TypeScript)

Code before:
        break
      }

      path.unshift(i)
      child = parent
    }

    throw new Error(
      `Unable to find the path for Slate node: ${JSON.stringify(node)}`
    )
  },

  /**
   * Find the DOM node that implements DocumentOrShadowRoot for the editor.
   */

  findDocumentOrShadowRoot(editor: ReactEditor): Document | ShadowRoot {
    const el = ReactEditor.toDOMNode(editor, editor)
    const root = el.getRootNode()

    // The below exception will always be thrown for iframes because the document inside an iframe
    // does not inherit it's prototype from the parent document, therefore we return early
    if (el.ownerDocument !== document) return el.ownerDocument

    if (!(root instanceof Document || root instanceof ShadowRoot))
      return ReactEditor.getWindow(editor).document

    // COMPAT: Only Chrome implements the DocumentOrShadowRoot mixin for
    // ShadowRoot; other browsers still implement it on the Document
    // interface. (2020/08/08)
    // https://developer.mozilla.org/en-US/docs/Web/API/ShadowRoot#Properties
    if (root.getSelection === undefined && el.ownerDocument !== null)
      return el.ownerDocument

    return root
  },

  /**
   * Check if the editor is focused.
   */

  isFocused(editor: ReactEditor): boolean {
    return !!IS_FOCUSED.get(editor)
  },

  /**
   * Check if the editor is in read-only mode.
   */

  isReadOnly(editor: ReactEditor): boolean {
    return !!IS_READ_ONLY.get(editor)

Review comment (suggestion):
I think this could be simplified to:

```suggestion
      return el.ownerDocument
```

Though to be honest, I think the entire method could be re-written, the logic is hard to follow currently.

Something like:

```ts
findDocumentOrShadowRoot(editor: ReactEditor): Document | ShadowRoot {
  const el = ReactEditor.toDOMNode(editor, editor)
  const root = el.getRootNode()

  if ((root instanceof Document || root instanceof ShadowRoot) && root.getSelection != null) {
    return root
  }
  
  return el.ownerDocument
}
```

Code after:
        break
      }

      path.unshift(i)
      child = parent
    }

    throw new Error(
      `Unable to find the path for Slate node: ${JSON.stringify(node)}`
    )
  },

  /**
   * Find the DOM node that implements DocumentOrShadowRoot for the editor.
   */

  findDocumentOrShadowRoot(editor: ReactEditor): Document | ShadowRoot {
    const el = ReactEditor.toDOMNode(editor, editor)
    const root = el.getRootNode()

    if (
      (root instanceof Document || root instanceof ShadowRoot) &&
      root.getSelection != null
    ) {
      return root
    }

    return el.ownerDocument
  },

  /**
   * Check if the editor is focused.
   */

  isFocused(editor: ReactEditor): boolean {
    return !!IS_FOCUSED.get(editor)
  },

  /**
   * Check if the editor is in read-only mode.
   */

  isReadOnly(editor: ReactEditor): boolean {
    return !!IS_READ_ONLY.get(editor)
  },

  /**
   * Blur the editor.
   */

  blur(editor: ReactEditor): void {
Repository: ianstormtaylor/slate
File: packages/slate-react/src/components/editable.tsx (TypeScript)

Code before:
import { ReactEditor } from '..'
import { ReadOnlyContext } from '../hooks/use-read-only'
import { useSlate } from '../hooks/use-slate'
import { useIsomorphicLayoutEffect } from '../hooks/use-isomorphic-layout-effect'
import { DecorateContext } from '../hooks/use-decorate'
import {
  DOMElement,
  DOMNode,
  DOMRange,
  getDefaultView,
  isDOMElement,
  isDOMNode,
  isPlainTextOnlyPaste,
} from '../utils/dom'

import {
  EDITOR_TO_ELEMENT,
  ELEMENT_TO_NODE,
  IS_READ_ONLY,
  NODE_TO_ELEMENT,
  IS_FOCUSED,
  PLACEHOLDER_SYMBOL,
  EDITOR_TO_WINDOW,
} from '../utils/weak-maps'

type DeferredOperation = { type: 'insertText'; text: string }

const Children = (props: Parameters<typeof useChildren>[0]) => (
  <React.Fragment>{useChildren(props)}</React.Fragment>
)

/**
 * `RenderElementProps` are passed to the `renderElement` handler.
 */

export interface RenderElementProps {
  children: any
  element: Element
  attributes: {
    'data-slate-node': 'element'
    'data-slate-inline'?: true
    'data-slate-void'?: true
    dir?: 'rtl'
    ref: any
  }
}

/**
 * `RenderLeafProps` are passed to the `renderLeaf` handler.
 */


Review comment (suggestion):
```suggestion
type DeferredOperation = () => void;
```

Code after:
import { ReactEditor } from '..'
import { ReadOnlyContext } from '../hooks/use-read-only'
import { useSlate } from '../hooks/use-slate'
import { useIsomorphicLayoutEffect } from '../hooks/use-isomorphic-layout-effect'
import { DecorateContext } from '../hooks/use-decorate'
import {
  DOMElement,
  DOMNode,
  DOMRange,
  getDefaultView,
  isDOMElement,
  isDOMNode,
  isPlainTextOnlyPaste,
} from '../utils/dom'

import {
  EDITOR_TO_ELEMENT,
  ELEMENT_TO_NODE,
  IS_READ_ONLY,
  NODE_TO_ELEMENT,
  IS_FOCUSED,
  PLACEHOLDER_SYMBOL,
  EDITOR_TO_WINDOW,
} from '../utils/weak-maps'

type DeferredOperation = () => void

const Children = (props: Parameters<typeof useChildren>[0]) => (
  <React.Fragment>{useChildren(props)}</React.Fragment>
)

/**
 * `RenderElementProps` are passed to the `renderElement` handler.
 */

export interface RenderElementProps {
  children: any
  element: Element
  attributes: {
    'data-slate-node': 'element'
    'data-slate-inline'?: true
    'data-slate-void'?: true
    dir?: 'rtl'
    ref: any
  }
}

/**
 * `RenderLeafProps` are passed to the `renderLeaf` handler.
 */
Repository: ianstormtaylor/slate
File: packages/slate-react/src/components/editable.tsx (TypeScript)

Code before:
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => <span {...attributes}>{children}</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
) => {
  if (editor.selection?.anchor?.path[0] !== editor.selection?.focus?.path[0]) {
    return
  }
  const leafEl = domRange.startContainer.parentElement!
  leafEl.getBoundingClientRect = domRange.getBoundingClientRect.bind(domRange)
  scrollIntoView(leafEl, {
    scrollMode: 'if-needed',
  })
  delete leafEl.getBoundingClientRect
}

/**
 * Check if two DOM range objects are equal.
 */

export const isRangeEqual = (a: DOMRange, b: DOMRange) => {
  return (
    (a.startContainer === b.startContainer &&
      a.startOffset === b.startOffset &&
      a.endContainer === b.endContainer &&
      a.endOffset === b.endOffset) ||
    (a.startContainer === b.endContainer &&
      a.startOffset === b.endOffset &&
      a.endContainer === b.startContainer &&
      a.endOffset === b.startOffset)
  )

Review comment (refactor):
Not sure if the check is for an expanded range, if so `Range.isExpanded` should be used. If not, a variable should be extracted so that the conditional is a bit more readable.

Code after:
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => <span {...attributes}>{children}</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))
  ) {
    const leafEl = domRange.startContainer.parentElement!
    leafEl.getBoundingClientRect = domRange.getBoundingClientRect.bind(domRange)
    scrollIntoView(leafEl, {
      scrollMode: 'if-needed',
    })
    delete leafEl.getBoundingClientRect
  }
}

/**
 * Check if two DOM range objects are equal.
 */

export const isRangeEqual = (a: DOMRange, b: DOMRange) => {
  return (
    (a.startContainer === b.startContainer &&
      a.startOffset === b.startOffset &&
      a.endContainer === b.endContainer &&
      a.endOffset === b.endOffset) ||
    (a.startContainer === b.endContainer &&
Repository: ianstormtaylor/slate
File: packages/slate-react/src/components/editable.tsx (TypeScript)

Code before:
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => <span {...attributes}>{children}</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 selecting multi blocks and dragging behaviour so, enabled only
  // if the selection is collapsed.
  if (
    !editor.selection ||
    (editor.selection && Range.isCollapsed(editor.selection))
  ) {
    const leafEl = domRange.startContainer.parentElement!
    leafEl.getBoundingClientRect = domRange.getBoundingClientRect.bind(domRange)
    scrollIntoView(leafEl, {
      scrollMode: 'if-needed',
    })
    delete leafEl.getBoundingClientRect
  }
}

/**
 * Check if two DOM range objects are equal.
 */

export const isRangeEqual = (a: DOMRange, b: DOMRange) => {
  return (
    (a.startContainer === b.startContainer &&
      a.startOffset === b.startOffset &&
      a.endContainer === b.endContainer &&
      a.endOffset === b.endOffset) ||
    (a.startContainer === b.endContainer &&

Review comment (suggestion):
```suggestion
  // This was affecting the selection of multiple blocks and dragging behavior, so enabled only
```

Code after:
}

/**
 * The default placeholder element
 */

export const DefaultPlaceholder = ({
  attributes,
  children,
}: RenderPlaceholderProps) => <span {...attributes}>{children}</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))
  ) {
    const leafEl = domRange.startContainer.parentElement!
    leafEl.getBoundingClientRect = domRange.getBoundingClientRect.bind(domRange)
    scrollIntoView(leafEl, {
      scrollMode: 'if-needed',
    })
    delete leafEl.getBoundingClientRect
  }
}

/**
 * Check if two DOM range objects are equal.
 */

export const isRangeEqual = (a: DOMRange, b: DOMRange) => {
  return (
    (a.startContainer === b.startContainer &&
      a.startOffset === b.startOffset &&
      a.endContainer === b.endContainer &&
      a.endOffset === b.endOffset) ||
    (a.startContainer === b.endContainer &&
Repository: ianstormtaylor/slate
File: packages/slate/src/create-editor.ts (TypeScript)

Code before:
      for (const ref of Editor.pointRefs(editor)) {
        PointRef.transform(ref, op)
      }

      for (const ref of Editor.rangeRefs(editor)) {
        RangeRef.transform(ref, op)
      }

      const set = new Set()
      const dirtyPaths: Path[] = []

      const add = (path: Path | null) => {
        if (path) {
          const key = path.join(',')

          if (!set.has(key)) {
            set.add(key)
            dirtyPaths.push(path)
          }
        }
      }

      const oldDirtyPaths = DIRTY_PATHS.get(editor) || []
      const canTransformPath = Path.operationCanTransformPath(op)
      for (const path of oldDirtyPaths) {
        if (canTransformPath) {
          const newPath = Path.transform(path, op)
          add(newPath)
        } else {
          add(path)
        }
      }

      const newDirtyPaths = getDirtyPaths(op)
      for (const path of newDirtyPaths) {
        add(path)
      }

      DIRTY_PATHS.set(editor, dirtyPaths)
      Transforms.transform(editor, op)
      editor.operations.push(op)
      Editor.normalize(editor)

      // Clear any formats applied to the cursor if the selection changes.
      if (op.type === 'set_selection') {
        editor.marks = null
      }

      if (!FLUSHING.get(editor)) {
        FLUSHING.set(editor, true)


Review comment (performance):
I was hoping we could optimize this more but I realized we have to rebuild the set for de-duping each time. Maybe the key set should be persisted with the DIRTY_PATHS so this doesn't need to be reconstructed each time. This code path can be so hot on large edits.

Code after:
      for (const ref of Editor.pointRefs(editor)) {
        PointRef.transform(ref, op)
      }

      for (const ref of Editor.rangeRefs(editor)) {
        RangeRef.transform(ref, op)
      }

      const oldDirtyPaths = DIRTY_PATHS.get(editor) || []
      const oldDirtyPathKeys = DIRTY_PATH_KEYS.get(editor) || new Set()
      let dirtyPaths: Path[]
      let dirtyPathKeys: Set<string>

      const add = (path: Path | null) => {
        if (path) {
          const key = path.join(',')

          if (!dirtyPathKeys.has(key)) {
            dirtyPathKeys.add(key)
            dirtyPaths.push(path)
          }
        }
      }

      if (Path.operationCanTransformPath(op)) {
        dirtyPaths = []
        dirtyPathKeys = new Set()
        for (const path of oldDirtyPaths) {
          const newPath = Path.transform(path, op)
          add(newPath)
        }
      } else {
        dirtyPaths = oldDirtyPaths
        dirtyPathKeys = oldDirtyPathKeys
      }

      const newDirtyPaths = getDirtyPaths(op)
      for (const path of newDirtyPaths) {
        add(path)
      }

      DIRTY_PATHS.set(editor, dirtyPaths)
      DIRTY_PATH_KEYS.set(editor, dirtyPathKeys)
      Transforms.transform(editor, op)
      editor.operations.push(op)
      Editor.normalize(editor)

      // Clear any formats applied to the cursor if the selection changes.
      if (op.type === 'set_selection') {
        editor.marks = null
      }
Repository: ianstormtaylor/slate
File: site/examples/iframe.tsx (TypeScript)

Code before:
    children = <em>{children}</em>
  }

  if (leaf.underline) {
    children = <u>{children}</u>
  }

  return <span {...attributes}>{children}</span>
}

const MarkButton = ({ format, icon }) => {
  const editor = useSlate()
  return (
    <Button
      active={isMarkActive(editor, format)}
      onMouseDown={event => {
        event.preventDefault()
        toggleMark(editor, format)
      }}
    >
      <Icon>{icon}</Icon>
    </Button>
  )
}

const IFrame = ({ children, onLoad = null, ...props }) => {
  const [iframeBody, setIframeBody] = useState(null)

  const handleLoad = e => {
    onLoad && onLoad(e)
    !e.defaultPrevented && setIframeBody(e.target.contentDocument.body)
  }

  return (
    <iframe srcDoc={`<!DOCTYPE html>`} {...props} onLoad={handleLoad}>
      {iframeBody && createPortal(children, iframeBody)}
    </iframe>
  )
}

const initialValue: Descendant[] = [
  {
    type: 'paragraph',
    children: [
      {
        text: 'In this example, the document gets rendered into a controlled ',
      },
      { text: '<iframe>', code: true },
      {
        text: '. This is ',
      },

Review comment (suggestion):
Any specific reason to assign null to `onLoad`, instead of leaving it undefined?

```suggestion
const IFrame = ({ children, onLoad, ...props }) => {
```

Code after:
    children = <em>{children}</em>
  }

  if (leaf.underline) {
    children = <u>{children}</u>
  }

  return <span {...attributes}>{children}</span>
}

const MarkButton = ({ format, icon }) => {
  const editor = useSlate()
  return (
    <Button
      active={isMarkActive(editor, format)}
      onMouseDown={event => {
        event.preventDefault()
        toggleMark(editor, format)
      }}
    >
      <Icon>{icon}</Icon>
    </Button>
  )
}

const IFrame = ({ children, ...props }) => {
  const [iframeBody, setIframeBody] = useState(null)
  const handleLoad = e => {
    setIframeBody(e.target.contentDocument.body)
  }
  return (
    <iframe srcDoc={`<!DOCTYPE html>`} {...props} onLoad={handleLoad}>
      {iframeBody && createPortal(children, iframeBody)}
    </iframe>
  )
}

const initialValue: Descendant[] = [
  {
    type: 'paragraph',
    children: [
      {
        text: 'In this example, the document gets rendered into a controlled ',
      },
      { text: '<iframe>', code: true },
      {
        text: '. This is ',
      },
      {
        text: 'particularly',
        italic: true,