ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/__tests__/authors.test.ts (TypeScript)

Code before:
      }),
    ).toEqual([
      {
        name: 'Sébastien Lorber',
        title: 'maintainer',
        imageURL: undefined,
        socials: {
          github: 'https://github.com/slorber',
        },
        key: null,
        page: null,
      },
    ]);
  });

  it('can read authors Author[]', () => {
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: [
            {
              name: 'Sébastien Lorber',
              title: 'maintainer',
              socials: {
                github: 'slorber',
              },
            },
            {
              name: 'Yangshun Tay',
              socials: {
                github: 'https://github.com/yangshun',
              },
            },
          ],
        },
        authorsMap: undefined,
        baseUrl: '/',
      }),
    ).toEqual([
      {
        name: 'Sébastien Lorber',
        title: 'maintainer',
        imageURL: undefined,
        key: null,
        socials: {
          github: 'https://github.com/slorber',
        },
        page: null,
      },
      {
        name: 'Yangshun Tay',

Review comment (bug):
Need to test more variety of social platforms, not just github

Also including edge cases behaviors: {}, null, undefined etc

Code after:
        authorsMap: undefined,
        baseUrl: '/',
      });
    }

    // @ts-expect-error test
    expect(() => testSocials(null)).not.toThrow();
    // @ts-expect-error test
    expect(testSocials(null)).toEqual([
      {
        name: 'Sébastien Lorber',
        title: 'maintainer',
        imageURL: undefined,
        socials: {},
        key: null,
        page: null,
      },
    ]);
    expect(() => () => testSocials(undefined)).not.toThrow();
    // @ts-expect-error test
    expect(() => testSocials({twitter: undefined}))
      .toThrowErrorMatchingInlineSnapshot(`
      "Author socials should be usernames/userIds/handles, or fully qualified HTTP(s) absolute URLs.
      Social platform 'twitter' has illegal value 'undefined'"
    `);
    expect(
      // @ts-expect-error test
      () => testSocials({twitter: null}),
    ).toThrowErrorMatchingInlineSnapshot(`
      "Author socials should be usernames/userIds/handles, or fully qualified HTTP(s) absolute URLs.
      Social platform 'twitter' has illegal value 'null'"
    `);
  });

  it('can read empty socials', () => {
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: {
            name: 'Sébastien Lorber',
            title: 'maintainer',
            socials: {},
          },
        },
        authorsMap: undefined,
        baseUrl: '/',
      }),
    ).toEqual([
      {
        name: 'Sébastien Lorber',
        title: 'maintainer',
Repository: facebook/docusaurus
File: packages/docusaurus-theme-classic/src/theme/Blog/Pages/BlogAuthorsPostsPage/index.tsx (TypeScript)

Code before:
}

function ViewAllAuthorsLink() {
  const {authorsListPath} = useBlogMetadata();
  return (
    <Link href={authorsListPath}>
      <BlogAuthorsListViewAllLabel />
    </Link>
  );
}

function Content({author, items, sidebar, listMetadata}: Props): JSX.Element {
  return (
    <BlogLayout sidebar={sidebar}>
      <header className="margin-bottom--xl">
        <Author as="h1" author={author} />
        {author.description && <p>{author.description}</p>}
        <ViewAllAuthorsLink />
      </header>
      {items.length === 0 ? (
        <p>
          <Translate
            id="theme.BlogAuthorsPostsPage.Content.noPosts"
            description="The text for authors with 0 blog post">
            This author has not written any posts yet.
          </Translate>
        </p>
      ) : (
        <>
          <hr />
          <BlogPostItems items={items} />
          <BlogListPaginator metadata={listMetadata} />
        </>
      )}
    </BlogLayout>
  );
}

export default function BlogAuthorsPostsPage(props: Props): JSX.Element {
  return (
    <HtmlClassNameProvider
      className={clsx(
        ThemeClassNames.wrapper.blogPages,
        ThemeClassNames.page.blogAuthorsPostsPage,
      )}>
      <Metadata {...props} />
      <Content {...props} />
    </HtmlClassNameProvider>
  );
}


Review comment (security):
```suggestion
          <Translate
            id="theme.blog.author.noPosts"
            description="The text for authors with 0 blog post">
            This author hasn't written any posts yet.
          </Translate>
```

minor changes + move to blogTranslations.ts

Code after:
}

function ViewAllAuthorsLink() {
  const {authorsListPath} = useBlogMetadata();
  return (
    <Link href={authorsListPath}>
      <BlogAuthorsListViewAllLabel />
    </Link>
  );
}

function Content({author, items, sidebar, listMetadata}: Props): JSX.Element {
  return (
    <BlogLayout sidebar={sidebar}>
      <header className="margin-bottom--xl">
        <Author as="h1" author={author} />
        {author.description && <p>{author.description}</p>}
        <ViewAllAuthorsLink />
      </header>
      {items.length === 0 ? (
        <p>
          <BlogAuthorNoPostsLabel />
        </p>
      ) : (
        <>
          <hr />
          <BlogPostItems items={items} />
          <BlogListPaginator metadata={listMetadata} />
        </>
      )}
    </BlogLayout>
  );
}

export default function BlogAuthorsPostsPage(props: Props): JSX.Element {
  return (
    <HtmlClassNameProvider
      className={clsx(
        ThemeClassNames.wrapper.blogPages,
        ThemeClassNames.page.blogAuthorsPostsPage,
      )}>
      <Metadata {...props} />
      <Content {...props} />
    </HtmlClassNameProvider>
  );
}
Repository: facebook/docusaurus
File: packages/docusaurus-theme-common/src/translations/blogTranslations.tsx (TypeScript)

Code before:
    },
    {
      nPosts: blogPostsPlural(author.count),
      authorName: author.name || author.key,
    },
  );
}

