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

Code before:
  }
}

async function normalizeOptions(command): Promise<InitialParcelOptions> {
  let nodeEnv;
  if (command.name() === 'build') {
    nodeEnv = initialNodeEnv || 'production';
    command.autoinstall = false;
  } else {
    nodeEnv = initialNodeEnv || 'development';
  }

  let https = !!command.https;
  if (command.cert && command.key) {
    https = {
      cert: command.cert,
      key: command.key,
    };
  }

  let serve = false;
  let {port = 1234, host} = command;

  // Throw an error if port value is invalid...
  if (!Number.isInteger(port)) {
    throw new Error(`Defined port is not a valid integer`);
  }

  // Ensure port is usable
  let originalPort = port;
  if (command.name() === 'serve' || command.hmr) {
    port = await getPort({port, host});

    if (port !== originalPort) {
      let errorMessage = `Port "${originalPort}" could not be used`;
      if (command.port != null) {
        // Throw the error if the user defined a custom port
        throw new Error(errorMessage);
      } else {
        // Parcel logger is not set up at this point, so just use native INTERNAL_ORIGINAL_CONSOLE
        INTERNAL_ORIGINAL_CONSOLE.warn(errorMessage);
      }
    }
  }

  if (command.name() === 'serve') {
    let {publicUrl} = command;

    serve = {
      https,
      port,

Review comment (suggestion):
```suggestion
    throw new Error(`Defined port "${port}" is not a valid integer`);
```

Code after:
}

function parsePort(portValue: string): number {
  let parsedPort = Number(portValue);

  // Throw an error if port value is invalid...
  if (!Number.isInteger(parsedPort)) {
    throw new Error(`Port ${portValue} is not a valid integer.`);
  }

  return parsedPort;
}

async function normalizeOptions(command): Promise<InitialParcelOptions> {
  let nodeEnv;
  if (command.name() === 'build') {
    nodeEnv = initialNodeEnv || 'production';
    command.autoinstall = false;
  } else {
    nodeEnv = initialNodeEnv || 'development';
  }

  let https = !!command.https;
  if (command.cert && command.key) {
    https = {
      cert: command.cert,
      key: command.key,
    };
  }

  let serve = false;
  let {host} = command;

  // Ensure port is valid and available
  let port = parsePort(command.port || '1234');
  let originalPort = port;
  if (command.name() === 'serve' || command.hmr) {
    port = await getPort({port, host});

    if (port !== originalPort) {
      let errorMessage = `Port "${port}" could not be used`;
      if (command.port != null) {
        // Throw the error if the user defined a custom port
        throw new Error(errorMessage);
      } else {
        // Parcel logger is not set up at this point, so just use native INTERNAL_ORIGINAL_CONSOLE
        INTERNAL_ORIGINAL_CONSOLE.warn(errorMessage);
      }
    }
  }
Repository: parcel-bundler/parcel
File: packages/utils/babel-preset-env/src/index.js (JavaScript)

Code before:
const semver = require('semver');

const COMPATIBLE_PARCEL_BABEL_TRANSFORMER_SEMVER = '2.0.0-beta.1';

module.exports = function parcelBabelPresetEnv(api, opts) {
  let name = api.caller(caller => caller && caller.name);
  let version = api.caller(caller => caller && caller.version);

  if (
    name === 'parcel' &&
    typeof version === 'string' &&
    semver.satisfies(version, COMPATIBLE_PARCEL_BABEL_TRANSFORMER_SEMVER)
  ) {
    let targets = api.caller(caller => caller && caller.targets);
    if (typeof targets !== 'string') {
      throw new Error('Expected targets to be a string');
    }

    return {
      presets: [
        [
          '@babel/preset-env',
          {
            modules: false,
            targets: JSON.parse(targets),
            ...opts,
          },
        ],

Review comment (bug):
Wouldn't this fail with the nightly?
```
semver.satisfies("2.0.0-nightly.1234", "2.0.0-beta.1") === false
```

Code after:
const semver = require('semver');

const COMPATIBLE_PARCEL_BABEL_TRANSFORMER_SEMVER = '^2.0.0-alpha.1.1';

module.exports = function parcelBabelPresetEnv(api, opts) {
  let name = api.caller(caller => caller && caller.name);
  let version = api.caller(caller => caller && caller.version);

  if (
    name === 'parcel' &&
    typeof version === 'string' &&
    semver.satisfies(version, COMPATIBLE_PARCEL_BABEL_TRANSFORMER_SEMVER)
  ) {
    let targets = api.caller(caller => caller && caller.targets);
    if (typeof targets !== 'string') {
      throw new Error('Expected targets to be a string');
    }

    return {
      presets: [
        [
          '@babel/preset-env',
          {
            modules: false,
            targets: JSON.parse(targets),
            ...opts,
          },
        ],
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/output-formats.js (JavaScript)

Code before:
      );

      let dist = await outputFS.readFile(
        b.getBundles().find(b => b.type === 'js').filePath,
        'utf8',
      );
      assert(dist.includes('export const foo'));
      assert(dist.includes('import "./index.css"'));
    });

    it('should support esmodule output (skipped exports)', async function() {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/formats/esm-isLibrary-false/skipped.js',
        ),
      );

      let dist = await outputFS.readFile(
        b.getBundles().find(b => b.type === 'js').filePath,
        'utf8',
      );
      assert(!dist.includes('foo'));
    });

    it.only('should support interop imports from other bundles', async function() {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/formats/esm-interop-cross-bundle/a.js',
        ),
      );

      assertBundles(b, [
        {
          type: 'js',
          assets: ['a.js', 'c.js', 'JSRuntime.js'],
        },
        {
          type: 'js',
          assets: ['b.js'],
        },
      ]);

      let dist = await outputFS.readFile(
        b.getBundles().find(b => !b.isEntry).filePath,
        'utf8',
      );
      assert(dist.includes('$parcel$interopDefault'));
    });


Review comment (suggestion):
```suggestion
    it('should support interop imports from other bundles', async function() {
```

Code after:
      );

      let dist = await outputFS.readFile(
        b.getBundles().find(b => b.type === 'js').filePath,
        'utf8',
      );
      assert(dist.includes('export const foo'));
      assert(dist.includes('import "./index.css"'));
    });

    it('should support esmodule output (skipped exports)', async function() {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/formats/esm-isLibrary-false/skipped.js',
        ),
      );

      let dist = await outputFS.readFile(
        b.getBundles().find(b => b.type === 'js').filePath,
        'utf8',
      );
      assert(!dist.includes('foo'));
    });

    it('should support interop imports from other bundles', async function() {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/formats/esm-interop-cross-bundle/a.js',
        ),
      );

      assertBundles(b, [
        {
          type: 'js',
          assets: ['a.js', 'c.js', 'JSRuntime.js'],
        },
        {
          type: 'js',
          assets: ['b.js'],
        },
      ]);

      let dist = await outputFS.readFile(
        b.getBundles().find(b => !b.isEntry).filePath,
        'utf8',
      );
      assert(dist.includes('$parcel$interopDefault'));
    });
