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

Code before:
            getNode,
            reporter,
            store,
          },
          pluginOptions
        )
      }
    } catch (e) {
      gracefullyRethrow(changesActivity, e)
      return
    }
    changesActivity.end()
    return
  }

  fastBuilds = fastBuilds || false
  if (fastBuilds) {
    let lastFetched = 0
    if (
      store.getState().status.plugins &&
      store.getState().status.plugins[`gatsby-source-drupal`] &&
      store.getState().status.plugins[`gatsby-source-drupal`].lastFetched
    ) {
      lastFetched = store.getState().status.plugins[`gatsby-source-drupal`]
        .lastFetched
    }

    const drupalFetchIncrementalActivity = reporter.activityTimer(
      `Fetch incremental changes from Drupal`
    )
    let requireFullRebuild = false

    drupalFetchIncrementalActivity.start()

    try {
      // Hit fastbuilds endpoint with the lastFetched date.
      const data = await axios.get(
        `${baseUrl}/gatsby-fastbuilds/sync/${lastFetched}`,
        {
          auth: basicAuth,
          headers,
          params,
        }
      )

      if (data.data.status === -1) {
        reporter.info(`Unable to pull incremental data changes from Drupal`)
        setPluginStatus({ lastFetched: data.data.timestamp })
        requireFullRebuild = true
      } else {
        // Touch nodes so they are not garbage collected by Gatsby.

Review comment (suggestion):
Embrace the optional chaining :)
```suggestion
    let lastFetched = store.getState().status.plugins?.[`gatsby-source-drupal`]?.lastFetched
```

If `undefined` is important then you can append `?? undefined` to that.

Code after:
            getNode,
            reporter,
            store,
          },
          pluginOptions
        )
      }
    } catch (e) {
      gracefullyRethrow(changesActivity, e)
      return
    }
    changesActivity.end()
    return
  }

  fastBuilds = fastBuilds || false
  if (fastBuilds) {
    let lastFetched =
      store.getState().status.plugins?.[`gatsby-source-drupal`]?.lastFetched ??
      0

    const drupalFetchIncrementalActivity = reporter.activityTimer(
      `Fetch incremental changes from Drupal`
    )
    let requireFullRebuild = false

    drupalFetchIncrementalActivity.start()

    try {
      // Hit fastbuilds endpoint with the lastFetched date.
      const data = await axios.get(
        `${baseUrl}/gatsby-fastbuilds/sync/${lastFetched}`,
        {
          auth: basicAuth,
          headers,
          params,
        }
      )

      if (data.data.status === -1) {
        // The incremental data is expired or this is the first fetch.
        reporter.info(`Unable to pull incremental data changes from Drupal`)
        setPluginStatus({ lastFetched: data.data.timestamp })
        requireFullRebuild = true
      } else {
        // Touch nodes so they are not garbage collected by Gatsby.
        getNodes().forEach(node => {
          if (node.internal.owner === `gatsby-source-drupal`) {
            touchNode({ nodeId: node.id })
          }
        })
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/api-runner-node.js (JavaScript)

Code before:
              )} or http requests).
              For more info and debugging tips: see ${chalk.bold(
                `https://gatsby.dev/sync-actions`
              )}
            `),
            ]

            const possiblyCodeFrame = getNonGatsbyCodeFrameFormatted()
            if (possiblyCodeFrame) {
              warning.push(possiblyCodeFrame)
            }

            reporter.warn(warning.join(`\n\n`))
            alreadyDisplayed = true
          }
        },
      }
    }
    const localReporter = getLocalReporter(activity, reporter)

    let runningActivities = new Set()

    const localReporterThatCleansUpAfterMisbehavingPlugins = {
      ...localReporter,
      activityTimer: (...args) => {
        const activity = reporter.activityTimer(...args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress(...args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)

Review comment (suggestion):
Wouldn't this be safer?

```suggestion
        const activity = reporter.activityTimer.apply(reporter, args)