export const translateBlogAuthorsListPageTitle = (): string =>
  translate({
    id: 'theme.blog.authorsList.pageTitle',
    message: 'Authors',
    description: 'The title of the authors page',
  });

export function BlogAuthorsListViewAllLabel(): ReactNode {
  return (
    <Translate
      id="theme.blog.authorsList.viewAll"
      description="The label of the link targeting the blog authors page">
      View all authors
    </Translate>
  );
}

export function BlogAuthorNoPostLabel(): ReactNode {
  return (
    <Translate
      id="theme.blog.author.noPosts"
      description="The text for authors with 0 blog post">
      This author has not written any posts yet.
    </Translate>
  );
}


Review comment (security):
```suggestion
export function BlogAuthorNoPostsLabel(): ReactNode {
```

Let's be consistent with the id ending with an s

Code after:
    },
    {
      nPosts: blogPostsPlural(author.count),
      authorName: author.name || author.key,
    },
  );
}

export const translateBlogAuthorsListPageTitle = (): string =>
  translate({
    id: 'theme.blog.authorsList.pageTitle',
    message: 'Authors',
    description: 'The title of the authors page',
  });

export function BlogAuthorsListViewAllLabel(): ReactNode {
  return (
    <Translate
      id="theme.blog.authorsList.viewAll"
      description="The label of the link targeting the blog authors page">
      View all authors
    </Translate>
  );
}

export function BlogAuthorNoPostsLabel(): ReactNode {
  return (
    <Translate
      id="theme.blog.author.noPosts"
      description="The text for authors with 0 blog post">
      This author has not written any posts yet.
    </Translate>
  );
}
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/authorsMap.ts (TypeScript)

Code before:
    const errorMessage = permalinkCounts
      .map(
        ([permalink, authors]) =>
          `Permalink: ${permalink}\nAuthors: ${authors
            .map((author) => author.name || 'Unknown')
            .join(', ')}`,
      )
      .join('\n');

    throw new Error(
      `The following permalinks are duplicated:\n${errorMessage}`,
    );
  }
}

function normalizeAuthor({
  authorsBaseRoutePath,
  authorKey,
  baseUrl,
  author,
}: {
  authorsBaseRoutePath: string;
  authorKey: string;
  baseUrl: string;
  author: AuthorInput;
}): Author & {key: string} {
  function getAuthorPage(): AuthorPage | null {
    if (!author.page) {
      return null;
    }
    const slug =
      author.page === true ? _.kebabCase(authorKey) : author.page.permalink;
    return {
      permalink: normalizeUrl([authorsBaseRoutePath, slug]),
    };
  }

  return {
    ...author,
    key: authorKey,
    page: getAuthorPage(),
    imageURL: normalizeImageUrl({imageURL: author.imageURL, baseUrl}),
    socials: author.socials ? normalizeSocials(author.socials) : undefined,
  };
}

function normalizeAuthorsMap({
  authorsBaseRoutePath,
  authorsMapInput,
  baseUrl,
}: {

Review comment (security):
Forgot to use it in another PR, but you can use type `AuthorWithKey` instead of `Author & {key: string}`

Code after:
    const errorMessage = permalinkCounts
      .map(
        ([permalink, authors]) =>
          `Permalink: ${permalink}\nAuthors: ${authors
            .map((author) => author.name || 'Unknown')
            .join(', ')}`,
      )
      .join('\n');

    throw new Error(
      `The following permalinks are duplicated:\n${errorMessage}`,
    );
  }
}