Repository: parcel-bundler/parcel
File: packages/core/core/src/requests/AssetGraphRequest.js (JavaScript)

Code before:
  AssetGraphNode,
  AssetGroup,
  AssetNode,
  AssetRequestInput,
  Dependency,
  DependencyNode,
  Entry,
  ParcelOptions,
  Target,
} from '../types';
import type {StaticRunOpts, RunAPI} from '../RequestTracker';
import type {EntryResult} from './EntryRequest';
import type {PathRequestInput} from './PathRequest';

import invariant from 'assert';
import path from 'path';
import {
  escapeMarkdown,
  md5FromOrderedObject,
  PromiseQueue,
} from '@parcel/utils';
import ThrowableDiagnostic from '@parcel/diagnostic';
import AssetGraph from '../AssetGraph';
import {PARCEL_VERSION} from '../constants';
import createEntryRequest from './EntryRequest';
import {envCache} from '../Environment';
import createTargetRequest from './TargetRequest';
import createAssetRequest from './AssetRequest';
import createPathRequest from './PathRequest';

import dumpToGraphViz from '../dumpGraphToGraphViz';

type AssetGraphRequestInput = {|
  entries?: Array<string>,
  assetGroups?: Array<AssetGroup>,
  optionsRef: SharedReference,
  name: string,
|};

type RunInput = {|
  input: AssetGraphRequestInput,
  ...StaticRunOpts<{|
    assetGraph: AssetGraph,
    changedAssets: Map<string, Asset>,
    assetRequests: Array<AssetGroup>,
  |}>,
|};

