ronantakizawa/codereview-bench huggingface dataset:
Repository: diegomura/react-pdf
File: index.d.ts (TypeScript)

Code before:
      loading: boolean;
      data: any;
      [key: string]: any;
    }

    type HyphenationCallback = (
      words: string[],
      glyphString: { [key: string]: any },
    ) => string[];

    const Font: {
      register: (options: {
        family: string;
        src: string;
        [key: string]: any;
      }) => void;
      getEmojiSource: () => EmojiSource;
      getRegisteredFonts: () => FontInstance[];
      getRegisteredFontFamilies: () => string[];
      registerEmojiSource: (emojiSource: EmojiSource) => void;
      registerHyphenationCallback: (
        hyphenationCallback: HyphenationCallback,
      ) => void;
      getHyphenationCallback: () => HyphenationCallback;
      getFont: (fontDescriptor: FontDescriptor) => RegisteredFont | undefined;
      load: (
        fontDescriptor: FontDescriptor,
        document: React.ReactElement<DocumentProps>,
      ) => Promise<void>;
      clear: () => void;
      reset: () => void;
    };

    const StyleSheet: {
      hairlineWidth: number;
      create: <TStyles>(styles: TStyles) => TStyles;
      resolve: (
        style: Style,
        container: {
          width: number;
          height: number;
          orientation: Orientation;
        },
      ) => Style;
      flatten: (...style: (Style[] | Style | undefined)[]) => Style;
      absoluteFillObject: {
        position: 'absolute';
        left: 0;
        right: 0;
        top: 0;
        bottom: 0;

Review comment (suggestion):
This is syntax error. Assuming you want array of fonts

```suggestion
        fonts?: {
          src: string;
          fontStyle: string;
          fontWeight: string | number;
          [key: string]: any;
        }[];
```

this is correct syntax

Code after:
    }

    interface FontDescriptor {
      family: string;
      fontStyle: FontStyle;
      fontWeight: FontWeight;
    }

    interface RegisteredFont {
      src: string;
      loaded: boolean;
      loading: boolean;
      data: any;
      [key: string]: any;
    }

    type HyphenationCallback = (
      words: string[],
      glyphString: { [key: string]: any },
    ) => string[];

    const Font: {
      register: (options: {
        family: string;
        src: string;
        fonts?: [font: {
          src: string;
          fontStyle: string;
          fontWeight: string | number;
          [key: string]: any;
        }];
        [key: string]: any;
      }) => void;
      getEmojiSource: () => EmojiSource;
      getRegisteredFonts: () => FontInstance[];
      getRegisteredFontFamilies: () => string[];
      registerEmojiSource: (emojiSource: EmojiSource) => void;
      registerHyphenationCallback: (
        hyphenationCallback: HyphenationCallback,
      ) => void;
      getHyphenationCallback: () => HyphenationCallback;
      getFont: (fontDescriptor: FontDescriptor) => RegisteredFont | undefined;
      load: (
        fontDescriptor: FontDescriptor,
        document: React.ReactElement<DocumentProps>,
      ) => Promise<void>;
      clear: () => void;
      reset: () => void;
    };

    const StyleSheet: {
Repository: diegomura/react-pdf
File: packages/render/src/primitives/renderBackground.js (JavaScript)

Code before:
import * as R from 'ramda';

import save from '../operations/save';
import restore from '../operations/restore';
import clipNode from '../operations/clipNode';
import parseColor from '../utils/parseColor';

const drawBackground = (ctx, node) => {
  if (node.box && node.style.backgroundColor) {
    const { top, left, width, height } = node.box;
    const color = parseColor(node.style.backgroundColor);
    const nodeOpacity = !R.isNil(node.style?.opacity) ? node.style.opacity : 1
    const opacity = Math.min(color.opacity, nodeOpacity);

    ctx
      .fillOpacity(opacity)
      .fillColor(color.value)
      .rect(left, top, width, height)
      .fill();
  }

  return node;
};

const renderBackground = (ctx, node) => {
  const hasBackground = !!node.box && !!node.style?.backgroundColor;

  if (hasBackground) {
    save(ctx, node);
    clipNode(ctx, node);
    drawBackground(ctx, node);
    restore(ctx, node);
  }

  return node;
};


Review comment (suggestion):
Nit. change :)

```suggestion
    const nodeOpacity = R.isNil(node.style?.opacity) ? 1 : node.style.opacity;
```

Code after:
import * as R from 'ramda';

import save from '../operations/save';
import restore from '../operations/restore';
import clipNode from '../operations/clipNode';
import parseColor from '../utils/parseColor';

const drawBackground = (ctx, node) => {
  if (node.box && node.style.backgroundColor) {
    const { top, left, width, height } = node.box;
    const color = parseColor(node.style.backgroundColor);
    const nodeOpacity = R.isNil(node.style?.opacity) ? 1 : node.style.opacity;
    const opacity = Math.min(color.opacity, nodeOpacity);

    ctx
      .fillOpacity(opacity)
      .fillColor(color.value)
      .rect(left, top, width, height)
      .fill();
  }

  return node;
};

const renderBackground = (ctx, node) => {
  const hasBackground = !!node.box && !!node.style?.backgroundColor;

  if (hasBackground) {
    save(ctx, node);
    clipNode(ctx, node);
    drawBackground(ctx, node);
    restore(ctx, node);
  }

  return node;
};
Repository: diegomura/react-pdf
File: packages/layout/src/steps/resolveTextLayout.js (JavaScript)

Code before:
const isSvg = isType(P.Svg);

const isText = isType(P.Text);

const isNotSvg = R.complement(isSvg);

const isNotText = R.complement(isText);

const shouldIterate = node => isNotSvg(node) && isNotText(node);

const shouldLayoutText = node => isText(node) && !node.lines;

/**
 * Performs text layout on text node if wasn't calculated before.
 * Text layout is usually performed on Yoga's layout process (via setMeasureFunc),
 * but we need to layout those nodes with fixed width and height.
 *
 * @param {Object} node
 * @returns {Object} layout node
 */
const resolveTextLayout = (node, fontStore) => {
  const mapChild = child => resolveTextLayout(child, fontStore);

  return R.compose(
    R.evolve({
      children: R.map(R.when(shouldIterate, mapChild)),
    }),
    R.when(
      shouldLayoutText,
      R.compose(
        R.converge(R.assoc('lines'), [
          R.converge(layoutText, [
            R.identity,
            R.path(['box', 'width']),
            R.path(['box', 'height']),
            R.always(fontStore),
          ]),
          R.identity,
        ]),
      ),
    ),
  )(node);
};

export default resolveTextLayout;


Review comment (suggestion):
this function doesn't call `layoutText` at all, because `shouldIterate` blocks execution for text nodes. I move condition to top level and everything is okey now

Code after:

const isSvg = isType(P.Svg);

const isText = isType(P.Text);

const isNotSvg = R.complement(isSvg);

const isNotText = R.complement(isText);

const shouldIterate = node => isNotSvg(node) && isNotText(node);

const shouldLayoutText = node => isText(node) && !node.lines;

/**
 * Performs text layout on text node if wasn't calculated before.
 * Text layout is usually performed on Yoga's layout process (via setMeasureFunc),
 * but we need to layout those nodes with fixed width and height.
 *
 * @param {Object} node
 * @returns {Object} layout node
 */
const resolveTextLayout = (node, fontStore) => {
  if (shouldLayoutText(node)) {
    const width =
      node.box.width - (node.box.paddingRight + node.box.paddingLeft);
    const height =
      node.box.height - (node.box.paddingTop + node.box.paddingBottom);

    node.lines = layoutText(node, width, height, fontStore);
  }

  if (shouldIterate(node)) {
    const mapChild = child => resolveTextLayout(child, fontStore);
    return R.evolve({
      children: R.map(mapChild),
    })(node);
  }

  return node;
};

export default resolveTextLayout;
Repository: diegomura/react-pdf
File: packages/textkit/src/engines/linebreaker/bestFit.js (JavaScript)

Code before:

    if (node.type === 'box') {
      sum.width += node.width;
    } else if (node.type === 'glue') {
      sum.width += node.width;
      sum.stretch += node.stretch;
      sum.shrink += node.shrink;
    }

    if (sum.width - sum.shrink > lineLength) {
      let j;
      if (i === 0) {
        // the first block is always "box" type
        j = i + 1;
      } else {
        j = i;
      }

      while (
        j < subnodes.length &&
        (subnodes[j].type === 'glue' || subnodes[j].type === 'penalty')
      ) {
        j++;
      }

      position = j - 1;
      break;
    }

    if (node.type === 'penalty' || node.type === 'glue') {
      const ratio = calculateRatio(node);
      const penalty = node.type === 'penalty' ? node.penalty : 0;
      const badness = 100 * Math.abs(ratio) ** 3 + penalty;

      if (minimumBadness >= badness) {
        position = i;
        minimumBadness = badness;
      }
    }
  }

  return sum.width - sum.shrink > lineLength ? position : null;
};

