ronantakizawa/codereview-bench huggingface dataset:
Repository: parcel-bundler/parcel
File: packages/core/types/index.js (JavaScript)

Code before:
    visit: GraphVisitor<TBundle, TContext>,
    startBundle: ?Bundle,
  ): ?TContext;
}

/**
 * @section bundler
 */
export type BundleResult = {|
  +contents: Blob,
  +ast?: AST,
  +map?: ?SourceMap,
  +type?: string,
|};

/**
 * @section resolver
 */
export type ResolveResult = {|
  +filePath?: FilePath,
  +isExcluded?: boolean,
  /** Corresponds to BaseAsset's <code>sideEffects</code>. */
  +sideEffects?: boolean,
  /** A resolver might want to resolve to a dummy, in this case <code>filePath</code> is rather "resolve from". */
  +code?: string,
  /** A resolver might return diagnostics */
  +diagnostics?: Diagnostic | Array<Diagnostic>,
|};

export type ConfigOutput = {|
  config: ConfigResult,
  files: Array<File>,
|};

/**
 * Turns an asset graph into a BundleGraph.
 *
 * bundle and optimize run in series and are functionally identitical.
 * @section bundler
 */
export type Bundler = {|
  loadConfig?: ({|
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<ConfigOutput>,
  bundle({|
    bundleGraph: MutableBundleGraph,
    config: ?ConfigResult,
    options: PluginOptions,
    logger: PluginLogger,
  |}): Async<void>,

Review comment (bug):
```suggestion
  /** A resolver might return diagnostics to also run subsequent resolvers while still providing a reason why it failed*/
```

Code after:
    visit: GraphVisitor<TBundle, TContext>,
    startBundle: ?Bundle,
  ): ?TContext;
}

/**
 * @section bundler
 */
export type BundleResult = {|
  +contents: Blob,
  +ast?: AST,
  +map?: ?SourceMap,
  +type?: string,
|};

/**
 * @section resolver
 */
export type ResolveResult = {|
  +filePath?: FilePath,
  +isExcluded?: boolean,
  /** Corresponds to BaseAsset's <code>sideEffects</code>. */
  +sideEffects?: boolean,
  /** A resolver might want to resolve to a dummy, in this case <code>filePath</code> is rather "resolve from". */
  +code?: string,
  /** A resolver might return diagnostics to also run subsequent resolvers while still providing a reason why it failed*/
  +diagnostics?: Diagnostic | Array<Diagnostic>,
|};

export type ConfigOutput = {|
  config: ConfigResult,
  files: Array<File>,
|};

/**
 * Turns an asset graph into a BundleGraph.
 *
 * bundle and optimize run in series and are functionally identitical.
 * @section bundler
 */
export type Bundler = {|
  loadConfig?: ({|
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<ConfigOutput>,
  bundle({|
    bundleGraph: MutableBundleGraph,
    config: ?ConfigResult,
    options: PluginOptions,
    logger: PluginLogger,
  |}): Async<void>,
Repository: parcel-bundler/parcel
File: packages/core/utils/src/sourcemap.js (JavaScript)

Code before:
  'jsx',
  'mjs',
  'es',
  'es6',
  'css',
]);

export function matchSourceMappingURL(
  contents: string,
): null | RegExp$matchResult {
  return contents.match(SOURCEMAP_RE);
}

export async function loadSourceMapUrl(
  fs: FileSystem,
  filename: string,
  contents: string,
): Promise<?{|filename: string, map: any, url: string|}> {
  let match = matchSourceMappingURL(contents);
  if (match) {
    let url = match[1].trim();
    let dataURLMatch = url.match(DATA_URL_RE);
    let mapFilePath = dataURLMatch ? filename : url.replace(/^file:\/\//, '');
    if (!dataURLMatch && !isAbsolute(mapFilePath)) {
      mapFilePath = path.join(path.dirname(filename), mapFilePath);
    }

    return {
      url,
      filename: mapFilePath,
      map: JSON.parse(
        dataURLMatch
          ? Buffer.from(dataURLMatch[1], 'base64').toString()
          : await fs.readFile(mapFilePath, 'utf8'),
      ),
    };
  }
}

export async function loadSourceMap(
  filename: string,
  contents: string,
  options: {fs: FileSystem, projectRoot: string, ...},
): Promise<?SourceMap> {
  let foundMap = await loadSourceMapUrl(options.fs, filename, contents);
  if (foundMap) {
    let mapSourceRoot = path.dirname(filename);
    if (
      foundMap.map.sourceRoot &&
      !normalizeSeparators(foundMap.map.sourceRoot).startsWith('/')
    ) {

Review comment (suggestion):
```suggestion
    let mapFilePath;
    if (dataURLMatch) {
        mapFilePath =  isAbsolute(mapFilePath) ? filename : path.join(path.dirname(filename), mapFilePath);
    } else {
        mapFilePath = url.replace(/^file:\/\//, '');
    }
```


Code after:
  'jsx',
  'mjs',
  'es',
  'es6',
  'css',
]);

export function matchSourceMappingURL(
  contents: string,
): null | RegExp$matchResult {
  return contents.match(SOURCEMAP_RE);
}

export async function loadSourceMapUrl(
  fs: FileSystem,
  filename: string,
  contents: string,
): Promise<?{|filename: string, map: any, url: string|}> {
  let match = matchSourceMappingURL(contents);
  if (match) {
    let url = match[1].trim();
    let dataURLMatch = url.match(DATA_URL_RE);

    let mapFilePath;
    if (dataURLMatch) {
      mapFilePath = filename;
    } else {
      mapFilePath = url.replace(/^file:\/\//, '');
      mapFilePath = isAbsolute(mapFilePath)
        ? mapFilePath
        : path.join(path.dirname(filename), mapFilePath);
    }

    return {
      url,
      filename: mapFilePath,
      map: JSON.parse(
        dataURLMatch
          ? Buffer.from(dataURLMatch[1], 'base64').toString()
          : await fs.readFile(mapFilePath, 'utf8'),
      ),
    };
  }
}

export async function loadSourceMap(
  filename: string,
  contents: string,
  options: {fs: FileSystem, projectRoot: string, ...},
): Promise<?SourceMap> {
  let foundMap = await loadSourceMapUrl(options.fs, filename, contents);
Repository: parcel-bundler/parcel
File: packages/shared/scope-hoisting/src/prelude.js (JavaScript)

Code before:
var $parcel$modules = {};
var $parcel$bundles = {};

var $parcel$global =
  typeof globalThis !== 'undefined'
    ? globalThis
    : typeof self !== 'undefined'
    ? self
    : typeof window !== 'undefined'
    ? window
    : typeof global !== 'undefined'
    ? global
    : {};

if ($parcel$global[parcelRequireName] == null) {
  var parcelRequire = function(name) {
    // Execute the bundle wrapper function if there is one registered.
    if (name in $parcel$bundles) {
      $parcel$bundles[name]();
      delete $parcel$bundles[name];
    }

    if (name in $parcel$modules) {
      return $parcel$modules[name];
    }

    // Try the node require function if it exists.
    // Do not use `require` to prevent Webpack from trying to bundle this call
    if (typeof module !== 'undefined' && typeof module.require === 'function') {
      return module.require(name);
    }

    var err = new Error("Cannot find module '" + name + "'");
    err.code = 'MODULE_NOT_FOUND';
    throw err;
  };

  parcelRequire.register = function register(id, exports) {

Review comment (refactor):
I couldn't find a way to reuse the one in helpers.js because that one will be inside the `$parcel$bundleWrapper`

Code after:
var $parcel$modules = {};
var $parcel$bundles = {};

if (parcelRequire == null) {
  parcelRequire = function(name) {
    // Execute the bundle wrapper function if there is one registered.
    if (name in $parcel$bundles) {
      let wrapper = $parcel$bundles[name];
      delete $parcel$bundles[name];
      wrapper();
    }

    if (name in $parcel$modules) {
      return $parcel$modules[name];
    }

    // Try the node require function if it exists.
    // Do not use `require` to prevent Webpack from trying to bundle this call
    if (typeof module !== 'undefined' && typeof module.require === 'function') {
      return module.require(name);
    }

    var err = new Error("Cannot find module '" + name + "'");
    err.code = 'MODULE_NOT_FOUND';
    throw err;
  };

  parcelRequire.register = function register(id, exports) {
    $parcel$modules[id] = exports;
  };

  parcelRequire.registerBundle = function registerBundle(id, fn) {
    $parcel$bundles[id] = fn;
    $parcel$modules[id] = {};
  };

  $parcel$global[parcelRequireName] = parcelRequire;
}
Repository: parcel-bundler/parcel
File: packages/core/core/src/Parcel.js (JavaScript)

Code before:

    this.#packagerRunner = new PackagerRunner({
      config: this.#config,
      farm: this.#farm,
      options: resolvedOptions,
      optionsRef,
      configRef,
      report,
    });

    this.#initialized = true;
  }

  async run(): Promise<IBundleGraph<INamedBundle>> {
    let startTime = Date.now();
    if (!this.#initialized) {
      await this._init();
    }

    let result = await this._build({startTime});

    if (result.type === 'buildFailure') {
      throw new BuildError(result.diagnostics);
    }

    await this._end();
    return result.bundleGraph;
  }

  async _end(): Promise<void> {
    this.#initialized = false;

    await Promise.all([
      this.#disposable.dispose(),
      this.#assetGraphBuilder.writeToCache(),
      this.#runtimesAssetGraphBuilder.writeToCache(),
    ]);
  }

  async _startNextBuild() {
    this.#watchAbortController = new AbortController();

    try {
      this.#watchEvents.emit({
        buildEvent: await this._build({
          signal: this.#watchAbortController.signal,
        }),
      });
    } catch (err) {
      // Ignore BuildAbortErrors and only emit critical errors.
      if (!(err instanceof BuildAbortError)) {

Review comment (bug):
So on buildFailure, we don't write anything to the cache?
Should `_end` be called before throwing the error?

Code after:
      config: this.#config,
      farm: this.#farm,
      options: resolvedOptions,
      optionsRef,
      configRef,
      report,
    });

    this.#initialized = true;
  }

  async run(): Promise<BuildSuccessEvent> {
    let startTime = Date.now();
    if (!this.#initialized) {
      await this._init();
    }

    let result = await this._build({startTime});
    await this._end();

    if (result.type === 'buildFailure') {
      throw new BuildError(result.diagnostics);
    }

    return result;
  }

  async _end(): Promise<void> {
    this.#initialized = false;

    await Promise.all([
      this.#disposable.dispose(),
      this.#assetGraphBuilder.writeToCache(),
      this.#runtimesAssetGraphBuilder.writeToCache(),
    ]);
  }

  async _startNextBuild() {
    this.#watchAbortController = new AbortController();

    try {
      this.#watchEvents.emit({
        buildEvent: await this._build({
          signal: this.#watchAbortController.signal,
        }),
      });
    } catch (err) {
      // Ignore BuildAbortErrors and only emit critical errors.
      if (!(err instanceof BuildAbortError)) {
        throw err;
      }
Repository: parcel-bundler/parcel
File: packages/core/core/src/Parcel.js (JavaScript)

Code before:
    this.#bundlerRunner = new BundlerRunner({
      options: resolvedOptions,
      runtimesBuilder: this.#runtimesAssetGraphBuilder,
      config: this.#config,
      workerFarm: this.#farm,
    });

    this.#reporterRunner = new ReporterRunner({
      config: this.#config,
      options: resolvedOptions,
      workerFarm: this.#farm,
    });

    this.#packagerRunner = new PackagerRunner({
      config: this.#config,
      farm: this.#farm,
      options: resolvedOptions,
      optionsRef,
      configRef,
      report,
    });

    this.#initialized = true;
  }

  async run(): Promise<IBundleGraph<INamedBundle>> {
    let startTime = Date.now();
    if (!this.#initialized) {
      await this._init();
    }

    let result = await this._build({startTime});

    if (result.type === 'buildFailure') {
      throw new BuildError(result.diagnostics);
    }

    await this._end();
    return result.bundleGraph;
  }

  async _end(): Promise<void> {
    this.#initialized = false;

    await Promise.all([
      this.#disposable.dispose(),
      this.#assetGraphBuilder.writeToCache(),
      this.#runtimesAssetGraphBuilder.writeToCache(),
    ]);
  }


