ronantakizawa/codereview-bench huggingface dataset:
Repository: gatsbyjs/gatsby
File: packages/gatsby-recipes/src/providers/gatsby/site-metadata.js (JavaScript)

Code before:

            const newSiteMetadataTemplate = template(`
              const foo = ${JSON.stringify(newSiteMetadataObj, null, 2)}
            `)()

            const newSiteMetadata = newSiteMetadataTemplate.declarations[0].init

            right.properties = right.properties.map(p => {
              if (p.key.name !== `siteMetadata`) return p

              return {
                ...p,
                value: newSiteMetadata,
              }
            })

            path.stop()
          },
        },
      }
    })
  }
}

class BabelPluginGetSiteMetadataFromConfig {
  constructor() {
    this.state = {}

    this.plugin = declare(api => {
      api.assertVersion(7)

      return {
        visitor: {
          ExpressionStatement: path => {
            const { node } = path
            const { left, right } = node.expression

            if (!isDefaultExport(left)) {
              return
            }

            const siteMetadata = right.properties.find(
              p => p.key.name === `siteMetadata`
            )

            if (!siteMetadata || !siteMetadata.value) return

            this.state = getObjectFromNode(siteMetadata.value)
          },
        },
      }

Review comment (suggestion):
@mxstbr was fixing this parallelism support & also added blank slate support i.e. when gatsby-config.js doesn't have `siteMetadata` added.

Code after:

            const valueType = typeof value
            const shouldParse =
              valueType !== `string` && valueType !== `undefined`
            const newSiteMetadataObj = {
              ...siteMetadataObj,
              [key]: shouldParse ? JSON.parse(value) : value,
            }

            const newSiteMetadataTemplate = template(`
              const foo = ${JSON.stringify(newSiteMetadataObj, null, 2)}
            `)()

            const newSiteMetadata = newSiteMetadataTemplate.declarations[0].init

            if (siteMetadataExists) {
              right.properties = right.properties.map(p => {
                if (p.key.name !== `siteMetadata`) return p

                return {
                  ...p,
                  value: newSiteMetadata,
                }
              })
            } else {
              right.properties.unshift(
                t.objectProperty(t.identifier(`siteMetadata`), newSiteMetadata)
              )
            }

            path.stop()
          },
        },
      }
    })
  }
}