const applyBestFit = (nodes, widths) => {
  let count = 0;
  let lineNumber = 0;
  let subnodes = nodes;
  const breakpoints = [{ position: 0 }];

  while (subnodes.length > 0) {

Review comment (suggestion):
please run all this code only when position is `null`, like

```js

if (position == null) {
  // code
}

```

Code after:

    if (node.type === 'box') {
      sum.width += node.width;
    } else if (node.type === 'glue') {
      sum.width += node.width;
      sum.stretch += node.stretch;
      sum.shrink += node.shrink;
    }

    if (sum.width - sum.shrink > lineLength) {
      if (position === null) {
        let j = i === 0 ? i + 1 : i;

        while (
          j < subnodes.length &&
          (subnodes[j].type === 'glue' || subnodes[j].type === 'penalty')
        ) {
          j++;
        }

        position = j - 1;
      }
      break;
    }

    if (node.type === 'penalty' || node.type === 'glue') {
      const ratio = calculateRatio(node);
      const penalty = node.type === 'penalty' ? node.penalty : 0;
      const badness = 100 * Math.abs(ratio) ** 3 + penalty;

      if (minimumBadness >= badness) {
        position = i;
        minimumBadness = badness;
      }
    }
  }

  return sum.width - sum.shrink > lineLength ? position : null;
};