Review comment (suggestion):
(Not really related to this change but it might be nice to be able to pass an `AbortSignal` here...)

Code after:
      runtimesBuilder: this.#runtimesAssetGraphBuilder,
      config: this.#config,
      workerFarm: this.#farm,
    });

    this.#reporterRunner = new ReporterRunner({
      config: this.#config,
      options: resolvedOptions,
      workerFarm: this.#farm,
    });

    this.#packagerRunner = new PackagerRunner({
      config: this.#config,
      farm: this.#farm,
      options: resolvedOptions,
      optionsRef,
      configRef,
      report,
    });

    this.#initialized = true;
  }

  async run(): Promise<BuildSuccessEvent> {
    let startTime = Date.now();
    if (!this.#initialized) {
      await this._init();
    }

    let result = await this._build({startTime});
    await this._end();

    if (result.type === 'buildFailure') {
      throw new BuildError(result.diagnostics);
    }

    return result;
  }

  async _end(): Promise<void> {
    this.#initialized = false;

    await Promise.all([
      this.#disposable.dispose(),
      this.#assetGraphBuilder.writeToCache(),
      this.#runtimesAssetGraphBuilder.writeToCache(),
    ]);
  }

  async _startNextBuild() {
    this.#watchAbortController = new AbortController();
Repository: parcel-bundler/parcel
File: packages/transformers/babel/src/env.js (JavaScript)

Code before:
// @flow strict-local

import type {Config} from '@parcel/types';
import type {BabelTargets} from './types';

import presetEnv from '@babel/preset-env';

import getBabelTargets from './getBabelTargets';
import {enginesToBabelTargets} from './utils';

/**
 * Generates a @babel/preset-env config for an asset.
 * This is done by finding the source module's target engines, and the app's
 * target engines, and doing a diff to include only the necessary plugins.
 */
export default async function getEnvOptions(
  config: Config,
): Promise<?{|
  presets: Array<
    Array<
      | string
      | {|
          corejs: number,
          ignoreBrowserslistConfig: boolean,
          shippedProposals: boolean,
          targets: BabelTargets,
          useBuiltIns: string,
        |},
    >,

Review comment (suggestion):
We can remove the existing definition of `BabelTargets` from this types file and use the new one 😄 

Code after:
// @flow strict-local

import type {Config} from '@parcel/types';
import presetEnv from '@babel/preset-env';
import type {Targets as BabelTargets, PresetEnvPlugin} from '@babel/preset-env';

import getBabelTargets from './getBabelTargets';
import {enginesToBabelTargets} from './utils';

/**
 * Generates a @babel/preset-env config for an asset.
 * This is done by finding the source module's target engines, and the app's
 * target engines, and doing a diff to include only the necessary plugins.
 */
export default async function getEnvOptions(
  config: Config,
): Promise<?{|
  presets: Array<
    Array<
      | string
      | {|
          corejs: number,
          ignoreBrowserslistConfig: boolean,
          shippedProposals: boolean,
          targets: BabelTargets,
          useBuiltIns: string,
        |},
    >,
  >,
Repository: parcel-bundler/parcel
File: packages/core/test-utils/src/utils.js (JavaScript)

Code before:
        let src = url.parse(nullthrows(node.attrs).src);
        if (src.hostname == null) {
          scripts.push(path.join(distDir, nullthrows(src.pathname)));
        }
      }
      return node;
    });

    let bundles = bundleGraph.getBundles();
    return runBundles(
      bundleGraph,
      bundle,
      scripts.map(p => nullthrows(bundles.find(b => b.filePath === p))),
      globals,
      opts,
    );
  } else {
    return runBundles(bundleGraph, bundle, [bundle], globals, opts);
  }
}

