ronantakizawa/codereview-bench huggingface dataset:
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/gatsby-webpack-virtual-modules.ts (TypeScript)

Code before:
import VirtualModulesPlugin from "webpack-virtual-modules"

interface IGatsbyWebpackVirtualModulesContext {
  writeModule: VirtualModulesPlugin["writeModule"]
}

const fileContentLookup: Record<string, string> = {}
const instances: IGatsbyWebpackVirtualModulesContext[] = []

export class GatsbyWebpackVirtualModules {
  apply(compiler): void {
    const virtualModules = new VirtualModulesPlugin(fileContentLookup)
    virtualModules.apply(compiler)
    instances.push({
      writeModule: virtualModules.writeModule.bind(virtualModules),
    })
  }
}

export function writeModule(filePath: string, fileContents: string): void {
  // "node_modules" added in front of filePath allow to allow importing
  // those modules using same path
  const adjustedFilePath = `node_modules/${filePath}`

  if (fileContentLookup[adjustedFilePath] === fileContents) {
    // we already have this, no need to cause invalidation
    return
  }

  fileContentLookup[adjustedFilePath] = fileContents

  instances.forEach(instance => {
    instance.writeModule(adjustedFilePath, fileContents)
  })
}


Review comment (suggestion):
This is really cool. i would love to have a header comment explaining what this module can do so in the future when a dev is reading this can know in what cases they might want to reuse this!

Code after:
export class GatsbyWebpackVirtualModules {
  apply(compiler): void {
    const virtualModules = new VirtualModulesPlugin(fileContentLookup)
    virtualModules.apply(compiler)
    instances.push({
      writeModule: virtualModules.writeModule.bind(virtualModules),
    })
  }
}

export function writeModule(filePath: string, fileContents: string): void {
  // "node_modules" added in front of filePath allow to allow importing
  // those modules using same path
  const adjustedFilePath = `node_modules/${filePath}`

  if (fileContentLookup[adjustedFilePath] === fileContents) {
    // we already have this, no need to cause invalidation
    return
  }

  fileContentLookup[adjustedFilePath] = fileContents

  instances.forEach(instance => {
    instance.writeModule(adjustedFilePath, fileContents)
  })
}
Repository: gatsbyjs/gatsby
File: packages/gatsby/cache-dir/app.js (JavaScript)

Code before:
import React from "react"
import ReactDOM from "react-dom"
import domReady from "@mikaelkristiansson/domready"
import io from "socket.io-client"

import socketIo from "./socketIo"
import emitter from "./emitter"
import { apiRunner, apiRunnerAsync } from "./api-runner-browser"
import { setLoader, publicLoader } from "./loader"
import DevLoader from "./dev-loader"
import syncRequires from "sync-requires"
// Generated during bootstrap
import matchPaths from "match-paths.json"

window.___emitter = emitter

const loader = new DevLoader(syncRequires, matchPaths)
setLoader(loader)
loader.setApiRunner(apiRunner)

window.___loader = publicLoader

