ronantakizawa/codereview-bench huggingface dataset:
Repository: parcel-bundler/parcel
File: packages/reporters/bundle-analyzer/client/index.js (JavaScript)

Code before:
function debounce(fn, delay) {
  let timeout;

  return function(...args) {
    if (timeout) {
      clearTimeout(timeout);
    }

    timeout = setTimeout(() => {
      fn(...args);
    }, delay);
  };
}

function translate3d(x, y, z) {
  return `translate3d(${x}px, ${y}px, ${z}px)`;
}

function formatSize(x){
  const units = ['bytes', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1024 && ++l){
      n /= 1024;
  }
  return(`${n.toFixed(n < 10 && l > 0 ? 1 : 0)} ${units[l]}` + (l ? ` (${x} bytes)` : ''));
}


Review comment (suggestion):
Parcel is using [`filesize`](https://github.com/parcel-bundler/parcel/blob/18b038d57043965afb31077db1f46dadb11a6a78/packages/reporters/cli/package.json#L39), therefore it would be better to have the same behavior:

```suggestion
function formatSize(x){
  const units = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1000 && ++l){
      n /= 1000;
  }
  return(`${n.toFixed(n < 10 && l > 0 ? 1 : 0)} ${units[l]}`);
}
```

Code after:
function debounce(fn, delay) {
  let timeout;

  return function(...args) {
    if (timeout) {
      clearTimeout(timeout);
    }

    timeout = setTimeout(() => {
      fn(...args);
    }, delay);
  };
}

function translate3d(x, y, z) {
  return `translate3d(${x}px, ${y}px, ${z}px)`;
}

const UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
function formatSize(x) {
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1000 && ++l){
      n /= 1000;
  }
  return(`${n.toFixed(l > 0 ? 2 : 0)} ${UNITS[l]}`);
}
Repository: parcel-bundler/parcel
File: packages/reporters/bundle-analyzer/client/index.js (JavaScript)

Code before:
function debounce(fn, delay) {
  let timeout;

  return function(...args) {
    if (timeout) {
      clearTimeout(timeout);
    }

    timeout = setTimeout(() => {
      fn(...args);
    }, delay);
  };
}

function translate3d(x, y, z) {
  return `translate3d(${x}px, ${y}px, ${z}px)`;
}

function formatSize(x){
  const units = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1000 && ++l){
      n /= 1000;
  }
  return(`${n.toFixed(l > 0 ? 2 : 0)} ${units[l]}`);
}


Review comment (suggestion):
```suggestion
const UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
function formatSize(x) {
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1000 && ++l){
      n /= 1000;
  }
  return(`${n.toFixed(l > 0 ? 2 : 0)} ${UNITS[l]}`);
}
```

Code after:
function debounce(fn, delay) {
  let timeout;

  return function(...args) {
    if (timeout) {
      clearTimeout(timeout);
    }

    timeout = setTimeout(() => {
      fn(...args);
    }, delay);
  };
}

function translate3d(x, y, z) {
  return `translate3d(${x}px, ${y}px, ${z}px)`;
}

const UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'];
function formatSize(x) {
  let l = 0, n = parseInt(x, 10) || 0;
  while(n >= 1000 && ++l){
      n /= 1000;
  }
  return(`${n.toFixed(l > 0 ? 2 : 0)} ${UNITS[l]}`);
}
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/scope-hoisting.js (JavaScript)

Code before:
      );

      assert.equal(await run(b), 'bar:bar');
    });

    it('should not insert interop default for commonjs modules with default export', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-require-commonjs/a.js',
        ),
      );

      let output = await run(b);
      let obj = {
        test: 2,
      };
      obj.default = obj;
      assert.deepEqual(output.default, obj);
    });

    it('should add a default interop for a CJS module', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-commonjs/a.js',
        ),
      );

      let output = await run(b);
      assert.equal(output, 2);
    });

    it('should not insert default interop for wrapped CJS modules', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-require-commonjs-wrapped/a.js',
        ),
      );

      let output = await run(b);
      assert.equal(output, 'default');
    });

    it('should support multiple requires in the same variable declaration', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/require-multiple/a.js',
        ),

Review comment (suggestion):
More specificity in the fixture name could help, too:
```suggestion
          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js',
```

