ronantakizawa/codereview-bench huggingface dataset:
Repository: gatsbyjs/gatsby
File: packages/gatsby-plugin-preact/src/gatsby-node.js (JavaScript)

Code before:
const PreactRefreshPlugin = require(`@prefresh/webpack`)

exports.onPreInit = () => {
  // force fast-refresh in gatsby
  process.env.GATSBY_HOT_LOADER = `fast-refresh`
}

exports.onCreateWebpackConfig = ({ stage, actions, getConfig }) => {
  const webpackPlugins = []
  if (stage === `develop`) {
    webpackPlugins.push(new PreactRefreshPlugin())

    // remove React refresh plugin, we want ot add preact refresh instead.
    const webpackConfig = getConfig()
    const reactRefreshPluginIndex = webpackConfig.plugins.findIndex(
      plugin => plugin.constructor.name === `ReactRefreshPlugin`
    )
    webpackConfig.plugins.splice(reactRefreshPluginIndex, 1)
    actions.replaceWebpackConfig(webpackConfig)

    // enable react-refresh babel plugin to enable hooks
    // @see https://github.com/JoviDeCroock/prefresh/tree/master/packages/webpack#using-hooks
    actions.setBabelPlugin({
      name: `react-refresh/babel`,
    })
  }

  actions.setWebpackConfig({
    resolve: {
      alias: {
        react: `preact/compat`,
        "react-dom": `preact/compat`,
      },
    },
    plugins: webpackPlugins,
  })
}


Review comment (suggestion):
NIT: maybe we could do `.filter` instead of `.findIndex` + `.splice`?
feel free to ignore if you find current one better

Code after:
const PreactRefreshPlugin = require(`@prefresh/webpack`)

exports.onPreInit = () => {
  // force fast-refresh in gatsby
  process.env.GATSBY_HOT_LOADER = `fast-refresh`
}

exports.onCreateWebpackConfig = ({ stage, actions, getConfig }) => {
  const webpackPlugins = []
  if (stage === `develop`) {
    webpackPlugins.push(new PreactRefreshPlugin())

    // remove React refresh plugin, we want to add preact refresh instead.
    const webpackConfig = getConfig()
    webpackConfig.plugins = webpackConfig.plugins.filter(
      plugin => plugin.constructor.name !== `ReactRefreshPlugin`
    )
    actions.replaceWebpackConfig(webpackConfig)

    // enable react-refresh babel plugin to enable hooks
    // @see https://github.com/JoviDeCroock/prefresh/tree/master/packages/webpack#using-hooks
    actions.setBabelPlugin({
      name: `react-refresh/babel`,
    })
  }

  actions.setWebpackConfig({
    resolve: {
      alias: {
        react: `preact/compat`,
        "react-dom": `preact/compat`,
      },
    },
    plugins: webpackPlugins,
  })
}
Repository: gatsbyjs/gatsby
File: packages/gatsby-plugin-preact/src/gatsby-node.js (JavaScript)

Code before:
const PreactRefreshPlugin = require(`@prefresh/webpack`)

exports.onPreInit = () => {
  // force fast-refresh in gatsby
  process.env.GATSBY_HOT_LOADER = `fast-refresh`
}

exports.onCreateWebpackConfig = ({ stage, actions, getConfig }) => {
  const webpackPlugins = []
  if (stage === `develop`) {
    webpackPlugins.push(new PreactRefreshPlugin())

    // remove React refresh plugin, we want to add preact refresh instead.
    const webpackConfig = getConfig()
    webpackConfig.plugins = webpackConfig.plugins.filter(
      plugin => plugin.constructor.name !== `ReactRefreshPlugin`
    )
    actions.replaceWebpackConfig(webpackConfig)

    // enable react-refresh babel plugin to enable hooks
    // @see https://github.com/JoviDeCroock/prefresh/tree/master/packages/webpack#using-hooks
    actions.setBabelPlugin({
      name: `react-refresh/babel`,
    })
  }

  actions.setWebpackConfig({
    resolve: {
      alias: {
        react: `preact/compat`,
        "react-dom": `preact/compat`,
      },
    },
    plugins: webpackPlugins,
  })
}


Review comment (performance):
Just a suggestion to avoid this breaking when the upstream regex changes:

```suggestion
      webpackConfig.optimization.splitChunks.cacheGroups.framework.test = [
        /(?<!node_modules.*)[\\/]node_modules[\\/](preact)[\\/]/
      ].concat(webpackConfig.optimization.splitChunks.cacheGroups.framework.test)
```