type AssetGraphRequest = {|
  id: string,
  +type: 'asset_graph_request',

Review comment (performance):
feels a bit leaky to export the entire cache map. maybe something like `getEnvironmentById`?

Code after:
  AssetGraphNode,
  AssetGroup,
  AssetNode,
  AssetRequestInput,
  Dependency,
  DependencyNode,
  Entry,
  ParcelOptions,
  Target,
} from '../types';
import type {StaticRunOpts, RunAPI} from '../RequestTracker';
import type {EntryResult} from './EntryRequest';
import type {PathRequestInput} from './PathRequest';

import invariant from 'assert';
import path from 'path';
import {
  escapeMarkdown,
  md5FromOrderedObject,
  PromiseQueue,
} from '@parcel/utils';
import ThrowableDiagnostic from '@parcel/diagnostic';
import AssetGraph from '../AssetGraph';
import {PARCEL_VERSION} from '../constants';
import createEntryRequest from './EntryRequest';
import createTargetRequest from './TargetRequest';
import createAssetRequest from './AssetRequest';
import createPathRequest from './PathRequest';

import dumpToGraphViz from '../dumpGraphToGraphViz';

type AssetGraphRequestInput = {|
  entries?: Array<string>,
  assetGroups?: Array<AssetGroup>,
  optionsRef: SharedReference,
  name: string,
|};

type RunInput = {|
  input: AssetGraphRequestInput,
  ...StaticRunOpts<{|
    assetGraph: AssetGraph,
    changedAssets: Map<string, Asset>,
    assetRequests: Array<AssetGroup>,
  |}>,
|};

type AssetGraphRequest = {|
  id: string,
  +type: 'asset_graph_request',
  run: RunInput => Async<{|
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/webmanifest.js (JavaScript)

Code before:
          {
            codeFrame: {
              code: manifest,
              codeHighlights: [
                {
                  end: {
                    column: 23,
                    line: 5,
                  },
                  start: {
                    column: 14,
                    line: 5,
                  },
                },
              ],
            },
            message,
            filePath: manifestPath,
            origin: '@parcel/core',
          },
          {
            hints: [],
            message: `Cannot load file './icon.png' in '${path.dirname(
              manifestPathRelative,
            )}'.`,
            // origin: '@parcel/resolver-default',
          },
        ],
      },
    );
  });
});


Review comment (suggestion):
The fix for that is
```patch
diff --git packages/core/core/src/requests/PathRequest.js packages/core/core/src/requests/PathRequest.js
index eacced75..3492e4b6 100644
--- packages/core/core/src/requests/PathRequest.js
+++ packages/core/core/src/requests/PathRequest.js
@@ -211,11 +211,11 @@ export class ResolverRunner {
           }

           if (result.diagnostics) {
-            if (Array.isArray(result.diagnostics)) {
-              diagnostics.push(...result.diagnostics);
-            } else {
-              diagnostics.push(result.diagnostics);
-            }
+            let errorDiagnostic = errorToDiagnostic(
+              new ThrowableDiagnostic({diagnostic: result.diagnostics}),
+              resolver.name,
+            );
+            diagnostics.push(...errorDiagnostic);
           }
         }
       } catch (e) {
```

After that, you should be able to uncomment the line.

Code after:
          {
            codeFrame: {
              code: manifest,
              codeHighlights: [
                {
                  end: {
                    column: 23,
                    line: 5,
                  },
                  start: {
                    column: 14,
                    line: 5,
                  },
                },
              ],
            },
            message,
            filePath: manifestPath,
            origin: '@parcel/core',
          },
          {
            hints: [],
            message: `Cannot load file './icon.png' in '${path.dirname(
              manifestPathRelative,
            )}'.`,
            origin: '@parcel/resolver-default',
          },
        ],
      },
    );
  });
});
Repository: parcel-bundler/parcel
File: packages/core/diagnostic/src/diagnostic.js (JavaScript)

Code before:
  | Array<Diagnostic>
  | ThrowableDiagnostic
  | PrintableError
  | string;

/** Normalize the given value into a diagnostic. */
export function anyToDiagnostic(input: Diagnostifiable): Array<Diagnostic> {
  // $FlowFixMe
  let diagnostic: Array<Diagnostic> = input;

  if (input instanceof ThrowableDiagnostic) {
    diagnostic = input.diagnostics;
  } else if (input instanceof Error) {
    diagnostic = errorToDiagnostic(input);
  }

  return Array.isArray(diagnostic) ? diagnostic : [diagnostic];
}