// Let the site/plugins run code very early.
apiRunnerAsync(`onClientEntry`).then(() => {
  // Hook up the client to socket.io on server
  const socket = socketIo()
  if (socket) {
    socket.on(`reload`, () => {
      window.location.reload()
    })
  }

  fetch(`/___services`)
    .then(res => res.json())
    .then(services => {
      if (services.developstatusserver) {

Review comment (suggestion):
Naming should be different, this can clash with other node-module names. Maybe make it part of gatsby package 🤷 
```suggestion
import syncRequires from "gatsby/virtual/sync-requires.js"
```

Code after:
import React from "react"
import ReactDOM from "react-dom"
import domReady from "@mikaelkristiansson/domready"
import io from "socket.io-client"

import socketIo from "./socketIo"
import emitter from "./emitter"
import { apiRunner, apiRunnerAsync } from "./api-runner-browser"
import { setLoader, publicLoader } from "./loader"
import DevLoader from "./dev-loader"
import syncRequires from "$virtual/sync-requires"
// Generated during bootstrap
import matchPaths from "$virtual/match-paths.json"

window.___emitter = emitter

const loader = new DevLoader(syncRequires, matchPaths)
setLoader(loader)
loader.setApiRunner(apiRunner)

window.___loader = publicLoader

// Let the site/plugins run code very early.
apiRunnerAsync(`onClientEntry`).then(() => {
  // Hook up the client to socket.io on server
  const socket = socketIo()
  if (socket) {
    socket.on(`reload`, () => {
      window.location.reload()
    })
  }

  fetch(`/___services`)
    .then(res => res.json())
    .then(services => {
      if (services.developstatusserver) {
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/bootstrap/create-graphql-runner.ts (TypeScript)

Code before:
    parentSpan,
    graphqlTracing,
  }: { parentSpan: Span | undefined; graphqlTracing?: boolean } = {
    parentSpan: undefined,
    graphqlTracing: false,
  }
): Runner => {
  // TODO: Move tracking of changed state inside GraphQLRunner itself. https://github.com/gatsbyjs/gatsby/issues/20941
  let runner = new GraphQLRunner(store, { graphqlTracing })

  const eventTypes: string[] = [
    `DELETE_CACHE`,
    `CREATE_NODE`,
    `DELETE_NODE`,
    `DELETE_NODES`,
    `SET_SCHEMA_COMPOSER`,
    `SET_SCHEMA`,
    `ADD_FIELD_TO_NODE`,
    `ADD_CHILD_NODE_TO_PARENT_NODE`,
  ]

  eventTypes.forEach(type => {
    emitter.on(type, () => {
      runner = new GraphQLRunner(store)
    })
  })

  return (query, context): ReturnType<Runner> =>
    runner
      .query(query, context, {
        queryName: `gatsby-node query`,
        parentSpan,
      })
      .then(result => {
        if (result.errors) {
          const structuredErrors = result.errors
            .map(e => {
              // Find the file where graphql was called.
              const file = stackTrace
                .parse(e)
                .find(file => /createPages/.test(file.getFunctionName()))

              if (file) {
                const structuredError = errorParser({
                  message: e.message,
                  location: {
                    start: {
                      line: file.getLineNumber(),
                      column: file.getColumnNumber(),
                    },
                  },

Review comment (bug):
I hope this doesn't lead to race conditions where the runner is undefined between firing certain events but where other parts expect it to exist anyways.

Code after:
    parentSpan,
    graphqlTracing,
  }: { parentSpan: Span | undefined; graphqlTracing?: boolean } = {
    parentSpan: undefined,
    graphqlTracing: false,
  }
): Runner => {
  // TODO: Move tracking of changed state inside GraphQLRunner itself. https://github.com/gatsbyjs/gatsby/issues/20941
  let runner: GraphQLRunner | undefined = new GraphQLRunner(store, {
    graphqlTracing,
  })

  const eventTypes: string[] = [
    `DELETE_CACHE`,
    `CREATE_NODE`,
    `DELETE_NODE`,
    `DELETE_NODES`,
    `SET_SCHEMA_COMPOSER`,
    `SET_SCHEMA`,
    `ADD_FIELD_TO_NODE`,
    `ADD_CHILD_NODE_TO_PARENT_NODE`,
  ]

  eventTypes.forEach(type => {
    emitter.on(type, () => {
      runner = undefined
    })
  })

  return (query, context): ReturnType<Runner> => {
    if (!runner) {
      runner = new GraphQLRunner(store, {
        graphqlTracing,
      })
    }
    return runner
      .query(query, context, {
        queryName: `gatsby-node query`,
        parentSpan,
      })
      .then(result => {
        if (result.errors) {
          const structuredErrors = result.errors
            .map(e => {
              // Find the file where graphql was called.
              const file = stackTrace
                .parse(e)
                .find(file => /createPages/.test(file.getFunctionName()))

              if (file) {
                const structuredError = errorParser({
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/db/nodes.js (JavaScript)

Code before:

/**
 * Get content for a node from the plugin that created it.
 *
 * @param {IGatsbyNode} node
 * @returns {Promise<string>}
 */
async function loadNodeContent(node) {
  if (_.isString(node.internal.content)) {
    return Promise.resolve(node.internal.content)
  }

  // Load plugin's loader function
  const plugin = store
    .getState()
    .flattenedPlugins.find(plug => plug.name === node.internal.owner)

  const { loadNodeContent } = require(plugin.resolve)

  if (!loadNodeContent) {
    throw new Error(
      `Could not find function loadNodeContent for plugin ${plugin.name}`
    )
  }

  const content = loadNodeContent(node)

  node.internal.context = content

  return content
}

module.exports = {
  ...nodesDb,
  runQuery: runFastFiltersAndSort,
  loadNodeContent,
}


Review comment (suggestion):
`loadNodeContent` function is expected to be async

```suggestion
  const content = await loadNodeContent(node)
```

Code after:

/**
 * Get content for a node from the plugin that created it.
 *
 * @param {IGatsbyNode} node
 * @returns {Promise<string>}
 */
async function loadNodeContent(node) {
  if (_.isString(node.internal.content)) {
    return node.internal.content
  }

  // Load plugin's loader function
  const plugin = store
    .getState()
    .flattenedPlugins.find(plug => plug.name === node.internal.owner)

  const { loadNodeContent } = require(plugin.resolve)

  if (!loadNodeContent) {
    throw new Error(
      `Could not find function loadNodeContent for plugin ${plugin.name}`
    )
  }

  const content = await loadNodeContent(node)

  node.internal.content = content

  return content
}

module.exports = {
  ...nodesDb,
  runQuery: runFastFiltersAndSort,
  loadNodeContent,
}
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/state-machines/data-layer/actions.ts (TypeScript)

Code before:
import {
  assign,
  AnyEventObject,
  ActionFunction,
  AssignAction,
  DoneInvokeEvent,
  ActionFunctionMap,
} from "xstate"
import { createGraphQLRunner } from "../../bootstrap/create-graphql-runner"
import reporter from "gatsby-cli/lib/reporter"
import { IDataLayerContext } from "./types"

const concatUnique = <T>(array1?: T[], array2?: T[]): T[] =>
  Array.from(new Set((array1 || []).concat(array2 || [])))

type BuildMachineAction =
  | ActionFunction<IDataLayerContext, any>
  | AssignAction<IDataLayerContext, any>

export const assignChangedPages: BuildMachineAction = assign<
  IDataLayerContext,
  DoneInvokeEvent<{
    changedPages: string[]
    deletedPages: string[]
  }>
>((context, event) => {
  return {
    pagesToBuild: concatUnique(context.pagesToBuild, event.data.changedPages),
    pagesToDelete: concatUnique(context.pagesToDelete, event.data.deletedPages),
  }
})

export const assignGatsbyNodeGraphQL: BuildMachineAction = assign<
  IDataLayerContext
>({
  gatsbyNodeGraphQLFunction: ({ store }: IDataLayerContext) =>
    store ? createGraphQLRunner(store, reporter) : undefined,
})


Review comment (suggestion):
```suggestion
const concatUnique = <T>(array1: T[] = [], array2: T[] = []): T[] =>
  Array.from(new Set(array1.concat(array2)))
```

Code after:
import {
  assign,
  AnyEventObject,
  ActionFunction,
  AssignAction,
  DoneInvokeEvent,
  ActionFunctionMap,
} from "xstate"
import { createGraphQLRunner } from "../../bootstrap/create-graphql-runner"
import reporter from "gatsby-cli/lib/reporter"
import { IDataLayerContext } from "./types"
import { assertStore } from "../../utils/assert-store"

const concatUnique = <T>(array1: T[] = [], array2: T[] = []): T[] =>
  Array.from(new Set(array1.concat(array2)))

type BuildMachineAction =
  | ActionFunction<IDataLayerContext, any>
  | AssignAction<IDataLayerContext, any>

export const assignChangedPages: BuildMachineAction = assign<
  IDataLayerContext,
  DoneInvokeEvent<{
    changedPages: string[]
    deletedPages: string[]
  }>
>((context, event) => {
  return {
    pagesToBuild: concatUnique(context.pagesToBuild, event.data.changedPages),
    pagesToDelete: concatUnique(context.pagesToDelete, event.data.deletedPages),
  }
})

export const assignGatsbyNodeGraphQL: BuildMachineAction = assign<
  IDataLayerContext
>({
  gatsbyNodeGraphQLFunction: ({ store }: IDataLayerContext) => {
    assertStore(store)
    return createGraphQLRunner(store, reporter)
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/query/queue.js (JavaScript)

Code before:
    merge: (oldTask, newTask, cb) => {
      cb(null, newTask)
    },
  }

  const handler = ({ job: queryJob, activity }, callback) => {
    queryRunner(getRunner(), queryJob, activity?.span).then(
      result => {
        if (!queryJob.isPage) {
          websocketManager.emitStaticQueryData({
            result,
            id: queryJob.id,
          })
        }

        callback(null, result)
      },
      error => callback(error)
    )
  }

  return new Queue(handler, queueOptions)
}

/**
 * Returns a promise that pushes jobs onto queue and resolves onces
 * they're all finished processing (or rejects if one or more jobs
 * fail)
 * Note: queue is reused in develop so make sure to thoroughly cleanup hooks
 */
const processBatch = async (queue, jobs, activity) => {
  if (jobs.length === 0) {
    return Promise.resolve()
  }

  return new Promise((resolve, reject) => {
    let taskFinishCallback
    if (activity.tick) {
      taskFinishCallback = () => activity.tick()
      queue.on(`task_finish`, taskFinishCallback)
    }

    const taskFailedCallback = (...err) => {
      gc()
      reject(err)
    }

    const drainCallback = () => {
      gc()
      resolve()
    }

Review comment (suggestion):
I don't think we should rely on NODE_ENV in this situation, but rather it should be passed from the program.

Code after:
    merge: (oldTask, newTask, cb) => {
      cb(null, newTask)
    },
  }

  const handler = ({ job: queryJob, activity }, callback) => {
    queryRunner(getRunner(), queryJob, activity?.span).then(
      result => {
        if (!queryJob.isPage) {
          websocketManager.emitStaticQueryData({
            result,
            id: queryJob.id,
          })
        }

        callback(null, result)
      },
      error => callback(error)
    )
  }

  return new Queue(handler, queueOptions)
}

const createAppropriateQueue = (graphqlRunner, runnerOptions = {}) => {
  if (process.env.NODE_ENV === `production`) {
    return createBuildQueue(graphqlRunner, runnerOptions)
  }
  if (!graphqlRunner) {
    graphqlRunner = new GraphQLRunner(store, runnerOptions)
  }
  return createDevelopQueue(() => graphqlRunner)
}

/**
 * Returns a promise that pushes jobs onto queue and resolves onces
 * they're all finished processing (or rejects if one or more jobs
 * fail)
 * Note: queue is reused in develop so make sure to thoroughly cleanup hooks
 */
const processBatch = async (queue, jobs, activity) => {
  if (jobs.length === 0) {
    return Promise.resolve()
  }

  return new Promise((resolve, reject) => {
    let taskFinishCallback
    if (activity.tick) {
      taskFinishCallback = () => activity.tick()
      queue.on(`task_finish`, taskFinishCallback)
    }
Repository: gatsbyjs/gatsby
File: packages/gatsby-cli/src/reporter/prepare-stack-trace.ts (TypeScript)

Code before:
/* Code borrowed and based on
 * https://github.com/evanw/node-source-map-support/blob/master/source-map-support.js
 */

import { readFileSync } from "fs"
import { codeFrameColumns } from "@babel/code-frame"
import stackTrace from "stack-trace"
import {
  SourceMapConsumer,
  BasicSourceMapConsumer,
  IndexedSourceMapConsumer,
  NullableMappedPosition,
} from "source-map"

export class ErrorWithCodeFrame extends Error {
  codeFrame = ``

  constructor(error) {
    super(error)

    Object.getOwnPropertyNames(error).forEach(key => {
      this[key] = error[key]
    })
  }
}

export async function prepareStackTrace(
  error: Error,
  source: string
): Promise<ErrorWithCodeFrame> {
  const newError = new ErrorWithCodeFrame(error)
  const map = await new SourceMapConsumer(readFileSync(source, `utf8`))
  const stack = stackTrace
    .parse(newError)
    .map(frame => wrapCallSite(map, frame))
    .filter(
      frame =>
        `wasConverted` in frame &&
        (!frame.getFileName() ||
          !frame
            .getFileName()

Review comment (suggestion):
Similar as below
```suggestion
  codeFrame: string = ``
```

Code after:
/* Code borrowed and based on
 * https://github.com/evanw/node-source-map-support/blob/master/source-map-support.js
 */

import { readFileSync } from "fs"
import { codeFrameColumns } from "@babel/code-frame"
import stackTrace from "stack-trace"
import {
  SourceMapConsumer,
  BasicSourceMapConsumer,
  IndexedSourceMapConsumer,
  NullableMappedPosition,
} from "source-map"

export class ErrorWithCodeFrame extends Error {
  codeFrame = ``

  constructor(error: Error) {
    super(error.message)

    // We must use getOwnProperty because keys like `stack` are not enumerable,
    // but we want to copy over the entire error
    Object.getOwnPropertyNames(error).forEach(key => {
      this[key] = error[key]
    })
  }
}

export async function prepareStackTrace(
  error: Error,
  source: string
): Promise<ErrorWithCodeFrame> {
  const newError = new ErrorWithCodeFrame(error)
  const map = await new SourceMapConsumer(readFileSync(source, `utf8`))
  const stack = stackTrace
    .parse(newError)
    .map(frame => wrapCallSite(map, frame))
    .filter(
      frame =>
        `wasConverted` in frame &&
        (!frame.getFileName() ||
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/query/queue.ts (TypeScript)

Code before:
import Queue from "better-queue"
import { store } from "../redux"
import { MemoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store"
import { queryRunner } from "../query/query-runner"
import { websocketManager } from "../utils/websocket-manager"
import { GraphQLRunner } from "./graphql-runner"
import BetterQueue from "better-queue"
import { IExecutionResult } from "./types"
import { ProgressActivityTracker } from "../.."

export type Task = any
type TaskResult = any

const createBaseOptions = (): Partial<
  BetterQueue.QueueOptions<Task, TaskResult>
> => {
  return {
    concurrent: Number(process.env.GATSBY_EXPERIMENTAL_QUERY_CONCURRENCY) || 4,
    // eslint-disable-next-line new-cap
    store: MemoryStoreWithPriorityBuckets<Task>(),
  }
}

const createBuildQueue = (
  graphqlRunner: GraphQLRunner,
  runnerOptions = {}
): Queue => {
  if (!graphqlRunner) {
    graphqlRunner = new GraphQLRunner(store, runnerOptions)
  }
  const handler = ({ job, activity }, callback): Promise<IExecutionResult> =>
    queryRunner(graphqlRunner, job, activity?.span)
      .then(result => callback(null, result))
      .catch(callback)
  const queue = new Queue(handler, createBaseOptions())
  return queue

Review comment (suggestion):
I couldn't locate a better type, but at least we can just change these lines if one exists.

Code after:
import Queue from "better-queue"
import { store } from "../redux"
import { memoryStoreWithPriorityBuckets } from "../query/better-queue-custom-store"
import { queryRunner } from "../query/query-runner"
import { websocketManager } from "../utils/websocket-manager"
import { GraphQLRunner, IGraphQLRunnerOptions } from "./graphql-runner"
import BetterQueue from "better-queue"
import { ProgressActivityTracker } from "../.."

export type Task = any
type TaskResult = any

const createBaseOptions = (): Pick<
  BetterQueue.QueueOptions<Task, TaskResult>,
  "concurrent" | "store"
> => {
  return {
    concurrent: Number(process.env.GATSBY_EXPERIMENTAL_QUERY_CONCURRENCY) || 4,
    store: memoryStoreWithPriorityBuckets<Task>(),
  }
}

const createBuildQueue = (
  graphqlRunner: GraphQLRunner,
  runnerOptions: IGraphQLRunnerOptions = {}
): Queue => {
  if (!graphqlRunner) {
    graphqlRunner = new GraphQLRunner(store, runnerOptions)
  }

  const queueOptions: BetterQueue.QueueOptions<Task, TaskResult> = {
    ...createBaseOptions(),
    process: ({ job, activity }, callback): void => {
      queryRunner(graphqlRunner, job, activity?.span)
        .then(result => callback(null, result))
        .catch(callback)
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/query/better-queue-custom-store.ts (TypeScript)

Code before:
import { Store } from "better-queue"

// getRunningTasks is an extension to the interface, and is used in the tests
interface IGatsbyBetterStore<T> extends Store<T> {
  getRunningTasks(cb: (error: any, runningTasks: any) => void): void
}

export function MemoryStoreWithPriorityBuckets<T>(): IGatsbyBetterStore<T> {
  type RunningTasks = Record<string, T>
  let uuid = 0

  /**
   * Task ids grouped by priority
   */
  const queueMap = new Map<number, string[]>()

  /**
   * Task id to task lookup
   */
  const tasks = new Map<string, T>()

  /**
   * Task id to priority lookup
   */
  const taskIdToPriority = new Map<string, number>()

  /**
   * Lock to running tasks object
   */

Review comment (suggestion):
Obviously, this goes against the guidelines of not using Generics, but if there's a good type for the Tasks then this can be updated.

Code after:
import { Store } from "better-queue"

export function memoryStoreWithPriorityBuckets<T>(): Store<T> {
  type RunningTasks = Record<string, T>
  let uuid = 0

  /**
   * Task ids grouped by priority
   */
  const queueMap = new Map<number, Array<string>>()

  /**
   * Task id to task lookup
   */
  const tasks = new Map<string, T>()

  /**
   * Task id to priority lookup
   */
  const taskIdToPriority = new Map<string, number>()

  /**
   * Lock to running tasks object
   */
  const running: Record<string, RunningTasks> = {}

  let priorityKeys: Array<number> = []
  const updatePriorityKeys = (): void => {
    priorityKeys = Array.from(queueMap.keys()).sort((a, b) => b - a)