export function run(
  bundleGraph: BundleGraph<NamedBundle>,
  globals: mixed,
  opts: RunOpts = {},
): Promise<any> {
  let bundle = nullthrows(
    bundleGraph.getBundles().find(b => b.type === 'js' || b.type === 'html'),
  );
  return runBundle(bundleGraph, bundle, globals, opts);
}

export function assertBundles(
  bundleGraph: BundleGraph<NamedBundle>,
  expectedBundles: Array<{|
    name?: string | RegExp,
    type?: string,
    assets: Array<string>,
  |}>,
) {
  let actualBundles = [];
  const byAlphabet = (a, b) => (a.toLowerCase() < b.toLowerCase() ? -1 : 1);

  bundleGraph.traverseBundles(bundle => {
    let assets = [];

    bundle.traverseAssets(asset => {
      const name = path.basename(asset.filePath);
      assets.push(name);
    });


Review comment (suggestion):
The only real "benefit" of `mixed` here is that you need to always add a `$FlowFixMe` when using it, so I'm not sure if this is better than before.

Code after:
      if (node.tag === 'script') {
        let src = url.parse(nullthrows(node.attrs).src);
        if (src.hostname == null) {
          scripts.push(path.join(distDir, nullthrows(src.pathname)));
        }
      }
      return node;
    });

    let bundles = bundleGraph.getBundles();
    return runBundles(
      bundleGraph,
      bundle,
      scripts.map(p => nullthrows(bundles.find(b => b.filePath === p))),
      globals,
      opts,
    );
  } else {
    return runBundles(bundleGraph, bundle, [bundle], globals, opts);
  }
}