Code after:
    webpackConfig.plugins = webpackConfig.plugins.filter(
      plugin => plugin.constructor.name !== `ReactRefreshPlugin`
    )
    actions.replaceWebpackConfig(webpackConfig)

    // enable react-refresh babel plugin to enable hooks
    // @see https://github.com/JoviDeCroock/prefresh/tree/master/packages/webpack#using-hooks
    actions.setBabelPlugin({
      name: `react-refresh/babel`,
    })
  }

  // add preact to the framework bundle
  if (stage === `build-javascript`) {
    const webpackConfig = getConfig()
    if (
      webpackConfig?.optimization?.splitChunks?.cacheGroups?.framework?.test
    ) {
      const frameworkRegex =
        webpackConfig.optimization.splitChunks.cacheGroups.framework.test

      // replace react libs with preact
      webpackConfig.optimization.splitChunks.cacheGroups.framework.test = module =>
        /(?<!node_modules.*)[\\/]node_modules[\\/](preact)[\\/]/.test(
          module.resource
        ) || frameworkRegex.test(module.resource)

      actions.replaceWebpackConfig(webpackConfig)
    }
  }

  actions.setWebpackConfig({
    resolve: {
      alias: {
        react: `preact/compat`,
        "react-dom": `preact/compat`,
      },
    },
    plugins: webpackPlugins,
  })
}
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/query/graphql-runner.ts (TypeScript)

Code before:
    const document = this.parse(query)
    const errors = this.validate(schema, document)

    let tracer
    if (this.graphqlTracing && parentSpan) {
      tracer = new GraphQLSpanTracer(`GraphQL Query`, {
        parentSpan,
        tags: {
          queryName: queryName,
        },
      })

      tracer.start()
    }

    try {
      const result =
        errors.length > 0
          ? { errors }
          : execute({
              schema,
              document,
              rootValue: context,
              contextValue: withResolverContext({
                schema,
                schemaComposer: schemaCustomization.composer!,
                context,
                customContext: schemaCustomization.context,
                nodeModel: this.nodeModel,
                stats: this.stats,
                tracer,
              }),
              variableValues: context,
            })

      // Queries are usually executed in batch. But after the batch is finished
      // cache just wastes memory without much benefits.
      // TODO: consider a better strategy for cache purging/invalidation
      this.scheduleClearCache()

      return Promise.resolve(result)
    } finally {
      if (tracer) {
        tracer.end()
      }
    }
  }
}


Review comment (suggestion):
Can we avoid using non-null assertions if at all possible. If it's not really nullable then the type should be updated. If it _is_ nullable, then there should be a runtime check.

Code after:
          queryName: queryName,
        },
      })

      tracer.start()
    }

    try {
      const result =
        errors.length > 0
          ? { errors }
          : execute({
              schema,
              document,
              rootValue: context,
              contextValue: withResolverContext({
                schema,
                schemaComposer: schemaCustomization.composer,
                context,
                customContext: schemaCustomization.context,
                nodeModel: this.nodeModel,
                stats: this.stats,
                tracer,
              }),
              variableValues: context,
            })

      // Queries are usually executed in batch. But after the batch is finished
      // cache just wastes memory without much benefits.
      // TODO: consider a better strategy for cache purging/invalidation
      this.scheduleClearCache()

      return Promise.resolve(result)
    } finally {
      if (tracer) {
        tracer.end()
      }
    }
  }
}
Repository: gatsbyjs/gatsby
File: packages/gatsby/index.d.ts (TypeScript)

Code before:
  stripIndent: (input: string) => string
  format: object
  setVerbose(isVerbose?: boolean): void
  setNoColor(isNoColor?: boolean): void
  panic: LogErrorType
  panicOnBuild: LogErrorType
  error: LogErrorType
  uptime(prefix: string): void
  success: LogMessageType
  verbose: LogMessageType
  info: LogMessageType
  warn: LogMessageType
  log: LogMessageType
  activityTimer(name: string, activityArgs?: ActivityArgs): ActivityTracker
  createProgress(
    text: string,
    total?: number,
    start?: number,
    activityArgs?: ActivityArgs
  ): ProgressActivityTracker
}

export interface Cache {
  /**
   * Retrieve cached value
   * @param key Cache key
   * @returns Promise resolving to cached value
   * @example
   * const value = await cache.get(`unique-key`)
   */
  get(key: string): Promise<any>

  /**
   * Cache value
   * @param key Cache key
   * @param value Value to be cached
   * @returns Promise resolving to cached value
   * @example
   * await cache.set(`unique-key`, value)
   */
  set(key: string, value: any): Promise<any>
}

export interface Tracing {
  tracer: object
  parentSpan: object
  startSpan: Function
}