Code after:
      assert(!contents.includes('bar'));
    });

    it('supports removing an unused inline export with uglify minification', async function () {
      // Uglify does strange things to multiple assignments in a line.
      // See https://github.com/parcel-bundler/parcel/issues/1549
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/export-local/a.js',
        ),
        {
          defaultTargetOptions: {
            shouldOptimize: true,
          },
        },
      );

      let output = await run(b);
      assert.deepEqual(output, 3);
    });

    it('should support sideEffects: false', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/side-effects-false/a.js',
        ),
      );

      let output = await run(b);
      assert.deepEqual(output, 9);
    });

    it('can bundle browserify-produced umd bundles', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/browserify-compat/index.js',
        ),
      );

      assert.equal(await run(b), 'foo');
    });

    it('replaces properties of require with undefined', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/require-extensions/index.js',
        ),
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/scope-hoisting.js (JavaScript)

Code before:
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-require-commonjs/a.js',
        ),
      );

      let output = await run(b);
      let obj = {
        test: 2,
      };
      obj.default = obj;
      assert.deepEqual(output.default, obj);
    });

    it('should add a default interop for a CJS module used in a hybrid module', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid/a.js',
        ),
      );

      let output = await run(b);
      assert.equal(output, 2);
    });

    it('should add a default interop for a CJS module used dynamically in a hybrid module', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-commonjs-hybrid-dynamic/a.js',
        ),
      );

      let output = await run(b);
      assert.equal(output, 2);
    });

    it('should not insert default interop for wrapped CJS modules', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/interop-require-commonjs-wrapped/a.js',
        ),
      );

      let output = await run(b);
      assert.equal(output, 'default');
    });

    it('should support multiple requires in the same variable declaration', async function () {
      let b = await bundle(

Review comment (suggestion):
```suggestion
    it('should add a default interop for a CJS module used non-statically in a hybrid module', async function () {
```

Code after:
          __dirname,
          '/integration/scope-hoisting/commonjs/export-local/a.js',
        ),
        {
          defaultTargetOptions: {
            shouldOptimize: true,
          },
        },
      );

      let output = await run(b);
      assert.deepEqual(output, 3);
    });

    it('should support sideEffects: false', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/side-effects-false/a.js',
        ),
      );

      let output = await run(b);
      assert.deepEqual(output, 9);
    });

    it('can bundle browserify-produced umd bundles', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/browserify-compat/index.js',
        ),
      );

      assert.equal(await run(b), 'foo');
    });

    it('replaces properties of require with undefined', async function () {
      let b = await bundle(
        path.join(
          __dirname,
          '/integration/scope-hoisting/commonjs/require-extensions/index.js',
        ),
      );

      await run(b);
    });

    it('should support two aliases to the same module', async function () {
      let b = await bundle(
        path.join(
Repository: parcel-bundler/parcel
File: packages/core/core/src/Dependency.js (JavaScript)

Code before:
      ),
  };
}

export function mergeDependencies(a: Dependency, b: Dependency): void {
  let {
    meta,
    symbols,
    needsStableName,
    isEntry,
    isOptional,
    bundleBehavior,
    ...other
  } = b;
  Object.assign(a, other);
  Object.assign(a.meta, meta);
  if (a.symbols && symbols) {
    for (let [k, v] of symbols) {
      a.symbols.set(k, v);
    }
  }
  if (needsStableName) a.needsStableName = true;
  if (isEntry) a.isEntry = true;
  if (!isOptional) a.isOptional = false;
  if (
    bundleBehavior == BundleBehavior['isolated'] ||
    a.bundleBehavior == null
  ) {
    a.bundleBehavior = bundleBehavior;
  }
}


Review comment (suggestion):
Probably shouldn't be specific to isolated, inline would have the same issue. But I'm thinking this should probably produce two different dependencies (i.e. be included in the dependency id), and we should let the bundler handle it just like if the dep were in two different files.

Code after:
        ]),
      ),
  };
}

export function mergeDependencies(a: Dependency, b: Dependency): void {
  let {
    meta,
    symbols,
    needsStableName,
    isEntry,
    isOptional,
    ...other
  } = b;
  Object.assign(a, other);
  Object.assign(a.meta, meta);
  if (a.symbols && symbols) {
    for (let [k, v] of symbols) {
      a.symbols.set(k, v);
    }
  }
  if (needsStableName) a.needsStableName = true;
  if (isEntry) a.isEntry = true;
  if (!isOptional) a.isOptional = false;
}
Repository: parcel-bundler/parcel
File: packages/core/types/index.js (JavaScript)