export function run(
  bundleGraph: BundleGraph<NamedBundle>,
  globals: mixed,
  opts: RunOpts = {},
): Promise<mixed> {
  let bundle = nullthrows(
    bundleGraph.getBundles().find(b => b.type === 'js' || b.type === 'html'),
  );
  return runBundle(bundleGraph, bundle, globals, opts);
}

export function assertBundles(
  bundleGraph: BundleGraph<NamedBundle>,
  expectedBundles: Array<{|
    name?: string | RegExp,
    type?: string,
    assets: Array<string>,
  |}>,
) {
  let actualBundles = [];
  const byAlphabet = (a, b) => (a.toLowerCase() < b.toLowerCase() ? -1 : 1);

  bundleGraph.traverseBundles(bundle => {
    let assets = [];

    bundle.traverseAssets(asset => {
      const name = path.basename(asset.filePath);
      assets.push(name);
    });
Repository: parcel-bundler/parcel
File: packages/core/utils/src/config.js (JavaScript)

Code before:

  // Don't traverse above the module root
  if (path.basename(filepath) === 'node_modules') {
    return null;
  }

  for (const filename of filenames) {
    let file = path.join(filepath, filename);
    if ((await fs.exists(file)) && (await fs.stat(file)).isFile()) {
      return file;
    }
  }

  if (filepath === root) {
    return null;
  }
  return resolveConfig(fs, filepath, filenames, opts);
}

export function resolveConfigSync(
  fs: FileSystem,
  filepath: FilePath,
  filenames: Array<FilePath>,
  opts: ?ConfigOptions,
  root: FilePath = path.parse(filepath).root,
): FilePath | null {
  filepath = fs.realpathSync(path.dirname(filepath));

  // Don't traverse above the module root
  if (filepath === root || path.basename(filepath) === 'node_modules') {
    return null;
  }

  for (const filename of filenames) {
    let file = path.join(filepath, filename);
    if (fs.existsSync(file) && fs.statSync(file).isFile()) {
      return file;
    }
  }

  return resolveConfigSync(fs, filepath, filenames, opts);
}

export function clearCache() {
  configCache.reset();
}

export async function loadConfig(
  fs: FileSystem,
  filepath: FilePath,
  filenames: Array<FilePath>,

Review comment (performance):
Should we set the cache here too before returning?

```
configCache.set(configFile, {
  // $FlowFixMe
  config: clone(require(configFile)),
  files: [{filePath: configFile}],
});
```

Code after:
}

export function resolveConfigSync(
  fs: FileSystem,
  filepath: FilePath,
  filenames: Array<FilePath>,
): ?FilePath {
  return fs.findAncestorFile(filenames, path.dirname(filepath));
}

export async function loadConfig(
  fs: FileSystem,
  filepath: FilePath,
  filenames: Array<FilePath>,
  opts: ?ConfigOptions,
): Promise<ConfigOutput | null> {
  let configFile = await resolveConfig(fs, filepath, filenames);
  if (configFile) {
    let cachedOutput = configCache.get(configFile);
    if (cachedOutput) {
      return cachedOutput;
    }

    try {
      let extname = path.extname(configFile).slice(1);
      if (extname === 'js') {
        let output = {
          // $FlowFixMe
          config: clone(require(configFile)),
          files: [{filePath: configFile}],
        };

        configCache.set(configFile, output);
        return output;
      }

      let configContent = await fs.readFile(configFile, 'utf8');
      if (!configContent) return null;

      let config;
      if (opts && opts.parse === false) {
        config = configContent;
      } else {
        let parse = getParser(extname);
        config = parse(configContent);
      }

      let output = {
        config,
        files: [{filePath: configFile}],
      };
Repository: parcel-bundler/parcel
File: packages/core/core/src/PackagerRunner.js (JavaScript)

Code before:
              hashRefToNameHash,
              bundleGraph,
            });
          }
        }),
      );
      assignComplexNameHashes(
        hashRefToNameHash,
        bundles,
        bundleInfoMap,
        this.options,
      );
      await Promise.all(
        bundles.map(
          bundle =>
            writeEarlyPromises[bundle.id] ??
            this.writeToDist({
              bundle,
              info: bundleInfoMap[bundle.id],
              hashRefToNameHash,
              bundleGraph,
            }),
        ),
      );
    } finally {
      loadConfig.clear();
      await dispose();
    }
  }

  async processBundle(
    bundle: InternalBundle,
    bundleGraph: InternalBundleGraph,
    bundleGraphReference: SharedReference,
  ): Promise<{|
    ...BundleInfo,
    cacheKeys: CacheKeyMap,
  |}> {
    let start = Date.now();

    return {
      ...(await this.getBundleInfoFromWorker({
        bundle,
        bundleGraphReference,
        optionsRef: nullthrows(this.optionsRef),
        configRef: nullthrows(this.configRef),
      })),
      time: Date.now() - start,
    };
  }