function normalizeAuthor({
  authorsBaseRoutePath,
  authorKey,
  baseUrl,
  author,
}: {
  authorsBaseRoutePath: string;
  authorKey: string;
  baseUrl: string;
  author: AuthorInput;
}): AuthorWithKey {
  function getAuthorPage(): AuthorPage | null {
    if (!author.page) {
      return null;
    }
    const slug =
      author.page === true ? _.kebabCase(authorKey) : author.page.permalink;
    return {
      permalink: normalizeUrl([authorsBaseRoutePath, slug]),
    };
  }

  return {
    ...author,
    key: authorKey,
    page: getAuthorPage(),
    imageURL: normalizeImageUrl({imageURL: author.imageURL, baseUrl}),
    socials: author.socials ? normalizeSocials(author.socials) : undefined,
  };
}

function normalizeAuthorsMap({
  authorsBaseRoutePath,
  authorsMapInput,
  baseUrl,
}: {
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/__tests__/authors.test.ts (TypeScript)

Code before:
      }),
    ).toEqual([
      {
        key: 'slorber',
        name: 'Sébastien Lorber',
        imageURL: '/img/slorber.png',
        page: null,
      },
    ]);
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: 'slorber',
        },
        authorsMap: {
          slorber: {
            name: 'Sébastien Lorber',
            imageURL: '/img/slorber.png',
            key: 'slorber',
            page: null,
          },
        },
        baseUrl: '/baseUrl',
      }),
    ).toEqual([
      {
        key: 'slorber',
        name: 'Sébastien Lorber',
        imageURL: '/img/slorber.png',
        page: null,
      },
    ]);
  });

  it('can read authors string[]', () => {
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: ['slorber', 'yangshun'],
        },
        authorsMap: {
          slorber: {
            name: 'Sébastien Lorber',
            title: 'maintainer',
            key: 'slorber',
            page: null,
          },
          yangshun: {name: 'Yangshun Tay', key: 'yangshun', page: null},
        },
        baseUrl: '/',
      }),

Review comment (security):
to make this test more realistic/consistent with real world usage, I'd use `imageURL: '/baseUrl/img/slorber.png',` for `slorber` because the authors map is  nowsupposed to be provided with the baseUrl already