Code before:
    options: PluginOptions,
    logger: PluginLogger,
  |}) => boolean,
  /** Parse the contents into an ast */
  parse?: ({|
    asset: Asset,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<?AST>,
  /** Transform the asset and/or add new assets */
  transform({|
    asset: MutableAsset,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}): Async<Array<TransformerResult | MutableAsset>>,
  /** Do some processing after the transformation  */
  postProcess?: ({|
    assets: Array<MutableAsset>,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<Array<TransformerResult>>,
  /** Stringify the AST */
  generate?: ({|
    asset: Asset,
    ast: AST,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<GenerateOutput>,
|};

/**
 * Used to control a traversal
 * @section bundler
 */
export type TraversalActions = {|
  /** Skip the current node's children and continue the traversal if there are other nodes in the queue. */
  skipChildren(): void,
  /** Stop the traversal */
  stop(): void,
|};

/**
 * Essentially GraphTraversalCallback, but allows adding specific node enter and exit callbacks.
 * @section bundler
 */

Review comment (suggestion):
other than `config` type on this line and some comments and some invalidation logic are completely same to https://github.com/parcel-bundler/parcel/pull/5674/files

Code after:
    options: PluginOptions,
    logger: PluginLogger,
  |}) => boolean,
  /** Parse the contents into an ast */
  parse?: ({|
    asset: Asset,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<?AST>,
  /** Transform the asset and/or add new assets */
  transform({|
    asset: MutableAsset,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}): Async<Array<TransformerResult | MutableAsset>>,
  /**
   * Do some processing after the transformation
   * @experimental
   */
  postProcess?: ({|
    assets: Array<MutableAsset>,
    config: ConfigType,
    resolve: ResolveFn,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<Array<TransformerResult>>,
  /** Stringify the AST */
  generate?: ({|
    asset: Asset,
    ast: AST,
    options: PluginOptions,
    logger: PluginLogger,
  |}) => Async<GenerateOutput>,
|};

/**
 * Used to control a traversal
 * @section bundler
 */
export type TraversalActions = {|
  /** Skip the current node's children and continue the traversal if there are other nodes in the queue. */
  skipChildren(): void,
  /** Stop the traversal */
  stop(): void,
|};

/**
Repository: parcel-bundler/parcel
File: packages/transformers/vue/src/VueTransformer.js (JavaScript)

Code before:
    // TODO: This parses the vue component multiple times. Fix?
    let code = await asset.getCode();
    let parsed = compiler.parse(code, {
      sourceMap: true,
      filename: asset.filePath,
    });
    if (parsed.errors.length) {
      throw new ThrowableDiagnostic({
        diagnostic: parsed.errors.map(err => {
          return createDiagnostic(err, asset.filePath);
        }),
      });
    }

    const descriptor = parsed.descriptor;
    let id = hashObject({
      filePath: asset.filePath,
    }).slice(-6);

    return {
      type: 'vue',
      version: '3.0.0',
      program: {
        ...descriptor,
        script:
          !!descriptor.script || !!descriptor.scriptSetup
            ? compiler.compileScript(descriptor, {
                id,
                isProd: options.mode === 'production',
              })
            : null,
      },
    };
  },
  async transform({asset, options, resolve, config}) {
    let id = hashObject({
      filePath: asset.filePath,
    }).slice(-6);
    let scopeId = 'data-v-' + id;
    let hmrId = id + '-hmr';
    let basePath = basename(asset.filePath);
    let {template, script, styles, customBlocks} = nullthrows(
      await asset.getAST(),
    ).program;
    if (asset.pipeline != null) {
      return processPipeline({
        asset,
        template,
        script,
        styles,
        customBlocks,

Review comment (suggestion):
Could this be rewritten as `... != null || ... != null`? I'd prefer that over `!!`

Code after:
  async parse({asset, options}) {
    // TODO: This parses the vue component multiple times. Fix?
    let code = await asset.getCode();
    let parsed = compiler.parse(code, {
      sourceMap: true,
      filename: asset.filePath,
    });
    if (parsed.errors.length) {
      throw new ThrowableDiagnostic({
        diagnostic: parsed.errors.map(err => {
          return createDiagnostic(err, asset.filePath);
        }),
      });
    }

    const descriptor = parsed.descriptor;
    let id = hashObject({
      filePath: asset.filePath,
      source: options.mode === 'production' ? code : null,
    }).slice(-6);

    return {
      type: 'vue',
      version: '3.0.0',
      program: {
        ...descriptor,
        script:
          descriptor.script != null || descriptor.scriptSetup != null
            ? compiler.compileScript(descriptor, {
                id,
                isProd: options.mode === 'production',
              })
            : null,
        id,
      },
    };
  },
  async transform({asset, options, resolve, config}) {
    let {template, script, styles, customBlocks, id} = nullthrows(
      await asset.getAST(),
    ).program;
    let scopeId = 'data-v-' + id;
    let hmrId = id + '-hmr';
    let basePath = basename(asset.filePath);
    if (asset.pipeline != null) {
      return processPipeline({
        asset,
        template,
        script,
        styles,
        customBlocks,
Repository: parcel-bundler/parcel
File: packages/core/graph/src/AdjacencyList.js (JavaScript)

Code before:
        edges.push({to, type});
        edge = this.#edges.nextOut(edge);
      }
      node = this.#nodes.next(node);
    }
    return edges;
  }

  /**
   * Get the list of nodes connected from this node.
   */
  getNodeIdsConnectedFrom(
    from: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||
      (Array.isArray(type)
        ? type.includes(this.#nodes.typeOf(node))
        : type === this.#nodes.typeOf(node));

    let nodes = new Set<NodeId>();
    let node = this.#nodes.head(from);
    while (node !== null) {
      if (matches(node)) {
        let edge = this.#nodes.firstOut(node);
        while (edge !== null) {
          nodes.add(this.#edges.to(edge));
          edge = this.#edges.nextOut(edge);
        }
      }
      node = this.#nodes.next(node);
    }
    return [...nodes];
  }

  /**
   * Get the list of nodes connected to this node.
   */
  getNodeIdsConnectedTo(
    to: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {

Review comment (suggestion):
```suggestion
    let nodes = [];
    let seen = new Set<NodeId>();
```

Code after:
        edges.push({to, type});
        edge = this.#edges.nextOut(edge);
      }
      node = this.#nodes.next(node);
    }
    return edges;
  }

  /**
   * Get the list of nodes connected from this node.
   */
  getNodeIdsConnectedFrom(
    from: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||
      (Array.isArray(type)
        ? type.includes(this.#nodes.typeOf(node))
        : type === this.#nodes.typeOf(node));
    let nodes = [];
    let seen = new Set<NodeId>();
    let node = this.#nodes.head(from);
    while (node !== null) {
      if (matches(node)) {
        let edge = this.#nodes.firstOut(node);
        while (edge !== null) {
          let to = this.#edges.to(edge);
          if (!seen.has(to)) {
            nodes.push(to);
            seen.add(to);
          }
          edge = this.#edges.nextOut(edge);
        }
      }
      node = this.#nodes.next(node);
    }
    return nodes;
  }

  /**
   * Get the list of nodes connected to this node.
   */
  getNodeIdsConnectedTo(
    to: NodeId,
    type:
      | AllEdgeTypes
Repository: parcel-bundler/parcel
File: packages/core/graph/src/AdjacencyList.js (JavaScript)

Code before:
  }

  /**
   * Get the list of nodes connected from this node.
   */
  getNodeIdsConnectedFrom(
    from: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||
      (Array.isArray(type)
        ? type.includes(this.#nodes.typeOf(node))
        : type === this.#nodes.typeOf(node));

    let nodes = new Set<NodeId>();
    let node = this.#nodes.head(from);
    while (node !== null) {
      if (matches(node)) {
        let edge = this.#nodes.firstOut(node);
        while (edge !== null) {
          nodes.add(this.#edges.to(edge));
          edge = this.#edges.nextOut(edge);
        }
      }
      node = this.#nodes.next(node);
    }
    return [...nodes];
  }

  /**
   * Get the list of nodes connected to this node.
   */
  getNodeIdsConnectedTo(
    to: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||
      (Array.isArray(type)
        ? type.includes(this.#nodes.typeOf(node))
        : type === this.#nodes.typeOf(node));


Review comment (suggestion):
```suggestion
          let to = this.#edges.to(edge);
          if (!seen.has(to)) {
            nodes.push(to);
            seen.add(to);
          }
```

Code after:
  }

  /**
   * Get the list of nodes connected from this node.
   */
  getNodeIdsConnectedFrom(
    from: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||
      (Array.isArray(type)
        ? type.includes(this.#nodes.typeOf(node))
        : type === this.#nodes.typeOf(node));
    let nodes = [];
    let seen = new Set<NodeId>();
    let node = this.#nodes.head(from);
    while (node !== null) {
      if (matches(node)) {
        let edge = this.#nodes.firstOut(node);
        while (edge !== null) {
          let to = this.#edges.to(edge);
          if (!seen.has(to)) {
            nodes.push(to);
            seen.add(to);
          }
          edge = this.#edges.nextOut(edge);
        }
      }
      node = this.#nodes.next(node);
    }
    return nodes;
  }

  /**
   * Get the list of nodes connected to this node.
   */
  getNodeIdsConnectedTo(
    to: NodeId,
    type:
      | AllEdgeTypes
      | TEdgeType
      | NullEdgeType
      | Array<TEdgeType | NullEdgeType> = 1,
  ): NodeId[] {
    let matches = node =>
      type === ALL_EDGE_TYPES ||