const applyBestFit = (nodes, widths) => {
  let count = 0;
  let lineNumber = 0;
  let subnodes = nodes;
  const breakpoints = [{ position: 0 }];

  while (subnodes.length > 0) {
    const breakpoint = getNextBreakpoint(subnodes, widths, lineNumber);

    if (breakpoint !== null) {
      count += breakpoint;
Repository: diegomura/react-pdf
File: packages/textkit/src/engines/linebreaker/bestFit.js (JavaScript)

Code before:
        ? (lineLength - sum.width) / sum.shrink
        : INFINITY;
    }

    return 0;
  };

  for (let i = 0; i < subnodes.length; i += 1) {
    const node = subnodes[i];

    if (node.type === 'box') {
      sum.width += node.width;
    } else if (node.type === 'glue') {
      sum.width += node.width;
      sum.stretch += node.stretch;
      sum.shrink += node.shrink;
    }

    if (sum.width - sum.shrink > lineLength) {
      let j;
      if (i === 0) {
        // the first block is always "box" type
        j = i + 1;
      } else {
        j = i;
      }

      while (
        j < subnodes.length &&
        (subnodes[j].type === 'glue' || subnodes[j].type === 'penalty')
      ) {
        j++;
      }

      position = j - 1;
      break;
    }

    if (node.type === 'penalty' || node.type === 'glue') {
      const ratio = calculateRatio(node);
      const penalty = node.type === 'penalty' ? node.penalty : 0;
      const badness = 100 * Math.abs(ratio) ** 3 + penalty;

      if (minimumBadness >= badness) {
        position = i;
        minimumBadness = badness;
      }
    }
  }

  return sum.width - sum.shrink > lineLength ? position : null;

Review comment (suggestion):
```suggestion
      // the first block is always "box" type
      let j = i === 0 ? i + 1 : i;
```

Code after:
        ? (lineLength - sum.width) / sum.shrink
        : INFINITY;
    }

    return 0;
  };

  for (let i = 0; i < subnodes.length; i += 1) {
    const node = subnodes[i];

    if (node.type === 'box') {
      sum.width += node.width;
    } else if (node.type === 'glue') {
      sum.width += node.width;
      sum.stretch += node.stretch;
      sum.shrink += node.shrink;
    }

    if (sum.width - sum.shrink > lineLength) {
      if (position === null) {
        let j = i === 0 ? i + 1 : i;

        while (
          j < subnodes.length &&
          (subnodes[j].type === 'glue' || subnodes[j].type === 'penalty')
        ) {
          j++;
        }

        position = j - 1;
      }
      break;
    }

    if (node.type === 'penalty' || node.type === 'glue') {
      const ratio = calculateRatio(node);
      const penalty = node.type === 'penalty' ? node.penalty : 0;
      const badness = 100 * Math.abs(ratio) ** 3 + penalty;

      if (minimumBadness >= badness) {
        position = i;
        minimumBadness = badness;
      }
    }
  }

  return sum.width - sum.shrink > lineLength ? position : null;
};