export interface PackageJson {
  name?: string

Review comment (suggestion):
This interface wasn't being used at all, so I don't think it's likely that this will cause a breaking change.

Code after:
  stripIndent: (input: string) => string
  format: object
  setVerbose(isVerbose?: boolean): void
  setNoColor(isNoColor?: boolean): void
  panic: LogErrorType
  panicOnBuild: LogErrorType
  error: LogErrorType
  uptime(prefix: string): void
  success: LogMessageType
  verbose: LogMessageType
  info: LogMessageType
  warn: LogMessageType
  log: LogMessageType
  activityTimer(name: string, activityArgs?: ActivityArgs): ActivityTracker
  createProgress(
    text: string,
    total?: number,
    start?: number,
    activityArgs?: ActivityArgs
  ): ProgressActivityTracker
}

/**
 * @deprecated Use `GatsbyCache` instead
 */
export interface Cache {
  name: string
  store: {
    create: Function
  }
  cache: {
    getAndPassUp: Function
    wrap: Function
    set: Function
    mset: Function
    get: Function
    mget: Function
    del: Function
    reset: Function
  }
}

export interface GatsbyCache {
  /**
   * Retrieve cached value
   * @param key Cache key
   * @returns Promise resolving to cached value
   * @example
   * const value = await cache.get(`unique-key`)
   */
  get(key: string): Promise<any>
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/commands/build-html.ts (TypeScript)

Code before:
  // const start = process.hrtime()
  const segments = chunk(pages, 50)

  await Bluebird.map(segments, async pageSegment => {
    await workerPool.renderHTML({
      envVars,
      htmlComponentRendererPath,
      paths: pageSegment,
    })

    if (activity && activity.tick) {
      activity.tick(pageSegment.length)
    }
  })
}

const doBuildPages = async (
  rendererPath: string,
  pagePaths: string[],
  activity: IActivity,
  workerPool: IWorkerPool
): Promise<void> => {
  telemetry.addSiteMeasurement(`BUILD_END`, {
    pagesCount: pagePaths.length,
  })

  try {
    await renderHTMLQueue(workerPool, activity, rendererPath, pagePaths)
  } catch (error) {
    const prettyError = await createErrorFromString(
      error.stack,
      `${rendererPath}.map`
    )
    prettyError.context = error.context
    throw prettyError
  }
}

export const buildHTML = async ({
  program,
  stage,
  pagePaths,
  activity,
  workerPool,
}: {
  program: IProgram
  stage: Stage
  pagePaths: string[]
  activity: IActivity
  workerPool: IWorkerPool
}): Promise<void> => {

Review comment (suggestion):
A bit late, but this is not copying `stack` property/value because `stack` is not enumarable, I think we want to use `getOwnPropertyNames` here? See snippet below for return values for `Object.keys()` and `Object.getOwnPropertyNames`:

```
keys [ 'name', 'context' ]
getOwnPropertyNames [ 'stack', 'message', 'name', 'context' ]
```

Effect this have is that SSR errors don't have real stack traces - instead they only show

```
  WebpackError: ReferenceError: window is not defined
  
  - build-html.js:111 doBuildPages
    [i25304]/[gatsby]/dist/commands/build-html.js:111:24
```

Without pointing to actual problem (because stack is newly created and not copied from original error)

Code after:
  // const start = process.hrtime()
  const segments = chunk(pages, 50)

  await Bluebird.map(segments, async pageSegment => {
    await workerPool.renderHTML({
      envVars,
      htmlComponentRendererPath,
      paths: pageSegment,
    })

    if (activity && activity.tick) {
      activity.tick(pageSegment.length)
    }
  })
}