```

Code after:
              )} or http requests).
              For more info and debugging tips: see ${chalk.bold(
                `https://gatsby.dev/sync-actions`
              )}
            `),
            ]

            const possiblyCodeFrame = getNonGatsbyCodeFrameFormatted()
            if (possiblyCodeFrame) {
              warning.push(possiblyCodeFrame)
            }

            reporter.warn(warning.join(`\n\n`))
            alreadyDisplayed = true
          }
        },
      }
    }
    const localReporter = getLocalReporter(activity, reporter)

    const runningActivities = new Set()

    const localReporterThatCleansUpAfterMisbehavingPlugins = {
      ...localReporter,
      activityTimer: (...args) => {
        const activity = reporter.activityTimer.apply(reporter, args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress.apply(reporter, args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/api-runner-node.js (JavaScript)

Code before:
        },
      }
    }
    const localReporter = getLocalReporter(activity, reporter)

    let runningActivities = new Set()

    const localReporterThatCleansUpAfterMisbehavingPlugins = {
      ...localReporter,
      activityTimer: (...args) => {
        const activity = reporter.activityTimer(...args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress(...args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
          done: () => {
            activity.done()
            runningActivities.delete(activity)
          },
        }
      },
    }

    const endInProgressActivitiesCreatedByThisRun = () => {
      runningActivities.forEach(timer => timer.end())
    }

    const apiCallArgs = [
      {

Review comment (suggestion):
```suggestion
        const activity = reporter.createProgress.apply(reporter, args)
```

Code after:
        },
      }
    }
    const localReporter = getLocalReporter(activity, reporter)

    const runningActivities = new Set()

    const localReporterThatCleansUpAfterMisbehavingPlugins = {
      ...localReporter,
      activityTimer: (...args) => {
        const activity = reporter.activityTimer.apply(reporter, args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress.apply(reporter, args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
          done: () => {
            activity.done()
            runningActivities.delete(activity)
          },
        }
      },
    }

    const endInProgressActivitiesCreatedByThisRun = () => {
      runningActivities.forEach(activity => activity.end())
    }

    const apiCallArgs = [
      {
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/api-runner-node.js (JavaScript)

Code before:
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress(...args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
          done: () => {
            activity.done()
            runningActivities.delete(activity)
          },
        }
      },
    }

    const endInProgressActivitiesCreatedByThisRun = () => {
      runningActivities.forEach(timer => timer.end())
    }

    const apiCallArgs = [
      {
        ...args,
        basePath: pathPrefix,
        pathPrefix: publicPath,
        boundActionCreators: actions,
        actions,
        loadNodeContent,
        store,
        emitter,
        getCache,
        getNodes,
        getNode,
        getNodesByType,
        hasNodeChanged,
        reporter: localReporterThatCleansUpAfterMisbehavingPlugins,
        getNodeAndSavePathDependency,
        cache,
        createNodeId: namespacedCreateNodeId,
        createContentDigest,
        tracing,
        schema: {
          buildObjectType,

Review comment (suggestion):
nit:
```suggestion
      runningActivities.forEach(activity => activity.end())
```

Code after:
          },
        }
      },
      createProgress: (...args) => {
        const activity = reporter.createProgress.apply(reporter, args)

        return {
          ...activity,
          start: () => {
            activity.start()
            runningActivities.add(activity)
          },
          end: () => {
            activity.end()
            runningActivities.delete(activity)
          },
          done: () => {
            activity.done()
            runningActivities.delete(activity)
          },
        }
      },
    }

    const endInProgressActivitiesCreatedByThisRun = () => {
      runningActivities.forEach(activity => activity.end())
    }

    const apiCallArgs = [
      {
        ...args,
        basePath: pathPrefix,
        pathPrefix: publicPath,
        boundActionCreators: actions,
        actions,
        loadNodeContent,
        store,
        emitter,
        getCache,
        getNodes,
        getNode,
        getNodesByType,
        hasNodeChanged,
        reporter: localReporterThatCleansUpAfterMisbehavingPlugins,
        getNodeAndSavePathDependency,
        cache,
        createNodeId: namespacedCreateNodeId,
        createContentDigest,
        tracing,
        schema: {
          buildObjectType,
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/__tests__/api-runner-node.js (JavaScript)

Code before:
        {
          name: `test-plugin-spinner`,
          resolve: `test-plugin-spinner`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-progress`,
          resolve: `test-plugin-progress`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-spinner-throw`,
          resolve: `test-plugin-spinner-throw`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-progress-throw`,
          resolve: `test-plugin-progress-throw`,
          nodeAPIs: [`testAPIHook`],
        },
      ],
    }
  })
  await apiRunnerNode(`testAPIHook`)

  expect(mockActivity.start).toBeCalledTimes(6)
  // we called end same amount of times we called start, even tho plugins
  // didn't call end/done themselves
  expect(mockActivity.end).toBeCalledTimes(6)
})


Review comment (suggestion):
Because activity is being mutated, `mockActivity.start` is no longer mocked after passing though api-runner, but we have reference to original `start` so we can use that. Same for the `end` case