class BabelPluginGetSiteMetadataFromConfig {
  constructor() {
    this.state = {}

    this.plugin = declare(api => {
      api.assertVersion(7)

      return {
        visitor: {
          ExpressionStatement: path => {
            const { node } = path
            const { left, right } = node.expression
Repository: gatsbyjs/gatsby
File: packages/gatsby-cli/src/reporter/redux/internal-actions.ts (TypeScript)

Code before:

  const globalStatus = getGlobalStatus(id, status)

  if (globalStatus !== logsState.status) {
    actionsToEmit.push(setStatus(globalStatus))
  }

  actionsToEmit.push({
    type: Actions.PendingActivity,
    payload: {
      id,
      type: ActivityTypes.Pending,
      status,
    },
  })

  return actionsToEmit
}

type QueuedStartActivityActions = Array<
  IStartActivity | ReturnType<typeof setStatus>
>
export const startActivity = ({
  id,
  text,
  type,
  status = ActivityStatuses.InProgress,
  current,
  total,
}: {
  id: string
  text: string
  type: ActivityTypes
  status?: ActivityStatuses
  current?: number
  total?: number
}): QueuedStartActivityActions => {
  const actionsToEmit: QueuedStartActivityActions = []

  const logsState = getStore().getState().logs

  const globalStatus = getGlobalStatus(id, status)

  if (globalStatus !== logsState.status) {
    actionsToEmit.push(setStatus(globalStatus))
  }

  actionsToEmit.push({
    type: Actions.StartActivity,
    payload: {
      id,

Review comment (suggestion):
gist of this and other activity related code changes is that we always would use `setStatus` action creator as it internally already checks for global status (or pending global status for delayed dispatchs)

Code after:
      timestamp: new Date().toJSON(),
      stack,
    },
  }
}

type ActionsToEmit = Array<IPendingActivity | ReturnType<typeof setStatus>>
export const createPendingActivity = ({
  id,
  status = ActivityStatuses.NotStarted,
}: {
  id: string
  status?: ActivityStatuses
}): ActionsToEmit => {
  const globalStatus = getGlobalStatus(id, status)
  return [
    setStatus(globalStatus),
    {
      type: Actions.PendingActivity,
      payload: {
        id,
        type: ActivityTypes.Pending,
        status,
      },
    },
  ]
}

type QueuedStartActivityActions = Array<
  IStartActivity | ReturnType<typeof setStatus>
>
export const startActivity = ({
  id,
  text,
  type,
  status = ActivityStatuses.InProgress,
  current,
  total,
}: {
  id: string
  text: string
  type: ActivityTypes
  status?: ActivityStatuses
  current?: number
  total?: number
}): QueuedStartActivityActions => {
  const globalStatus = getGlobalStatus(id, status)

  return [
    setStatus(globalStatus),
    {
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-drupal/src/gatsby-node.js (JavaScript)

Code before:
    concurrentFileRequests,
    disallowedLinkTypes,
    skipFileDownloads,
    fastBuilds,
    translation,
  } = pluginOptions
  const { createNode, setPluginStatus, touchNode } = actions

  // Default apiBase to `jsonapi`
  apiBase = apiBase || `jsonapi`

  // Default disallowedLinkTypes to self, describedby.
  disallowedLinkTypes = disallowedLinkTypes || [`self`, `describedby`]

  // Default concurrentFileRequests to `20`
  concurrentFileRequests = concurrentFileRequests || 20

  // Default skipFileDownloads to false.
  skipFileDownloads = skipFileDownloads || false

  // Determine what entities can be translated and what languages are enabled.
  
  const languageConfig = (
    store.getState().status.plugins?.[`gatsby-source-drupal`]?.languageConfig
    || await fetchLanguageConfig({ translation, baseUrl, apiBase, basicAuth, headers, params })
  )
  setPluginStatus({ languageConfig })

  if (webhookBody && Object.keys(webhookBody).length) {
    const changesActivity = reporter.activityTimer(
      `loading Drupal content changes`,
      {
        parentSpan,
      }
    )
    changesActivity.start()

    try {
      const { secret, action, id, data } = webhookBody
      if (pluginOptions.secret && pluginOptions.secret !== secret) {
        reporter.warn(
          `The secret in this request did not match your plugin options secret.`
        )
        return
      }
      if (action === `delete`) {
        actions.deleteNode({ node: getNode(createNodeId(id)) })
        reporter.log(`Deleted node: ${id}`)
        return
      }


Review comment (suggestion):
Had to move default values further up in the process to allow access to them when fetching language configuration.

Code after:
    reporter,
    webhookBody,
  },
  pluginOptions
) => {
  let {
    baseUrl,
    apiBase,
    basicAuth,
    filters,
    headers,
    params,
    concurrentFileRequests,
    disallowedLinkTypes,
    skipFileDownloads,
    fastBuilds,
    languageConfig = {
      defaultLanguage: `und`,
      enabledLanguages: [`und`],
      translatableEntities: [],
    },
    entityReferenceRevisions = [],
  } = pluginOptions
  const { createNode, setPluginStatus, touchNode } = actions

  // Default apiBase to `jsonapi`
  apiBase = apiBase || `jsonapi`

  // Default disallowedLinkTypes to self, describedby.
  disallowedLinkTypes = disallowedLinkTypes || [`self`, `describedby`]

  // Default concurrentFileRequests to `20`
  concurrentFileRequests = concurrentFileRequests || 20

  // Default skipFileDownloads to false.
  skipFileDownloads = skipFileDownloads || false

  if (webhookBody && Object.keys(webhookBody).length) {
    const changesActivity = reporter.activityTimer(
      `loading Drupal content changes`,
      {
        parentSpan,
      }
    )
    changesActivity.start()

    try {
      const { secret, action, id, data } = webhookBody
      if (pluginOptions.secret && pluginOptions.secret !== secret) {
        reporter.warn(
          `The secret in this request did not match your plugin options secret.`
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-drupal/src/normalize.js (JavaScript)

Code before:
const { URL } = require(`url`)
const { createRemoteFileNode } = require(`gatsby-source-filesystem`)

const nodeFromData = (datum, createNodeId) => {
  const { attributes: { id: _attributes_id, ...attributes } = {} } = datum
  const preservedId =
    typeof _attributes_id !== `undefined` ? { _attributes_id } : {}
  return {
    id: createNodeId(`${datum.attributes.langcode}${datum.id}`),
    drupal_id: datum.id,
    parent: null,
    drupal_parent_menu_item: attributes.parent,
    children: [],
    ...attributes,
    ...preservedId,
    drupal_relationships: datum.relationships,
    relationships: {},
    internal: {
      type: datum.type.replace(/-|__|:|\.|\s/g, `_`),
    },
  }
}

exports.nodeFromData = nodeFromData

const isFileNode = node =>
  node.internal.type === `files` || node.internal.type === `file__file`

exports.isFileNode = isFileNode

exports.downloadFile = async (
  { node, store, cache, createNode, createNodeId, getCache, reporter },
  { basicAuth, baseUrl }
) => {

Review comment (security):
At first glance, it seems like all entities have a `attributes.langcode` attribute. However, I have found that for some reason JSONAPI does not always return this langcode value if you are browsing as an anonymous user. For an example try going to `/jsonapi/date_format/date_format` as an authenticated and an anonymous user. For me I see the langcode when logged in but not when anonymous. This breaks builds if the user is not using authentication.

The simple workaround here is to just check if the langcode exists and if it doesn't, default to `und`

Code after:
const { URL } = require(`url`)
const { createRemoteFileNode } = require(`gatsby-source-filesystem`)

const { getOptions } = require(`./plugin-options`)
const getHref = link => {
  if (typeof link === `object`) {
    return link.href
  }
  return link
}

exports.getHref = getHref

const nodeFromData = (datum, createNodeId, entityReferenceRevisions = []) => {
  const { attributes: { id: attributeId, ...attributes } = {} } = datum
  const preservedId =
    typeof attributeId !== `undefined` ? { _attributes_id: attributeId } : {}
  const langcode = datum.attributes.langcode || `und`
  return {
    id: createNodeId(
      createNodeIdWithVersion(
        datum.id,
        datum.type,
        langcode,
        attributes.drupal_internal__revision_id,
        entityReferenceRevisions
      )
    ),
    drupal_id: datum.id,
    parent: null,
    drupal_parent_menu_item: attributes.parent,
    children: [],
    ...attributes,
    ...preservedId,
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-drupal/src/utils.js (JavaScript)

Code before:
const _ = require(`lodash`)
const axios = require(`axios`)

const {
  nodeFromData,
  downloadFile,
  isFileNode,
  getHref,
  createNodeIdWithVersion,
} = require(`./normalize`)

const { getOptions } = require(`./plugin-options`)

const backRefsNamesLookup = new WeakMap()
const referencedNodesLookup = new WeakMap()

const fetchLanguageConfig = async ({
  translation,
  baseUrl,
  apiBase,
  basicAuth,
  headers,
  params,
  reporter,
}) => {
  const defaultLanguageConfig = {
    defaultLanguage: `und`,
    enabledLanguages: [`und`],
    translatableEntities: [],
  }

  if (!translation) {
    return defaultLanguageConfig
  }

  try {
    let next = `${baseUrl}/${apiBase}/configurable_language/configurable_language?sort=weight`
    let availableLanguagesResponses = []
    let translatableEntitiesResponses = []

    while (next) {
      const response = await axios.get(next, {
        auth: basicAuth,
        headers,
        params,
      })

      availableLanguagesResponses = availableLanguagesResponses.concat(
        response.data.data
      )
      next = getHref(response.data.links.next)

Review comment (security):
This could all be passed via options instead of being resolved during the build. It would require documentation but means less code and more flexibility. Requiring a Drupal user is especially impactful on larger sites which would benefit greatly from edge caching like Varnish but are forced to bypass it because they are using Basic Auth to access language config.

Code after:
const _ = require(`lodash`)
const axios = require(`axios`)

const {
  nodeFromData,
  downloadFile,
  isFileNode,
  getHref,
  createNodeIdWithVersion,
} = require(`./normalize`)

const { getOptions } = require(`./plugin-options`)

const backRefsNamesLookup = new WeakMap()
const referencedNodesLookup = new WeakMap()

const handleReferences = (
  node,
  { getNode, createNodeId, entityReferenceRevisions = [] }
) => {
  const relationships = node.relationships
  const rootNodeLanguage = getOptions().languageConfig ? node.langcode : `und`

  if (node.drupal_relationships) {
    const referencedNodes = []
    _.each(node.drupal_relationships, (v, k) => {
      if (!v.data) return
      const nodeFieldName = `${k}___NODE`
      if (_.isArray(v.data)) {
        relationships[nodeFieldName] = _.compact(
          v.data.map(data => {
            const referencedNodeId = createNodeId(
              createNodeIdWithVersion(
                data.id,
                data.type,
                rootNodeLanguage,
                data.meta?.target_version,
                entityReferenceRevisions
              )
            )
            if (!getNode(referencedNodeId)) {
              return null
            }

            referencedNodes.push(referencedNodeId)
            return referencedNodeId
          })
        )

        const meta = _.compact(
          v.data.map(data => (!_.isEmpty(data.meta) ? data.meta : null))
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-filesystem/src/extend-file-node.js (JavaScript)

Code before:

  return {
    publicURL: {
      type: GraphQLString,
      args: {},
      description: `Copy file to static directory and return public url to it`,
      resolve: (file, fieldArgs, context) => {
        const details = getNodeAndSavePathDependency(file.id, context.path)
        const fileName = `${file.internal.contentDigest}/${details.base}`

        const publicPath = path.join(
          process.cwd(),
          `public`,
          `static`,
          fileName
        )

        if (!fs.existsSync(publicPath)) {
          fs.copy(
            details.absolutePath,
            publicPath,
            { dereference: true },
            err => {
              if (err) {
                // Capture error within new Reporter API
                if (reporter.setErrorMap) {
                  reporter.panic({
                    id: `TBD`,
                    error: err
                  },
                  err)
                } else {  // "old" error handling (except, now, we use `reporter.panic` instead of `console.error`)
                  reporter.panic(`error copying file from ${details.absolutePath} to ${publicPath}`)
                }
              }
            }
          )
        }

        return `${pathPrefix}/static/${fileName}`
      },
    },
  }
}


Review comment (suggestion):
I think you can call it in a backwards compatible way like this:

```js
reporter.panic({
  id: `TBD`,
  context: {
    sourceMessage: `error copying file from ${details.absolutePath} to ${publicPath}`,
  }
}, err)
```

Code after:
  pathPrefix = ``,
  reporter,
}) => {
  if (type.name !== `File`) {
    return {}
  }

  return {
    publicURL: {
      type: GraphQLString,
      args: {},
      description: `Copy file to static directory and return public url to it`,
      resolve: (file, fieldArgs, context) => {
        const details = getNodeAndSavePathDependency(file.id, context.path)
        const fileName = `${file.internal.contentDigest}/${details.base}`

        const publicPath = path.join(
          process.cwd(),
          `public`,
          `static`,
          fileName
        )

        if (!fs.existsSync(publicPath)) {
          fs.copy(
            details.absolutePath,
            publicPath,
            { dereference: true },
            err => {
              if (err) {
                reporter.panic(
                  {
                    id: prefixId(CODES.MissingResource),
                    context: {
                      sourceMessage: `error copying file from ${details.absolutePath} to ${publicPath}`,
                    },
                  },
                  err
                )
              }
            }
          )
        }

        return `${pathPrefix}/static/${fileName}`
      },
    },
  }
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-transformer-sharp/src/customize-schema.js (JavaScript)

Code before:
  GraphQLNonNull,
} = require(`gatsby/graphql`)
const {
  queueImageResizing,
  base64,
  fluid,
  fixed,
  traceSVG,
} = require(`gatsby-plugin-sharp`)

const sharp = require(`./safe-sharp`)
const fs = require(`fs-extra`)
const imageSize = require(`probe-image-size`)
const path = require(`path`)

const DEFAULT_PNG_COMPRESSION_SPEED = 4

const {
  ImageFormatType,
  ImageCropFocusType,
  DuotoneGradientType,
  PotraceTurnPolicyType,
  PotraceType,
  ImageFitType,
} = require(`./types`)
const { report } = require("process")

function toArray(buf) {
  const arr = new Array(buf.length)

  for (let i = 0; i < buf.length; i++) {
    arr[i] = buf[i]
  }

  return arr
}

const getTracedSVG = async ({ file, image, fieldArgs, cache, reporter }) =>
  traceSVG({
    file,
    args: { ...fieldArgs.traceSVG },
    fileArgs: fieldArgs,
    cache,
    reporter,
  })

const fixedNodeType = ({
  pathPrefix,
  getNodeAndSavePathDependency,
  reporter,
  name,

Review comment (suggestion):
```suggestion
```

Seems like a stray import. You'll use the passed in `reporter` for the call :)

Code after:
  GraphQLNonNull,
} = require(`gatsby/graphql`)
const {
  queueImageResizing,
  base64,
  fluid,
  fixed,
  traceSVG,
} = require(`gatsby-plugin-sharp`)

const sharp = require(`./safe-sharp`)
const fs = require(`fs-extra`)
const imageSize = require(`probe-image-size`)
const path = require(`path`)

const DEFAULT_PNG_COMPRESSION_SPEED = 4

const {
  ImageFormatType,
  ImageCropFocusType,
  DuotoneGradientType,
  PotraceTurnPolicyType,
  PotraceType,
  ImageFitType,
} = require(`./types`)
const { prefixId, CODES } = require(`./error-utils`)

function toArray(buf) {
  const arr = new Array(buf.length)

  for (let i = 0; i < buf.length; i++) {
    arr[i] = buf[i]
  }

  return arr
}

const getTracedSVG = async ({ file, image, fieldArgs, cache, reporter }) =>
  traceSVG({
    file,
    args: { ...fieldArgs.traceSVG },
    fileArgs: fieldArgs,
    cache,
    reporter,
  })

const fixedNodeType = ({
  pathPrefix,
  getNodeAndSavePathDependency,
  reporter,
  name,
Repository: gatsbyjs/gatsby
File: packages/gatsby-source-filesystem/src/error-utils.js (JavaScript)

Code before:
export const CODES = {
  Generic: `10000`,
  MissingResource: `10001`,
}

export const pluginPrefix = `gatsby-source-filesystem`

export function prefixId(id) {
  return `${pluginPrefix}_${id}`
}

// TODO: Refactor to use contextual data instead of only context.sourceMessage
// once reporter.setErrorMap is guaranteed to be available
export const ERROR_MAP = {
  [CODES.Generic]: {
    text: context => `FileSystem: ${context.sourceMessage}`,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => `FileSystem: ${context.sourceMessage}`,
    level: `ERROR`,
    type: `PLUGIN`,
    category: `USER`,
  },
}


Review comment (suggestion):
```suggestion
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => context.sourceMessage,
```

We won't need the message prefix as this was only added in `gatsby-plugin-page-creator` as it's an internal plugin used by default and it makes it easier to see where it's coming from.

This plugin is something you explicitly add and the ID will also have the prefix, so we can do it like this 👍 

Code after:
export const CODES = {
  Generic: `10000`,
  MissingResource: `10001`,
}

export const pluginPrefix = `gatsby-source-filesystem`

export function prefixId(id) {
  return `${pluginPrefix}_${id}`
}

// TODO: Refactor to use contextual data instead of only context.sourceMessage
// once reporter.setErrorMap is guaranteed to be available
export const ERROR_MAP = {
  [CODES.Generic]: {
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
    category: `USER`,
  },
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-transformer-sharp/src/error-utils.js (JavaScript)

Code before:
export const CODES = {
  Generic: `20000`,
  MissingResource: `20001`,
}

export const pluginPrefix = `gatsby-transformer-sharp`

export function prefixId(id) {
  return `${pluginPrefix}_${id}`
}

// TODO: Refactor to use contextual data instead of only context.sourceMessage
// once reporter.setErrorMap is guaranteed to be available
export const ERROR_MAP = {
  [CODES.Generic]: {
    text: context => `TransformerSharp: ${context.sourceMessage}`,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => `TransformerSharp: ${context.sourceMessage}`,
    level: `ERROR`,
    type: `PLUGIN`,
    category: `USER`,
  },
}


Review comment (suggestion):
```suggestion
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => context.sourceMessage,
```

Same as above

Code after:
export const CODES = {
  Generic: `20000`,
  MissingResource: `20001`,
}

export const pluginPrefix = `gatsby-transformer-sharp`

export function prefixId(id) {
  return `${pluginPrefix}_${id}`
}

// TODO: Refactor to use contextual data instead of only context.sourceMessage
// once reporter.setErrorMap is guaranteed to be available
export const ERROR_MAP = {
  [CODES.Generic]: {
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
  },
  [CODES.MissingResource]: {
    text: context => context.sourceMessage,
    level: `ERROR`,
    type: `PLUGIN`,
    category: `USER`,
  },
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-transformer-sharp/src/gatsby-node.js (JavaScript)

Code before:
const {
  onCreateNode,
  unstable_shouldOnCreateNode,
} = require(`./on-node-create`)
exports.onPreInit = (context, options) => {
  const { reporter } = context
  if (reporter.setErrorMap) {
    reporter.setErrorMap(ERROR_MAP)
  }
}
exports.onCreateNode = onCreateNode
exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
exports.createSchemaCustomization = require(`./customize-schema`)
exports.createResolvers = require(`./create-resolvers`)


Review comment (suggestion):
[nitpick]
We usually do it like following:

```suggestion
exports.onPreInit = ({ reporter }) => {
  if (reporter.setErrorMap) {
    reporter.setErrorMap(ERROR_MAP)
  }
}
```

Code after:
const {
  onCreateNode,
  unstable_shouldOnCreateNode,
} = require(`./on-node-create`)
const { ERROR_MAP } = require(`./error-utils`)

exports.onPreInit = ({ reporter }) => {
  if (reporter.setErrorMap) {
    reporter.setErrorMap(ERROR_MAP)
  }
}
exports.onCreateNode = onCreateNode
exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
exports.createSchemaCustomization = require(`./customize-schema`)
exports.createResolvers = require(`./create-resolvers`)