class BuildHTMLError extends Error {
  codeFrame = ``
  context?: {
    path: string
  }

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

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

const doBuildPages = async (
  rendererPath: string,
  pagePaths: string[],
  activity: IActivity,
  workerPool: IWorkerPool
): Promise<void> => {
  telemetry.addSiteMeasurement(`BUILD_END`, {
    pagesCount: pagePaths.length,
  })

  try {
    await renderHTMLQueue(workerPool, activity, rendererPath, pagePaths)
  } catch (error) {
    const prettyError = await createErrorFromString(
      error.stack,
      `${rendererPath}.map`
    )
    const buildError = new BuildHTMLError(prettyError)
    buildError.context = error.context
    throw buildError
Repository: gatsbyjs/gatsby
File: www/src/components/sidebar/section-title.js (JavaScript)

Code before:
      css={{
        alignItems: `flex-end`,
        display: `flex`,
        position: `relative`,
        width: `100%`,
      }}
    >
      <span
        sx={{
          // borderRightWidth: "1px",
          // borderRightStyle: "solid",
          // borderRightColor: "sidebar.itemBorderColor"
          flexGrow: 1,
        }}
      >
        <ItemLink
          item={item}
          overrideCSS={{
            "&&": item.level === 0 && item.ui !== `steps` && styles.level0,
            pr: t => t.sizes.sidebarItemMinHeight,
          }}
        />
      </span>
      <button
        aria-controls={uid}
        aria-expanded={!!isExpanded}
        aria-label={
          isExpanded
            ? i18n._(t`${item.title} collapse`)
            : i18n._(t`${item.title} expand`)
        }
        sx={{
          ...styles.resetButton,
          bottom: 0,
          ml: `auto`,
          minHeight: `sidebarItemMinHeight`,
          position: `absolute`,
          right: 0,
          top: 0,
          width: `sidebarItemMinHeight`,
          zIndex: 1,
        }}
        onClick={() => onSectionTitleClick(item)}
      >
        <Chevron isExpanded={isExpanded} />
      </button>
    </span>
  )
})

export default function SectionTitle({ itemRef, item, uid }) {

Review comment (suggestion):
There's one more aria-expanded in `sectionTitle` that could use this.

Alternately, in the line above where `isExpanded` is called you can do

```js
// used to set `aria-expanded`, so needs to be a boolean value.
const { isExpanded = false } = getItemState(item)
```

Code after:
        position: `relative`,
        width: `100%`,
      }}
    >
      <span
        sx={{
          // borderRightWidth: "1px",
          // borderRightStyle: "solid",
          // borderRightColor: "sidebar.itemBorderColor"
          flexGrow: 1,
        }}
      >
        <ItemLink
          item={item}
          overrideCSS={{
            "&&": item.level === 0 && item.ui !== `steps` && styles.level0,
            pr: t => t.sizes.sidebarItemMinHeight,
          }}
        />
      </span>
      <button
        aria-controls={uid}
        aria-expanded={isExpanded}
        aria-label={`${item.title} toggle`}
        sx={{
          ...styles.resetButton,
          bottom: 0,
          ml: `auto`,
          minHeight: `sidebarItemMinHeight`,
          position: `absolute`,
          right: 0,
          top: 0,
          width: `sidebarItemMinHeight`,
          zIndex: 1,
        }}
        onClick={() => onSectionTitleClick(item)}
      >
        <Chevron isExpanded={isExpanded} />
      </button>
    </span>
  )
}

export default function SectionTitle({ itemRef, item, uid }) {
  const { disableAccordions } = useSidebarContext()
  // If the item has a link, render it as a combination link and toggle button
  if (item.link) {
    return <SplitButton itemRef={itemRef} item={item} uid={uid} />
  }
  // Otherwise, render the toggle button depending on if toggling is enabled
  const SectionTitleComponent = disableAccordions ? Title : TitleButton
Repository: gatsbyjs/gatsby
File: www/src/components/sidebar/section-title.js (JavaScript)

Code before:
        alignItems: `flex-end`,
        display: `flex`,
        position: `relative`,
        width: `100%`,
      }}
    >
      <span
        sx={{
          // borderRightWidth: "1px",
          // borderRightStyle: "solid",
          // borderRightColor: "sidebar.itemBorderColor"
          flexGrow: 1,
        }}
      >
        <ItemLink
          item={item}
          overrideCSS={{
            "&&": item.level === 0 && item.ui !== `steps` && styles.level0,
            pr: t => t.sizes.sidebarItemMinHeight,
          }}
        />
      </span>
      <button
        aria-controls={uid}
        aria-expanded={isExpanded}
        aria-label={i18n._(item.title)}
        sx={{
          ...styles.resetButton,
          bottom: 0,
          ml: `auto`,
          minHeight: `sidebarItemMinHeight`,
          position: `absolute`,
          right: 0,
          top: 0,
          width: `sidebarItemMinHeight`,
          zIndex: 1,
        }}
        onClick={() => onSectionTitleClick(item)}
      >
        <Chevron isExpanded={isExpanded} />
      </button>
    </span>
  )
})