Code after:
        {
          name: `test-plugin-spinner`,
          resolve: `test-plugin-spinner`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-progress`,
          resolve: `test-plugin-progress`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-spinner-throw`,
          resolve: `test-plugin-spinner-throw`,
          nodeAPIs: [`testAPIHook`],
        },
        {
          name: `test-plugin-progress-throw`,
          resolve: `test-plugin-progress-throw`,
          nodeAPIs: [`testAPIHook`],
        },
      ],
    }
  })
  await apiRunnerNode(`testAPIHook`)

  expect(start).toBeCalledTimes(6)
  // we called end same amount of times we called start, even tho plugins
  // didn't call end/done themselves
  expect(end).toBeCalledTimes(6)
})
Repository: gatsbyjs/gatsby
File: packages/gatsby/cache-dir/loader.js (JavaScript)

Code before:
    page,
    staticQueryHashes,
  }
}

export class BaseLoader {
  constructor(loadComponent, matchPaths) {
    // Map of pagePath -> Page. Where Page is an object with: {
    //   status: PageResourceStatus.Success || PageResourceStatus.Error,
    //   payload: PageResources, // undefined if PageResourceStatus.Error
    // }
    // PageResources is {
    //   component,
    //   json: pageData.result,
    //   page: {
    //     componentChunkName,
    //     path,
    //     webpackCompilationHash,
    //   }
    // }
    this.pageDb = new Map()
    this.inFlightDb = new Map()

    this.staticQueryDb = new Map()

    this.inFlightPromisesByUrlDb = new Map()

    this.pageDataDb = new Map()
    this.prefetchTriggered = new Set()
    this.prefetchCompleted = new Set()
    this.loadComponent = loadComponent
    setMatchPaths(matchPaths)
  }

  memoizedGet(url) {
    if (!this.inFlightPromisesByUrlDb.has(url)) {
      this.inFlightPromisesByUrlDb.set(url, doFetch(url, `GET`))
    }
    return this.inFlightPromisesByUrlDb.get(url)
  }

  setApiRunner(apiRunner) {
    this.apiRunner = apiRunner
    this.prefetchDisabled = apiRunner(`disableCorePrefetching`).some(a => a)
  }

  loadPageDataJson(rawPath) {
    const pagePath = findPath(rawPath)
    if (this.pageDataDb.has(pagePath)) {
      return Promise.resolve(this.pageDataDb.get(pagePath))
    }

Review comment (suggestion):
We can probably use the same `inFlightDb` for both (and `memoizedGet` then too).

Code after:
    const url = createPageDataUrl(pagePath)
    return this.memoizedGet(url).then(req => {
      const { status, responseText } = req

      // Handle 200
      if (status === 200) {
        try {
          const jsonPayload = JSON.parse(responseText)
          if (jsonPayload.path === undefined) {
            throw new Error(`not a valid pageData response`)
          }

          return Object.assign(loadObj, {
            status: PageResourceStatus.Success,
            payload: jsonPayload,
          })
        } catch (err) {
          // continue regardless of error
        }
      }

      // Handle 404
      if (status === 404 || status === 200) {
        // If the request was for a 404 page and it doesn't exist, we're done
        if (pagePath === `/404.html`) {
          return Object.assign(loadObj, {
            status: PageResourceStatus.Error,
          })
        }

        // Need some code here to cache the 404 request. In case
        // multiple loadPageDataJsons result in 404s
        return this.fetchPageDataJson(
          Object.assign(loadObj, { pagePath: `/404.html`, notFound: true })
        )
      }

      // handle 500 response (Unrecoverable)
      if (status === 500) {
        return Object.assign(loadObj, {
          status: PageResourceStatus.Error,
        })
      }

      // Handle everything else, including status === 0, and 503s. Should retry
      if (retries < 3) {
        return this.fetchPageDataJson(
          Object.assign(loadObj, { retries: retries + 1 })
        )
      }
Repository: gatsbyjs/gatsby
File: packages/gatsby/cache-dir/loader.js (JavaScript)

Code before:
    // PageResources is {
    //   component,
    //   json: pageData.result,
    //   page: {
    //     componentChunkName,
    //     path,
    //     webpackCompilationHash,
    //   }
    // }
    this.pageDb = new Map()
    this.inFlightDb = new Map()

    this.staticQueryDb = new Map()

    this.inFlightPromisesByUrlDb = new Map()

    this.pageDataDb = new Map()
    this.prefetchTriggered = new Set()
    this.prefetchCompleted = new Set()
    this.loadComponent = loadComponent
    setMatchPaths(matchPaths)
  }

  memoizedGet(url) {
    if (!this.inFlightPromisesByUrlDb.has(url)) {
      this.inFlightPromisesByUrlDb.set(url, doFetch(url, `GET`))
    }
    return this.inFlightPromisesByUrlDb.get(url)
  }

  setApiRunner(apiRunner) {
    this.apiRunner = apiRunner
    this.prefetchDisabled = apiRunner(`disableCorePrefetching`).some(a => a)
  }

  loadPageDataJson(rawPath) {
    const pagePath = findPath(rawPath)
    if (this.pageDataDb.has(pagePath)) {
      return Promise.resolve(this.pageDataDb.get(pagePath))
    }

    return loadPageDataJson({ pagePath }).then(pageData => {
      this.pageDataDb.set(pagePath, pageData)

      return pageData
    })
  }

  findMatchPath(rawPath) {
    return findMatchPath(rawPath)
  }

Review comment (suggestion):
Need to clean inFlight promise after it's done

```suggestion
      const fetchPromise = doFetch(url, `GET`)
      this.inFlightPromisesByUrlDb.set(url, fetchPromise)
      fetchPromise.then(() => {
        this.inFlightPromisesByUrlDb.delete(url)
      })
```

Code after:

          return Object.assign(loadObj, {
            status: PageResourceStatus.Success,
            payload: jsonPayload,
          })
        } catch (err) {
          // continue regardless of error
        }
      }