const applyBestFit = (nodes, widths) => {
  let count = 0;
Repository: diegomura/react-pdf
File: packages/renderer/src/renderer.js (JavaScript)

Code before:
/* eslint-disable no-unused-vars */
/* eslint-disable no-param-reassign */

import ReactFiberReconciler from 'react-reconciler';
import {
  unstable_scheduleCallback as schedulePassiveEffects,
  unstable_cancelCallback as cancelPassiveEffects,
} from 'scheduler';

import propsEqual from './utils/propsEqual';

const emptyObject = {};

const createRenderer = ({ onChange = () => {} }) => {
  return ReactFiberReconciler({
    schedulePassiveEffects,

    cancelPassiveEffects,

    supportsMutation: true,

    isPrimaryRenderer: false,

    warnsIfNotActing: false,

    appendInitialChild(parentInstance, child) {
      parentInstance.children.push(child);
    },

    createInstance(type, { style, children, ...props }) {
      return {
        type,
        box: {},
        style: style || {},
        props: props || {},
        children: [],
      };
    },

    createTextInstance(text, rootContainerInstance) {
      return { type: 'TEXT_INSTANCE', value: text };
    },

    finalizeInitialChildren(element, type, props) {
      return false;
    },

    getPublicInstance(instance) {
      return instance;
    },

Review comment (suggestion):
Hi, I think there are some mistakes here. If we haven't push these node, that may got error when `insertBefore` called. 
```
    insertBefore(parentInstance, child, beforeChild) {
      const index = parentInstance.children?.indexOf(beforeChild);

      if (index === undefined) return;

      if (index !== -1 && child)
        parentInstance.children.splice(index, 0, child);
    },
```
child will be droped if beforeChild is not OrphanTextInstance

Code after:
/* eslint-disable no-unused-vars */
/* eslint-disable no-param-reassign */

import ReactFiberReconciler from 'react-reconciler';
import {
  unstable_scheduleCallback as schedulePassiveEffects,
  unstable_cancelCallback as cancelPassiveEffects,
} from 'scheduler';

import propsEqual from './utils/propsEqual';

const emptyObject = {};

const appendChild = (parentInstance, child) => {
  const isParentText = parentInstance.type === 'TEXT';
  const isChildTextInstance = child.type === 'TEXT_INSTANCE';
  const isOrphanTextInstance = isChildTextInstance && !isParentText;

  // Ignore orphan text instances.
  // Caused by cases such as <>{name && <Text>{name}</Text>}</>
  if (isOrphanTextInstance) {
    console.warn(
      `Invalid '${child.value}' string child outside <Text> component`,
    );
    return;
  }

  parentInstance.children.push(child);
};

const createRenderer = ({ onChange = () => {} }) => {
  return ReactFiberReconciler({
    schedulePassiveEffects,

    cancelPassiveEffects,

    supportsMutation: true,

    isPrimaryRenderer: false,

    warnsIfNotActing: false,

    appendInitialChild: appendChild,

    createInstance(type, { style, children, ...props }) {
      return {
        type,
        box: {},
        style: style || {},
        props: props || {},
Repository: diegomura/react-pdf
File: packages/layout/src/steps/resolvePagination.js (JavaScript)

Code before:

  while (nextPage !== null) {
    splittedPage = splitPage(nextPage, pageNumber + pages.length, fontStore);

    pages.push(splittedPage[0]);
    nextPage = splittedPage[1];
  }

  return pages;
};

/**
 * Performs pagination. This is the step responsible of breaking the whole document
 * into pages following pagiation rules, such as `fixed`, `break` and dynamic nodes.
 *
 * @param {Object} node
 * @param {Object} fontStore font store
 * @returns {Object} layout node
 */
const resolvePagination = (doc, fontStore) => {
  let pages = [];
  let pageNumber = 1;

  for (let i = 0; i < doc.children.length; i += 1) {
    const page = doc.children[i];
    if (!page.props?.wrap) {
      pages.push(page);
      continue;
    }

    let subpages = paginate(page, pageNumber, fontStore);

    subpages = assocSubPageData(subpages);
    pageNumber += subpages.length;
    pages = pages.concat(subpages);
  }

  pages = pages.map((...args) =>
    dissocSubPageData(resolvePageIndices(fontStore, ...args)),
  );

  return assingChildren(pages, doc);
};

export default resolvePagination;


Review comment (suggestion):
We should explicitely check for false here. Otherwise missing `wrap` prop would prevent pages from wrapping which isn't what we want

Code after:
  const pages = [splittedPage[0]];
  let nextPage = splittedPage[1];

  while (nextPage !== null) {
    splittedPage = splitPage(nextPage, pageNumber + pages.length, fontStore);

    pages.push(splittedPage[0]);
    nextPage = splittedPage[1];
  }

  return pages;
};

/**
 * Performs pagination. This is the step responsible of breaking the whole document
 * into pages following pagiation rules, such as `fixed`, `break` and dynamic nodes.
 *
 * @param {Object} node
 * @param {Object} fontStore font store
 * @returns {Object} layout node
 */
const resolvePagination = (doc, fontStore) => {
  let pages = [];
  let pageNumber = 1;

  for (let i = 0; i < doc.children.length; i += 1) {
    const page = doc.children[i];
    let subpages = paginate(page, pageNumber, fontStore);

    subpages = assocSubPageData(subpages);
    pageNumber += subpages.length;
    pages = pages.concat(subpages);
  }

  pages = pages.map((...args) =>
    dissocSubPageData(resolvePageIndices(fontStore, ...args)),
  );

  return assingChildren(pages, doc);
};

export default resolvePagination;
Repository: diegomura/react-pdf
File: packages/renderer/src/dom/PDFDownloadLink.js (JavaScript)

Code before:
/* eslint-disable no-console */

import React, { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF({ document });
  useEffect(updateInstance, [children]);

  if (!document) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };


Review comment (suggestion):
Let's keep alias so we avoid issues with global document value

```suggestion
  document: doc,
```

Code after:
/* eslint-disable no-console */

import { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document: doc,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF();

  useEffect(() => updateInstance(doc), [doc]);

  if (!doc) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };
Repository: diegomura/react-pdf
File: packages/renderer/src/dom/PDFDownloadLink.js (JavaScript)

Code before:
/* eslint-disable no-console */

import React, { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF({ document });
  useEffect(updateInstance, [children]);

  if (!document) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };

  return (
    <a
      href={instance.url}
      download={fileName}
      onClick={handleClick}
      {...rest}

Review comment (suggestion):
```suggestion
  const [instance, updateInstance] = usePDF({ document: doc });
```

Code after:
/* eslint-disable no-console */

import { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document: doc,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF();

  useEffect(() => updateInstance(doc), [doc]);

  if (!doc) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };

  return (
    <a
      href={instance.url}
      download={fileName}
      onClick={handleClick}
Repository: diegomura/react-pdf
File: packages/renderer/src/dom/PDFDownloadLink.js (JavaScript)

Code before:
/* eslint-disable no-console */

import React, { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document: doc,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF({ document: doc });
  useEffect(updateInstance, [children]);

  if (!doc) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };

  return (
    <a
      href={instance.url}
      download={fileName}
      onClick={handleClick}
      {...rest}

Review comment (suggestion):
```suggestion
  const [instance, updateInstance] = usePDF({ document: doc });

```

Code after:
/* eslint-disable no-console */

import { useEffect } from 'react';

import usePDF from './usePDF';

export const PDFDownloadLink = ({
  fileName = 'document.pdf',
  document: doc,
  children,
  onClick,
  href: _filteredOutHref,
  ...rest
}) => {
  const [instance, updateInstance] = usePDF();

  useEffect(() => updateInstance(doc), [doc]);

  if (!doc) {
    console.warn('You should pass a valid document to PDFDownloadLink');
    return null;
  }

  const handleDownloadIE = () => {
    if (window.navigator.msSaveBlob) {
      // IE
      window.navigator.msSaveBlob(instance.blob, fileName);
    }
  };

  const handleClick = event => {
    handleDownloadIE();
    if (typeof onClick === 'function') onClick(event, instance);
  };

  return (
    <a
      href={instance.url}
      download={fileName}
      onClick={handleClick}