ronantakizawa/codereview-bench huggingface dataset:
Repository: gatsbyjs/gatsby
File: packages/gatsby-plugin-mdx/gatsby/source-nodes.js (JavaScript)

Code before:
      options,
      store,
      pathPrefix,
      getNode,
      getNodes,
      cache,
      reporter,
      actions,
      schema,
      ...helpers,
    })

  // New Code // Schema
  const MdxType = schema.buildObjectType({
    name: `Mdx`,
    fields: {
      rawBody: { type: `String!` },
      fileAbsolutePath: { type: `String!` },
      frontmatter: { type: `MdxFrontmatter` },
      slug: {
        type: `String`,
        async resolve(mdxNode, args, context) {
          try {
            const nodeWithContext = context.nodeModel.findRootNodeAncestor(
              mdxNode,
              node => node.internal && node.internal.type === `File`
            )
            const fileRelativePath = nodeWithContext.relativePath

            const parsedPath = path.parse(fileRelativePath)

            let relevantPath
            if (parsedPath.name === `index`) {
              relevantPath = fileRelativePath.replace(parsedPath.base, ``)
            } else {
              relevantPath = fileRelativePath.replace(parsedPath.ext, ``)
            }
            return slugify(relevantPath, {
              remove: /[^\w\s$*_+~.()'"!\-:@/]/g,
            })
          } catch {
            reporter.warn(`gatsby-plugin-mdx: Your MDX files are not sourced from your local file system.
            \nAs a result there will be no slug available.`)
            return null
          }
        },
      },
      body: {
        type: `String!`,
        async resolve(mdxNode) {
          const { body } = await processMDX({ node: mdxNode })

Review comment (suggestion):
You can use optional chaining `node.internal?.type === 'File'`

Code after:
      options,
      store,
      pathPrefix,
      getNode,
      getNodes,
      cache,
      reporter,
      actions,
      schema,
      ...helpers,
    })

  // New Code // Schema
  const MdxType = schema.buildObjectType({
    name: `Mdx`,
    fields: {
      rawBody: { type: `String!` },
      fileAbsolutePath: { type: `String!` },
      frontmatter: { type: `MdxFrontmatter` },
      slug: {
        type: `String`,
        async resolve(mdxNode, args, context) {
          const nodeWithContext = context.nodeModel.findRootNodeAncestor(
            mdxNode,
            node => node.internal && node.internal.type === `File`
          )

          if (!nodeWithContext) {
            return null
          }
          const fileRelativePath = nodeWithContext.relativePath

          const parsedPath = path.parse(fileRelativePath)

          let relevantPath
          if (parsedPath.name === `index`) {
            relevantPath = fileRelativePath.replace(parsedPath.base, ``)
          } else {
            relevantPath = fileRelativePath.replace(parsedPath.ext, ``)
          }
          return slugify(relevantPath, {
            remove: /[^\w\s$*_+~.()'"!\-:@/]/g, // this is the set of allowable characters
          })
        },
      },
      body: {
        type: `String!`,
        async resolve(mdxNode) {
          const { body } = await processMDX({ node: mdxNode })
          return body
        },
Repository: gatsbyjs/gatsby
File: packages/gatsby-telemetry/src/event-storage.ts (TypeScript)

Code before:
import path from "path"
import Configstore from "configstore"
import createFetch from "@turist/fetch"
import { Store } from "./store"
import { ensureDirSync } from "fs-extra"
import { isTruthy } from "./is-truthy"
import { InMemoryConfigStore } from "./in-memory-store"

const fetch = createFetch()

/* The events data collection is a spooled process that
 * buffers events to a local fs based buffer
 * which then is asynchronously flushed to the server.
 * This both increases the fault tolerance and allows collection
 * to continue even when working offline.
 */
export class EventStorage {
  analyticsApi =
    process.env.GATSBY_TELEMETRY_API || `https://analytics.gatsbyjs.com/events`
  // eslint-disable-line @typescript-eslint/no-explicit-any
  config: Configstore
  store: Store
  verbose: boolean
  debugEvents: boolean
  disabled: boolean

  constructor() {
    try {
      this.config = new Configstore(`gatsby`, {}, { globalConfigPath: true })
    } catch (e) {
      this.config = new InMemoryConfigStore()
    }

    const baseDir = path.dirname(this.config.path)

    try {
      ensureDirSync(baseDir)
    } catch (e) {
      // TODO: Log this event
    }

    this.store = new Store(baseDir)
    this.verbose = isTruthy(process.env.GATSBY_TELEMETRY_VERBOSE)
    this.debugEvents = isTruthy(process.env.GATSBY_TELEMETRY_DEBUG)
    this.disabled = isTruthy(process.env.GATSBY_TELEMETRY_DISABLED)
  }

Review comment (suggestion):

```suggestion
  config: Configstore | InMemoryConfigStore
```

Code after:
import path from "path"
import Configstore from "configstore"
import createFetch from "@turist/fetch"
import { Store } from "./store"
import { ensureDirSync } from "fs-extra"
import { isTruthy } from "./is-truthy"
import { InMemoryConfigStore } from "./in-memory-store"

const fetch = createFetch()

/* The events data collection is a spooled process that
 * buffers events to a local fs based buffer
 * which then is asynchronously flushed to the server.
 * This both increases the fault tolerance and allows collection
 * to continue even when working offline.
 */
export class EventStorage {
  analyticsApi =
    process.env.GATSBY_TELEMETRY_API || `https://analytics.gatsbyjs.com/events`
  config: Configstore | InMemoryConfigStore
  store: Store
  verbose: boolean
  debugEvents: boolean
  disabled: boolean

  constructor() {
    try {
      this.config = new Configstore(`gatsby`, {}, { globalConfigPath: true })
    } catch (e) {
      this.config = new InMemoryConfigStore()
    }

    const baseDir = path.dirname(this.config.path)

    try {
      ensureDirSync(baseDir)
    } catch (e) {
      // TODO: Log this event
    }

    this.store = new Store(baseDir)
    this.verbose = isTruthy(process.env.GATSBY_TELEMETRY_VERBOSE)
    this.debugEvents = isTruthy(process.env.GATSBY_TELEMETRY_DEBUG)
    this.disabled = isTruthy(process.env.GATSBY_TELEMETRY_DISABLED)
  }
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-contentful/src/extend-node-type.js (JavaScript)

Code before:
            },
            type: `ContentfulEntry`,
          })

          const resultAssets = await context.nodeModel.runQuery({
            query: {
              filter: {
                contentful_id: { in: rawAssets },
              },
            },
            type: `ContentfulAsset`,
          })

          // Localize results
          const nodeLocale = parent.node_locale

          const findForLocaleWithFallback = (nodeList = [], referenceId) =>
            nodeList.find(
              result =>
                result.contentful_id === referenceId &&
                result.node_locale === nodeLocale
            )

          const localizedEntryReferences = rawEntries.map(referenceId =>
            findForLocaleWithFallback(resultEntries, referenceId)
          )

          const localizedAssetReferences = rawAssets.map(referenceId =>
            findForLocaleWithFallback(resultAssets, referenceId)
          )

          return [...localizedEntryReferences, ...localizedAssetReferences]
        },
      },
    }
  }

  if (type.name !== `ContentfulAsset`) {
    return {}
  }

  const getTracedSVG = async args => {
    const { traceSVG } = require(`gatsby-plugin-sharp`)

    const { image, options } = args
    const {
      file: { contentType },
    } = image

    if (contentType.indexOf(`image/`) !== 0) {
      return null

Review comment (suggestion):
Can the same contentful id appear more than once? This implies that the id is not unique for multiple languages. If the id is unique then the locale check could be dropped here.

Code after:
          const resultEntries = await context.nodeModel.runQuery({
            query: {
              filter: {
                contentful_id: { in: rawEntries },
              },
            },
            type: `ContentfulEntry`,
          })

          const resultAssets = await context.nodeModel.runQuery({
            query: {
              filter: {
                contentful_id: { in: rawAssets },
              },
            },
            type: `ContentfulAsset`,
          })

          // Localize results
          const nodeLocale = parent.node_locale

          const findForLocaleWithFallback = (nodeList = [], referenceId) =>
            nodeList.find(
              result =>
                result.contentful_id === referenceId &&
                result.node_locale === nodeLocale
            )

          const localizedEntryReferences = rawEntries.map(referenceId =>
            findForLocaleWithFallback(resultEntries, referenceId)
          )

          const localizedAssetReferences = rawAssets.map(referenceId =>
            findForLocaleWithFallback(resultAssets, referenceId)
          )

          return [...localizedEntryReferences, ...localizedAssetReferences]
        },
      },
    }
  }

  if (type.name !== `ContentfulAsset`) {
    return {}
  }

  const getTracedSVG = async args => {
    const { traceSVG } = require(`gatsby-plugin-sharp`)

    const { image, options } = args
    const {
Repository: gatsbyjs/gatsby
File: packages/gatsby-cli/src/init-starter.ts (TypeScript)

Code before:
    trackError(`NEW_PROJECT_IS_NPM_PROJECT`)
    report.panic({
      id: `11613`,
      context: {
        rootPath,
      },
    })
    return
  }

  const hostedInfo = hostedGitInfo.fromUrl(starterPath)

  trackCli(`NEW_PROJECT`, {
    starterName: hostedInfo ? hostedInfo.shortcut() : `local:starter`,
  })
  if (hostedInfo) {
    await clone(hostedInfo, rootPath)
  } else {
    await copy(starterPath, rootPath)
  }

  const sitePath = sysPath.resolve(rootPath)

  const sitePackageJson = await fs
    .readJSON(sysPath.join(sitePath, `package.json`))
    .catch(() => {})

  await createServiceLock(sitePath, `metadata`, {
    name: sitePackageJson?.name || rootPath,
    sitePath,
    lastRun: Date.now(),
  }).then(unlock => unlock?.())

  successMessage(rootPath)
  trackCli(`NEW_PROJECT_END`)
}


Review comment (suggestion):
Nit: Add a verbose log instead of the infamous swallow catch `() => {}`

Code after:
  if (existsSync(sysPath.join(rootPath, `package.json`))) {
    trackError(`NEW_PROJECT_IS_NPM_PROJECT`)
    report.panic({
      id: `11613`,
      context: {
        rootPath,
      },
    })
    return
  }

  const hostedInfo = hostedGitInfo.fromUrl(starterPath)

  trackCli(`NEW_PROJECT`, {
    starterName: hostedInfo ? hostedInfo.shortcut() : `local:starter`,
  })
  if (hostedInfo) {
    await clone(hostedInfo, rootPath)
  } else {
    await copy(starterPath, rootPath)
  }

  const sitePath = sysPath.resolve(rootPath)

  const sitePackageJson = await fs
    .readJSON(sysPath.join(sitePath, `package.json`))
    .catch(() => {
      reporter.verbose(
        `Could not read "${sysPath.join(sitePath, `package.json`)}"`
      )
    })

  await createServiceLock(sitePath, `metadata`, {
    name: sitePackageJson?.name || rootPath,
    sitePath,
    lastRun: Date.now(),
  }).then(unlock => unlock?.())

  successMessage(rootPath)
  trackCli(`NEW_PROJECT_END`)
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-cli/src/init-starter.ts (TypeScript)

Code before:
import opn from "better-opn"
import { execSync } from "child_process"
import execa from "execa"
import { sync as existsSync } from "fs-exists-cached"
import fs from "fs-extra"
import { trackCli, trackError } from "gatsby-telemetry"
import hostedGitInfo from "hosted-git-info"
import isValid from "is-valid-path"
import sysPath from "path"
import prompts from "prompts"
import url from "url"

import report from "./reporter"
import { getPackageManager, promptPackageManager } from "./util/package-manager"
import { isTTY } from "./util/is-tty"

const spawnWithArgs = (
  file: string,
  args: string[],
  options?: execa.Options
): execa.ExecaChildProcess =>
  execa(file, args, { stdio: `inherit`, preferLocal: false, ...options })

const spawn = (
  cmd: string,
  options?: execa.Options
): execa.ExecaChildProcess => {
  const [file, ...args] = cmd.split(/\s+/)
  return spawnWithArgs(file, args, options)
}
// Checks the existence of yarn package and user preference if it exists
// We use yarnpkg instead of yarn to avoid conflict with Hadoop yarn
// Refer to https://github.com/yarnpkg/yarn/issues/673
const shouldUseYarn = async (): Promise<boolean> => {
  try {
    execSync(`yarnpkg --version`, { stdio: `ignore` })


Review comment (suggestion):
```suggestion
// TODO make service-lock export from gatsby-core-utils
import { createServiceLock } from "gatsby-core-utils/dist/service-lock"
```

Code after:
import opn from "better-opn"
import { execSync } from "child_process"
import execa from "execa"
import { sync as existsSync } from "fs-exists-cached"
import fs from "fs-extra"
import { trackCli, trackError } from "gatsby-telemetry"
import hostedGitInfo from "hosted-git-info"
import isValid from "is-valid-path"
import sysPath from "path"
import prompts from "prompts"
import url from "url"
import { createServiceLock } from "gatsby-core-utils/dist/service-lock"
import report from "./reporter"
import { getPackageManager, promptPackageManager } from "./util/package-manager"
import { isTTY } from "./util/is-tty"
import reporter from "../lib/reporter"

const spawnWithArgs = (
  file: string,
  args: string[],
  options?: execa.Options
): execa.ExecaChildProcess =>
  execa(file, args, { stdio: `inherit`, preferLocal: false, ...options })

const spawn = (
  cmd: string,
  options?: execa.Options
): execa.ExecaChildProcess => {
  const [file, ...args] = cmd.split(/\s+/)
  return spawnWithArgs(file, args, options)
}
// Checks the existence of yarn package and user preference if it exists
// We use yarnpkg instead of yarn to avoid conflict with Hadoop yarn
// Refer to https://github.com/yarnpkg/yarn/issues/673
const shouldUseYarn = async (): Promise<boolean> => {
  try {
    execSync(`yarnpkg --version`, { stdio: `ignore` })
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/commands/develop.ts (TypeScript)

Code before:
// NOTE(@mxstbr): Do not use the reporter in this file, as that has side-effects on import which break structured logging
import path from "path"
import http from "http"
import tmp from "tmp"
import { spawn, ChildProcess } from "child_process"
import chokidar from "chokidar"
import getRandomPort from "detect-port"
import { detectPortInUseAndPrompt } from "../utils/detect-port-in-use-and-prompt"
import socket from "socket.io"
import fs from "fs-extra"
import { isCI, slash } from "gatsby-core-utils"
import { createServiceLock } from "gatsby-core-utils/dist/service-lock"
import { UnlockFn } from "gatsby-core-utils/src/service-lock"
import reporter from "gatsby-cli/lib/reporter"
import { getSslCert } from "../utils/get-ssl-cert"
import { startDevelopProxy } from "../utils/develop-proxy"
import { IProgram, IDebugInfo } from "./types"

// Adapted from https://stackoverflow.com/a/16060619
const requireUncached = (file: string): any => {
  try {
    delete require.cache[require.resolve(file)]
  } catch (e) {
    return null
  }

  try {
    return require(file)
  } catch (e) {
    return null
  }
}

// Heuristics for gatsby-config.js, as not all changes to it require a full restart to take effect
const doesConfigChangeRequireRestart = (
  lastConfig: Record<string, any>,
  newConfig: Record<string, any>
): boolean => {
  // Ignore changes to siteMetadata
  const replacer = (_, v): string | void => {

Review comment (suggestion):
```suggestion
// TODO make service-lock export from gatsby-core-utils
import {
  createServiceLock,
  getService,
} from "gatsby-core-utils/dist/service-lock"
```

Code after:
// NOTE(@mxstbr): Do not use the reporter in this file, as that has side-effects on import which break structured logging
import path from "path"
import http from "http"
import tmp from "tmp"
import { spawn, ChildProcess } from "child_process"
import chokidar from "chokidar"
import getRandomPort from "detect-port"
import { detectPortInUseAndPrompt } from "../utils/detect-port-in-use-and-prompt"
import socket from "socket.io"
import fs from "fs-extra"
import { isCI, slash } from "gatsby-core-utils"
import {
  createServiceLock,
  getService,
} from "gatsby-core-utils/dist/service-lock"
import { UnlockFn } from "gatsby-core-utils/src/service-lock"
import reporter from "gatsby-cli/lib/reporter"
import { getSslCert } from "../utils/get-ssl-cert"
import { startDevelopProxy } from "../utils/develop-proxy"
import { IProgram, IDebugInfo } from "./types"

// Adapted from https://stackoverflow.com/a/16060619
const requireUncached = (file: string): any => {
  try {
    delete require.cache[require.resolve(file)]
  } catch (e) {
    return null
  }

  try {
    return require(file)
  } catch (e) {
    return null
  }
}

// Heuristics for gatsby-config.js, as not all changes to it require a full restart to take effect
const doesConfigChangeRequireRestart = (
  lastConfig: Record<string, any>,
  newConfig: Record<string, any>
Repository: gatsbyjs/gatsby
File: packages/gatsby-core-utils/src/site-metadata.ts (TypeScript)

Code before:
import { createServiceLock, getService } from "./service-lock"

export interface ISiteMetadata {
  sitePath: string
  name?: string
  pid?: number
  lastRun?: number
}

export async function getSiteMetadata(
  sitePath: string
): Promise<ISiteMetadata | null> {
  return getService(sitePath, `metadata`, true)
}

export async function updateSiteMetadata(
  metadata: ISiteMetadata,
  merge = true
): Promise<void> {
  if (merge) {
    const oldMetadata = (await getSiteMetadata(metadata.sitePath)) || {}
    metadata = { ...oldMetadata, ...metadata }
  }

  return createServiceLock(
    metadata.sitePath,
    `metadata`,
    metadata
  ).then(unlock => unlock?.())
}


Review comment (suggestion):
Nit

I would kind of suggest to not use a default and to use constants to prevent magic variables and "the boolean trap"-problem.

```js
export const AND_MERGE = true
export const NOT_MERGE = false
```
or smth, or to export a version of this function that passes on false for you

```js
export async function updateSiteMetadataAndMerge(
  metadata: ISiteMetadata
) {
  return updateSiteMetadata(metadata, true)
}
```
etc

Code after:
import { createServiceLock, getService } from "./service-lock"

export interface ISiteMetadata {
  sitePath: string
  name?: string
  pid?: number
  lastRun?: number
}

export async function getSiteMetadata(
  sitePath: string
): Promise<ISiteMetadata | null> {
  return getService(sitePath, `metadata`, true)
}

export async function updateSiteMetadata(
  metadata: ISiteMetadata,
  merge = true
): Promise<void> {
  if (merge) {
    const oldMetadata = await getSiteMetadata(metadata.sitePath)
    if (oldMetadata) {
      metadata = { ...oldMetadata, ...metadata }
    }
  }

  return createServiceLock(
    metadata.sitePath,
    `metadata`,
    metadata
  ).then(unlock => unlock?.())
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-core-utils/src/site-metadata.ts (TypeScript)

Code before:
import { createServiceLock, getService } from "./service-lock"

export interface ISiteMetadata {
  sitePath: string
  name?: string
  pid?: number
  lastRun?: number
}

export async function getSiteMetadata(
  sitePath: string
): Promise<ISiteMetadata | null> {
  return getService(sitePath, `metadata`, true)
}

export async function updateSiteMetadata(
  metadata: ISiteMetadata,
  merge = true
): Promise<void> {
  if (merge) {
    const oldMetadata = (await getSiteMetadata(metadata.sitePath)) || {}
    metadata = { ...oldMetadata, ...metadata }
  }

  return createServiceLock(
    metadata.sitePath,
    `metadata`,
    metadata
  ).then(unlock => unlock?.())
}


Review comment (suggestion):
Nit
```suggestion
    const oldMetadata = await getSiteMetadata(metadata.sitePath)
    if (oldMetadata) {
      metadata = { ...oldMetadata, ...metadata }
    }
```

Code after:
import { createServiceLock, getService } from "./service-lock"

export interface ISiteMetadata {
  sitePath: string
  name?: string
  pid?: number
  lastRun?: number
}

export async function getSiteMetadata(
  sitePath: string
): Promise<ISiteMetadata | null> {
  return getService(sitePath, `metadata`, true)
}

export async function updateSiteMetadata(
  metadata: ISiteMetadata,
  merge = true
): Promise<void> {
  if (merge) {
    const oldMetadata = await getSiteMetadata(metadata.sitePath)
    if (oldMetadata) {
      metadata = { ...oldMetadata, ...metadata }
    }
  }

  return createServiceLock(
    metadata.sitePath,
    `metadata`,
    metadata
  ).then(unlock => unlock?.())
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-plugin-sharp/src/scheduler.js (JavaScript)

Code before:
const { v4: uuidv4 } = require(`uuid`)
const path = require(`path`)
const fs = require(`fs`)
const got = require(`got`)
const { createContentDigest } = require(`gatsby-core-utils`)
const worker = require(`./gatsby-worker`)
const { createOrGetProgressBar } = require(`./utils`)

const processImages = async (jobId, job, boundActionCreators) => {
  try {
    await worker.IMAGE_PROCESSING(job)
  } catch (err) {
    throw err
  } finally {
    boundActionCreators.endJob({ id: jobId }, { name: `gatsby-plugin-sharp` })
  }
}

const jobsInFlight = new Map()
const scheduleJob = async (job, boundActionCreators, reporter) => {
  const inputPaths = job.inputPaths.filter(
    inputPath => !fs.existsSync(path.join(job.outputDir, inputPath))
  )

  // all paths exists so we bail
  if (!inputPaths.length) {
    return Promise.resolve()
  }

Review comment (suggestion):
Please keep fs-extra as it has better windows support.
```suggestion
const fs = require(`fs-extra`)
```

Code after:
const { v4: uuidv4 } = require(`uuid`)
const path = require(`path`)
const fs = require(`fs-extra`)
const got = require(`got`)
const { createContentDigest } = require(`gatsby-core-utils`)
const worker = require(`./gatsby-worker`)
const { createOrGetProgressBar } = require(`./utils`)

const processImages = async (jobId, job, boundActionCreators) => {
  try {
    await worker.IMAGE_PROCESSING(job)
  } catch (err) {
    throw err
  } finally {
    boundActionCreators.endJob({ id: jobId }, { name: `gatsby-plugin-sharp` })
  }
}

const jobsInFlight = new Map()
const scheduleJob = async (job, boundActionCreators, reporter) => {
  const inputPaths = job.inputPaths.filter(
    inputPath => !fs.existsSync(path.join(job.outputDir, inputPath))
  )

  // all paths exists so we bail
  if (!inputPaths.length) {
    return Promise.resolve()
  }