      // Handle 404
      if (status === 404 || status === 200) {
        // If the request was for a 404 page and it doesn't exist, we're done
        if (pagePath === `/404.html`) {
          return Object.assign(loadObj, {
            status: PageResourceStatus.Error,
          })
        }

        // Need some code here to cache the 404 request. In case
        // multiple loadPageDataJsons result in 404s
        return this.fetchPageDataJson(
          Object.assign(loadObj, { pagePath: `/404.html`, notFound: true })
        )
      }

      // handle 500 response (Unrecoverable)
      if (status === 500) {
        return Object.assign(loadObj, {
          status: PageResourceStatus.Error,
        })
      }

      // Handle everything else, including status === 0, and 503s. Should retry
      if (retries < 3) {
        return this.fetchPageDataJson(
          Object.assign(loadObj, { retries: retries + 1 })
        )
      }

      // Retried 3 times already, result is an error.
      return Object.assign(loadObj, {
        status: PageResourceStatus.Error,
      })
    })
  }

  loadPageDataJson(rawPath) {
    const pagePath = findPath(rawPath)
    if (this.pageDataDb.has(pagePath)) {
      return Promise.resolve(this.pageDataDb.get(pagePath))
Repository: gatsbyjs/gatsby
File: packages/gatsby/cache-dir/loader.js (JavaScript)

Code before:
          return payload
        }
      )
    })

    inFlightPromise
      .then(response => {
        this.inFlightDb.delete(pagePath)
      })
      .catch(error => {
        this.inFlightDb.delete(pagePath)
        throw error
      })

    this.inFlightDb.set(pagePath, inFlightPromise)

    return inFlightPromise
  }

  // returns undefined if loading page ran into errors
  loadPageSync(rawPath) {
    const pagePath = findPath(rawPath)
    if (this.pageDb.has(pagePath)) {
      const pageData = this.pageDb.get(pagePath).payload
      return pageData
    }
    return undefined
  }

  shouldPrefetch(pagePath) {
    // Skip prefetching if we know user is on slow or constrained connection
    if (!doesConnectionSupportPrefetch()) {
      return false
    }

    // Check if the page exists.
    if (this.pageDb.has(pagePath)) {
      return false
    }

    return true
  }

  prefetch(pagePath) {
    if (!this.shouldPrefetch(pagePath)) {
      return false
    }

    // Tell plugins with custom prefetching logic that they should start
    // prefetching this path.
    if (!this.prefetchTriggered.has(pagePath)) {

Review comment (suggestion):
```suggestion
      const pageData = this.pageDb.get(pagePath)?.payload
```

Code after:
          this.pageDb.set(pagePath, finalResult)

          return payload
        }
      )
    })

    inFlightPromise
      .then(response => {
        this.inFlightDb.delete(pagePath)
      })
      .catch(error => {
        this.inFlightDb.delete(pagePath)
        throw error
      })

    this.inFlightDb.set(pagePath, inFlightPromise)

    return inFlightPromise
  }

  // returns undefined if loading page ran into errors
  loadPageSync(rawPath) {
    const pagePath = findPath(rawPath)
    if (this.pageDb.has(pagePath)) {
      const pageData = this.pageDb.get(pagePath).payload
      return pageData
    }
    return undefined
  }

  shouldPrefetch(pagePath) {
    // Skip prefetching if we know user is on slow or constrained connection
    if (!doesConnectionSupportPrefetch()) {
      return false
    }

    // Check if the page exists.
    if (this.pageDb.has(pagePath)) {
      return false
    }

    return true
  }

  prefetch(pagePath) {
    if (!this.shouldPrefetch(pagePath)) {
      return false
    }

    // Tell plugins with custom prefetching logic that they should start