Code after:
      }),
    ).toEqual([
      {
        key: 'slorber',
        name: 'Sébastien Lorber',
        imageURL: '/img/slorber.png',
        page: null,
      },
    ]);
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: 'slorber',
        },
        authorsMap: {
          slorber: {
            name: 'Sébastien Lorber',
            imageURL: '/baseUrl/img/slorber.png',
            key: 'slorber',
            page: null,
          },
        },
        baseUrl: '/baseUrl',
      }),
    ).toEqual([
      {
        key: 'slorber',
        name: 'Sébastien Lorber',
        imageURL: '/baseUrl/img/slorber.png',
        page: null,
      },
    ]);
  });

  it('can read authors string[]', () => {
    expect(
      getBlogPostAuthors({
        frontMatter: {
          authors: ['slorber', 'yangshun'],
        },
        authorsMap: {
          slorber: {
            name: 'Sébastien Lorber',
            title: 'maintainer',
            key: 'slorber',
            page: null,
          },
          yangshun: {name: 'Yangshun Tay', key: 'yangshun', page: null},
        },
        baseUrl: '/',
      }),
Repository: facebook/docusaurus
File: packages/docusaurus-theme-classic/src/theme/BlogPostPage/Metadata/index.tsx (TypeScript)

Code before:
/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import React from 'react';
import {PageMetadata} from '@docusaurus/theme-common';
import {useBlogPost} from '@docusaurus/plugin-content-blog/client';

export default function BlogPostPageMetadata(): JSX.Element {
  const {assets, metadata} = useBlogPost();
  const {title, description, date, tags, authors, frontMatter} = metadata;

  const {keywords} = frontMatter;
  const image = assets.image ?? frontMatter.image;
  return (
    <PageMetadata
      title={metadata.title_meta ?? title}
      description={description}
      keywords={keywords}
      image={image}>
      <meta property="og:type" content="article" />
      <meta property="article:published_time" content={date} />
      {/* TODO double check those article meta array syntaxes, see https://ogp.me/#array */}
      {authors.some((author) => author.url) && (
        <meta
          property="article:author"
          content={authors
            .map((author) => author.url)
            .filter(Boolean)
            .join(',')}
        />
      )}
      {tags.length > 0 && (
        <meta
          property="article:tag"
          content={tags.map((tag) => tag.label).join(',')}
        />
      )}
    </PageMetadata>
  );
}


Review comment (suggestion):
```suggestion
      title={frontMatter.title_meta ?? title}
```

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import React from 'react';
import {PageMetadata} from '@docusaurus/theme-common';
import {useBlogPost} from '@docusaurus/plugin-content-blog/client';

export default function BlogPostPageMetadata(): JSX.Element {
  const {assets, metadata} = useBlogPost();
  const {title, description, date, tags, authors, frontMatter} = metadata;

  const {keywords} = frontMatter;
  const image = assets.image ?? frontMatter.image;
  return (
    <PageMetadata
      title={frontMatter.title_meta ?? title}
      description={description}
      keywords={keywords}
      image={image}>
      <meta property="og:type" content="article" />
      <meta property="article:published_time" content={date} />
      {/* TODO double check those article meta array syntaxes, see https://ogp.me/#array */}
      {authors.some((author) => author.url) && (
        <meta
          property="article:author"
          content={authors
            .map((author) => author.url)
            .filter(Boolean)
            .join(',')}
        />
      )}
      {tags.length > 0 && (
        <meta
          property="article:tag"
          content={tags.map((tag) => tag.label).join(',')}
        />
      )}
    </PageMetadata>
  );
}
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/readingTime.ts (TypeScript)

Code before:
/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import readingTime from 'reading-time';

const DEFAULT_WORDS_PER_MINUTE = 300;

interface ReadingTimeOptions {
  wordsPerMinute?: number;
  wordBound?: (char: string) => boolean;
}

/**
 * Calculates the reading time for a given content string.
 * Uses the reading-time package under the hood.
 */
export function calculateReadingTime(
  content: string,
  options: ReadingTimeOptions = {},
): number {
  const wordsPerMinute = options.wordsPerMinute ?? DEFAULT_WORDS_PER_MINUTE;
  return readingTime(content, {wordsPerMinute}).minutes;
}


Review comment (suggestion):
Here you are not passing the `wordBound` option to the underlying lib. 

Even though we may want to remove it in the future, it is likely a breaking change on our public API, so I'd rather not do it now.

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import readingTime from 'reading-time';

const DEFAULT_WORDS_PER_MINUTE = 200;

interface ReadingTimeOptions {
  wordsPerMinute?: number;
  wordBound?: (char: string) => boolean;
}

/**
 * Calculates the reading time for a given content string.
 * Uses the reading-time package under the hood.
 */
export function calculateReadingTime(
  content: string,
  options: ReadingTimeOptions = {},
): number {
  const wordsPerMinute = options.wordsPerMinute ?? DEFAULT_WORDS_PER_MINUTE;
  const {wordBound} = options;
  return readingTime(content, {wordsPerMinute, ...(wordBound && {wordBound})})
    .minutes;
}
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/blogUtils.ts (TypeScript)

Code before:
}: {
  filePath: string;
  parseFrontMatter: ParseFrontMatter;
}) {
  const fileContent = await fs.readFile(filePath, 'utf-8');
  try {
    const result = await parseMarkdownFile({
      filePath,
      fileContent,
      parseFrontMatter,
      removeContentTitle: true,
    });
    return {
      ...result,
      frontMatter: validateBlogPostFrontMatter(result.frontMatter),
    };
  } catch (err) {
    logger.error`Error while parsing blog post file path=${filePath}.`;
    throw err;
  }
}

const defaultReadingTime: ReadingTimeFunction = ({
  content,
  options,
  locale = 'en',
}) => calculateReadingTime(content, {...options, locale});

async function processBlogSourceFile(
  blogSourceRelative: string,
  contentPaths: BlogContentPaths,
  context: LoadContext,
  options: PluginOptions,
  tagsFile: TagsFile | null,
  authorsMap?: AuthorsMap,
): Promise<BlogPost | undefined> {
  const {
    siteConfig: {
      baseUrl,
      markdown: {parseFrontMatter},
    },
    siteDir,
    i18n,
  } = context;
  const {
    routeBasePath,
    tagsBasePath: tagsRouteBasePath,
    truncateMarker,
    showReadingTime,
    editUrl,
  } = options;

Review comment (refactor):
I'd prefer not to have this magic value hardcoded. This probably requires to refactor types a bit.

Code after:
}: {
  filePath: string;
  parseFrontMatter: ParseFrontMatter;
}) {
  const fileContent = await fs.readFile(filePath, 'utf-8');
  try {
    const result = await parseMarkdownFile({
      filePath,
      fileContent,
      parseFrontMatter,
      removeContentTitle: true,
    });
    return {
      ...result,
      frontMatter: validateBlogPostFrontMatter(result.frontMatter),
    };
  } catch (err) {
    logger.error`Error while parsing blog post file path=${filePath}.`;
    throw err;
  }
}