/** Normalize the given error into a diagnostic. */
export function errorToDiagnostic(
  error: ThrowableDiagnostic | PrintableError | string,
  defaultValues: {|
    origin?: ?string,
    filePath?: ?string,
  |} = {},
): Array<Diagnostic> {
  let codeFrame: DiagnosticCodeFrame | void = undefined;

  if (typeof error === 'string') {
    return [
      {
        origin: defaultValues?.origin ?? 'Error',
        message: error,
        codeFrame,
      },
    ];
  }

  if (error instanceof ThrowableDiagnostic) {
    return error.diagnostics.map(d => {
      return {
        ...d,
        origin: d.origin ?? defaultValues?.origin ?? 'unknown',
      };
    });
  }

  if (error.loc && error.source != null) {
    codeFrame = {
      code: error.source,

Review comment (suggestion):
```suggestion
  |} = {...null},
```

Thanks flow...

Code after:
  | Array<Diagnostic>
  | ThrowableDiagnostic
  | PrintableError
  | string;

/** Normalize the given value into a diagnostic. */
export function anyToDiagnostic(input: Diagnostifiable): Array<Diagnostic> {
  // $FlowFixMe
  let diagnostic: Array<Diagnostic> = input;

  if (input instanceof ThrowableDiagnostic) {
    diagnostic = input.diagnostics;
  } else if (input instanceof Error) {
    diagnostic = errorToDiagnostic(input);
  }

  return Array.isArray(diagnostic) ? diagnostic : [diagnostic];
}

/** Normalize the given error into a diagnostic. */
export function errorToDiagnostic(
  error: ThrowableDiagnostic | PrintableError | string,
  defaultValues: {|
    origin?: ?string,
    filePath?: ?string,
  |} = {...null},
): Array<Diagnostic> {
  let codeFrame: DiagnosticCodeFrame | void = undefined;

  if (typeof error === 'string') {
    return [
      {
        origin: defaultValues?.origin ?? 'Error',
        message: error,
        codeFrame,
      },
    ];
  }

  if (error instanceof ThrowableDiagnostic) {
    return error.diagnostics.map(d => {
      return {
        ...d,
        origin: d.origin ?? defaultValues?.origin ?? 'unknown',
      };
    });
  }

  if (error.loc && error.source != null) {
    codeFrame = {
      code: error.source,
Repository: parcel-bundler/parcel
File: packages/core/package-manager/src/installPackage.js (JavaScript)

Code before:
  modules: Array<ModuleRequest>,
  from: FilePath,
  options: InstallOptions = {},
): Promise<void> {
  let {installPeers = true, saveDev = true, packageInstaller} = options;
  let moduleNames = modules.map(m => m.name).join(', ');

  logger.progress(`Installing ${moduleNames}...`);

  let fromPkgPath = await resolveConfig(fs, from, ['package.json']);
  let cwd = fromPkgPath ? path.dirname(fromPkgPath) : fs.cwd();

  if (!packageInstaller) {
    packageInstaller = await determinePackageInstaller(fs, from);
  }

  try {
    await packageInstaller.install({
      modules,
      saveDev,
      cwd,
      packagePath: fromPkgPath,
      fs,
    });
  } catch (err) {
    throw new Error(`Failed to install ${moduleNames}: ` + err.message);
  }

  if (installPeers) {
    await Promise.all(
      modules.map(m =>
        installPeerDependencies(fs, packageManager, m, from, options),
      ),
    );
  }
}

async function installPeerDependencies(
  fs: FileSystem,
  packageManager: PackageManager,
  module: ModuleRequest,
  from: FilePath,
  options,
) {
  const {resolved} = await packageManager.resolve(module.name, from);
  const modulePkg: PackageJSON = nullthrows(
    await loadConfig(fs, resolved, ['package.json']),
  ).config;
  const peers = modulePkg.peerDependencies || {};

  let modules: Array<ModuleRequest> = [];

Review comment (bug):
```suggestion
    throw new Error(`Failed to install ${moduleNames}: ${err.message}`);
```

Code after:
  modules: Array<ModuleRequest>,
  from: FilePath,
  options: InstallOptions = {},
): Promise<void> {
  let {installPeers = true, saveDev = true, packageInstaller} = options;
  let moduleNames = modules.map(m => m.name).join(', ');

  logger.progress(`Installing ${moduleNames}...`);

  let fromPkgPath = await resolveConfig(fs, from, ['package.json']);
  let cwd = fromPkgPath ? path.dirname(fromPkgPath) : fs.cwd();

  if (!packageInstaller) {
    packageInstaller = await determinePackageInstaller(fs, from);
  }

  try {
    await packageInstaller.install({
      modules,
      saveDev,
      cwd,
      packagePath: fromPkgPath,
      fs,
    });
  } catch (err) {
    throw new Error(`Failed to install ${moduleNames}: ${err.message}`);
  }

  if (installPeers) {
    await Promise.all(
      modules.map(m =>
        installPeerDependencies(fs, packageManager, m, from, options),
      ),
    );
  }
}

async function installPeerDependencies(
  fs: FileSystem,
  packageManager: PackageManager,
  module: ModuleRequest,
  from: FilePath,
  options,
) {
  const {resolved} = await packageManager.resolve(module.name, from);
  const modulePkg: PackageJSON = nullthrows(
    await loadConfig(fs, resolved, ['package.json']),
  ).config;
  const peers = modulePkg.peerDependencies || {};

  let modules: Array<ModuleRequest> = [];
Repository: parcel-bundler/parcel
File: packages/core/parcel/src/cli.js (JavaScript)

Code before:
    command.detailedReport = '10';
  }

  let reporters = [];

  repoterOuter: for (let name of (command.reporter: Array<string>)) {
    let potentialNames = [
      name,
      `@parcel/reporter-${name}`,
      `parcel-reporter-${name}`,
    ];
    for (let packageName of potentialNames) {
      try {
        await packageManager.resolveSync(packageName, packageManager.fs.cwd());
        reporters.push(packageName);
        continue repoterOuter;
      } catch (e) {
        // noop
      }
    }
    throw new Error(
      `Could not find reporter for ${name}, tried: ` +
        potentialNames.join(', '),
    );
  }
  console.log(reporters);

  let mode = command.name() === 'build' ? 'production' : 'development';
  return {
    shouldDisableCache: command.cache === false,
    cacheDir: command.cacheDir,
    mode,
    hmrOptions,
    shouldContentHash: hmrOptions ? false : command.shouldContentHash,
    serveOptions,
    targets: command.target.length > 0 ? command.target : null,
    shouldAutoInstall: command.autoinstall ?? true,
    logLevel: command.logLevel,
    shouldProfile: command.profile,
    detailedReport:
      command.detailedReport != null
        ? {
            assetsPerBundle: parseInt(command.detailedReport, 10),
          }
        : null,
    env: {
      NODE_ENV: nodeEnv,
    },
    reporters: reporters,
    defaultTargetOptions: {
      shouldOptimize:

Review comment (suggestion):
```suggestion
  INTERNAL_ORIGINAL_CONSOLE.log(reporters);
```

if you were intending to report the logs

Code after:

    hmrOptions = {port: hmrport, host};
  }

  if (command.detailedReport === true) {
    command.detailedReport = '10';
  }

  let additionalReporters = [
    {packageName: '@parcel/reporter-cli', resolveFrom: __filename},
    {packageName: '@parcel/reporter-dev-server', resolveFrom: __filename},
    ...(command.reporter: Array<string>).map(packageName => ({
      packageName,
      resolveFrom: path.join(inputFS.cwd(), 'index'),
    })),
  ];

  let mode = command.name() === 'build' ? 'production' : 'development';
  return {
    shouldDisableCache: command.cache === false,
    cacheDir: command.cacheDir,
    mode,
    hmrOptions,
    shouldContentHash: hmrOptions ? false : command.contentHash,
    serveOptions,
    targets: command.target.length > 0 ? command.target : null,
    shouldAutoInstall: command.autoinstall ?? true,
    logLevel: command.logLevel,
    shouldProfile: command.profile,
    shouldBuildLazily: command.lazy,
    detailedReport:
      command.detailedReport != null
        ? {
            assetsPerBundle: parseInt(command.detailedReport, 10),
          }
        : null,
    env: {
      NODE_ENV: nodeEnv,
    },
    additionalReporters,
    defaultTargetOptions: {
      shouldOptimize:
        command.optimize != null ? command.optimize : mode === 'production',
      sourceMaps: command.sourceMaps ?? true,
      shouldScopeHoist: command.scopeHoist,
      publicUrl: command.publicUrl,
      distDir: command.distDir,
    },
  };
}