export default function SectionTitle({ itemRef, item, uid }) {
  const { disableAccordions } = useSidebarContext()
  // If the item has a link, render it as a combination link and toggle button
  if (item.link) {
    return <SplitButton itemRef={itemRef} item={item} uid={uid} />
  }

Review comment (suggestion):
After talking with @madalynrose we think the best title for this would be `${item.title} toggle` to highlight the fact that it's a toggle.

Code after:
        display: `flex`,
        position: `relative`,
        width: `100%`,
      }}
    >
      <span
        sx={{
          // borderRightWidth: "1px",
          // borderRightStyle: "solid",
          // borderRightColor: "sidebar.itemBorderColor"
          flexGrow: 1,
        }}
      >
        <ItemLink
          item={item}
          overrideCSS={{
            "&&": item.level === 0 && item.ui !== `steps` && styles.level0,
            pr: t => t.sizes.sidebarItemMinHeight,
          }}
        />
      </span>
      <button
        aria-controls={uid}
        aria-expanded={isExpanded}
        aria-label={`${item.title} toggle`}
        sx={{
          ...styles.resetButton,
          bottom: 0,
          ml: `auto`,
          minHeight: `sidebarItemMinHeight`,
          position: `absolute`,
          right: 0,
          top: 0,
          width: `sidebarItemMinHeight`,
          zIndex: 1,
        }}
        onClick={() => onSectionTitleClick(item)}
      >
        <Chevron isExpanded={isExpanded} />
      </button>
    </span>
  )
}

export default function SectionTitle({ itemRef, item, uid }) {
  const { disableAccordions } = useSidebarContext()
  // If the item has a link, render it as a combination link and toggle button
  if (item.link) {
    return <SplitButton itemRef={itemRef} item={item} uid={uid} />
  }
  // Otherwise, render the toggle button depending on if toggling is enabled
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/redux/reducers/inference-metadata.ts (TypeScript)

Code before:

    case `SET_SCHEMA`: {
      Object.keys(state).forEach(type => {
        state[type].dirty = false
      })
      return state
    }

    default:
      return state
  }
}

const StepsEnum = {
  initialBuild: `initialBuild`,
  incrementalBuild: `incrementalBuild`,
}

const initialState = (): IGatsbyState["inferenceMetadata"] => {
  return {
    step: StepsEnum.initialBuild, // `initialBuild` | `incrementalBuild`
    typeMap: {},
  }
}

module.exports = (
  state: IGatsbyState["inferenceMetadata"] = initialState(),
  action: ActionsUnion
): IGatsbyState["inferenceMetadata"] => {
  switch (action.type) {
    case `CREATE_NODE`:
    case `DELETE_NODE`:
    case `DELETE_NODES`:
    case `ADD_CHILD_NODE_TO_PARENT_NODE`:
    case `ADD_FIELD_TO_NODE`: {
      // Perf: disable incremental inference until the first schema build.
      // There are plugins which create and delete lots of nodes during bootstrap,
      // which makes this reducer to do a lot of unnecessary work.
      // Instead we defer the initial metadata creation until the first schema build
      // and then enable incremental updates explicitly
      if (state.step === StepsEnum.initialBuild) {
        return state
      }
      state.typeMap = incrementalReducer(state.typeMap, action)
      return state
    }

    case `START_INCREMENTAL_INFERENCE`: {
      return {
        ...state,
        step: StepsEnum.incrementalBuild,

Review comment (suggestion):
this should be a named export and update the import reference.

e.g.
```js
export function inferenceMetadataReducer(state, action) { .... }
```

Code after:

    case `SET_SCHEMA`: {
      Object.keys(state).forEach(type => {
        state[type].dirty = false
      })
      return state
    }

    default:
      return state
  }
}

enum StepsEnum {
  initialBuild = `initialBuild`,
  incrementalBuild = `incrementalBuild`,
}

const initialState = (): IGatsbyState["inferenceMetadata"] => {
  return {
    step: StepsEnum.initialBuild, // `initialBuild` | `incrementalBuild`
    typeMap: {},
  }
}

export const inferenceMetadataReducer = (
  state: IGatsbyState["inferenceMetadata"] = initialState(),
  action: ActionsUnion
): IGatsbyState["inferenceMetadata"] => {
  switch (action.type) {
    case `CREATE_NODE`:
    case `DELETE_NODE`:
    case `DELETE_NODES`:
    case `ADD_CHILD_NODE_TO_PARENT_NODE`:
    case `ADD_FIELD_TO_NODE`: {
      // Perf: disable incremental inference until the first schema build.
      // There are plugins which create and delete lots of nodes during bootstrap,
      // which makes this reducer to do a lot of unnecessary work.
      // Instead we defer the initial metadata creation until the first schema build
      // and then enable incremental updates explicitly
      if (state.step === StepsEnum.initialBuild) {
        return state
      }
      state.typeMap = incrementalReducer(state.typeMap, action)
      return state
    }

    case `START_INCREMENTAL_INFERENCE`: {
      return {
        ...state,
        step: StepsEnum.incrementalBuild,