const defaultReadingTime: ReadingTimeFunction = ({content, locale, options}) =>
  calculateReadingTime(content, locale, options);

async function processBlogSourceFile(
  blogSourceRelative: string,
  contentPaths: BlogContentPaths,
  context: LoadContext,
  options: PluginOptions,
  tagsFile: TagsFile | null,
  authorsMap?: AuthorsMap,
): Promise<BlogPost | undefined> {
  const {
    siteConfig: {
      baseUrl,
      markdown: {parseFrontMatter},
    },
    siteDir,
    i18n,
  } = context;
  const {
    routeBasePath,
    tagsBasePath: tagsRouteBasePath,
    truncateMarker,
    showReadingTime,
    editUrl,
  } = options;

  // Lookup in localized folder in priority
  const blogDirPath = await getFolderContainingFile(
Repository: facebook/docusaurus
File: packages/docusaurus-plugin-content-blog/src/plugin-content-blog.d.ts (TypeScript)

Code before:
    defaultCreateFeedItems: (
      params: DefaultCreateFeedItemsParams,
    ) => Promise<BlogFeedItem[]>;
  };

  /**
   * Options for reading time calculation using Intl.Segmenter.
   */
  type ReadingTimeOptions = {
    wordsPerMinute?: number;
    locale: string;
  };

  /**
   * Represents the default reading time implementation.
   * @returns The reading time directly plugged into metadata.
   */
  export type ReadingTimeFunction = (params: {
    /** Markdown content. */
    content: string;
    /** Front matter. */
    frontMatter?: BlogPostFrontMatter & {[key: string]: unknown};
    /** Options for reading time calculation. */
    options?: ReadingTimeOptions;
    /** Locale for word segmentation. */
    locale?: string;
  }) => number;

  /**
   * @returns The reading time directly plugged into metadata.
   * `undefined` to hide reading time for a specific post.
   */
  export type ReadingTimeFunctionOption = (
    params: Required<Omit<Parameters<ReadingTimeFunction>[0], 'options'>> & {
      /**
       * The default reading time implementation.
       */
      defaultReadingTime: ReadingTimeFunction;
    },
  ) => number | undefined;

  export type ProcessBlogPostsFn = (params: {
    blogPosts: BlogPost[];
  }) => Promise<void | BlogPost[]>;

  /**
   * Plugin options after normalization.
   */
  export type PluginOptions = MDXOptions &
    TagsPluginOptions & {
      /** Plugin ID. */

Review comment (suggestion):
In our case, the locale should probably be required; there's no single case we would pass undefined here, we always have a site locale available

Code after:
    defaultCreateFeedItems: (
      params: DefaultCreateFeedItemsParams,
    ) => Promise<BlogFeedItem[]>;
  };

  /**
   * Options for reading time calculation using Intl.Segmenter.
   */
  type ReadingTimeOptions = {
    wordsPerMinute?: number;
  };

  /**
   * Represents the default reading time implementation.
   * @returns The reading time directly plugged into metadata.
   */
  export type ReadingTimeFunction = (params: {
    /** Markdown content. */
    content: string;
    /** Locale for word segmentation. */
    locale: string;
    /** Front matter. */
    frontMatter?: BlogPostFrontMatter & {[key: string]: unknown};
    /** Options for reading time calculation. */
    options?: ReadingTimeOptions;
  }) => number;

  /**
   * @returns The reading time directly plugged into metadata.
   * `undefined` to hide reading time for a specific post.
   */
  export type ReadingTimeFunctionOption = (
    params: Required<Omit<Parameters<ReadingTimeFunction>[0], 'options'>> & {
      /**
       * The default reading time implementation.
       */
      defaultReadingTime: ReadingTimeFunction;
    },
  ) => number | undefined;

  export type ProcessBlogPostsFn = (params: {
    blogPosts: BlogPost[];
  }) => Promise<void | BlogPost[]>;

  /**
   * Plugin options after normalization.
   */
  export type PluginOptions = MDXOptions &
    TagsPluginOptions & {
      /** Plugin ID. */
      id?: string;