Review comment (performance):
Can we make this happen after the full build instead of making it packager specific? I'd like to be able to use the config cache during transformation/optimization too. You could add a function to `worker.js` to clear the cache, and call it from `Parcel.js` after a build is complete using the WorkerFarm's `callAllWorkers` function.

Code after:
              bundleGraph,
            });
          }
        }),
      );
      assignComplexNameHashes(
        hashRefToNameHash,
        bundles,
        bundleInfoMap,
        this.options,
      );
      await Promise.all(
        bundles.map(
          bundle =>
            writeEarlyPromises[bundle.id] ??
            this.writeToDist({
              bundle,
              info: bundleInfoMap[bundle.id],
              hashRefToNameHash,
              bundleGraph,
            }),
        ),
      );
    } finally {
      await dispose();
    }
  }

  async processBundle(
    bundle: InternalBundle,
    bundleGraph: InternalBundleGraph,
    bundleGraphReference: SharedReference,
  ): Promise<{|
    ...BundleInfo,
    cacheKeys: CacheKeyMap,
  |}> {
    let start = Date.now();

    return {
      ...(await this.getBundleInfoFromWorker({
        bundle,
        bundleGraphReference,
        optionsRef: nullthrows(this.optionsRef),
        configRef: nullthrows(this.configRef),
      })),
      time: Date.now() - start,
    };
  }

  async loadConfigs(
    bundleGraph: InternalBundleGraph,