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

Code before:
|};

export type ConfigOptions = {|
  parse?: boolean,
|};

const configCache = new LRU<FilePath, ConfigOutput>({max: 500});

export async function resolveConfig(
  fs: FileSystem,
  filepath: FilePath,
  filenames: Array<FilePath>,
  opts: ?ConfigOptions,
  root: FilePath = path.parse(filepath).root,
): Promise<FilePath | null> {
  filepath = await fs.realpath(path.dirname(filepath));

  // 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);
    try {
      if ((await fs.stat(file)).isFile()) {
        return file;
      }
    } catch {
      // empty
    }
  }

  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;

Review comment (performance):
You could also cache whether a particular file exists in a separate map (e.g. Map<FilePath, boolean>). This way we don't have to keep searching the filesystem even when we already have the resolved content cached.

Code after:
|};

export type ConfigOptions = {|
  parse?: boolean,
|};

const configCache = new LRU<FilePath, ConfigOutput>({max: 500});

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

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;
      }
Repository: parcel-bundler/parcel
File: packages/core/workers/src/WorkerFarm.js (JavaScript)

Code before:

    logger.info({
      origin: '@parcel/workers',
      message: escapeMarkdown(`Wrote profile to ${filename}`),
    });
  }

  async callAllWorkers(method: string, args: Array<any>) {
    let promises = [];
    for (let worker of this.workers.values()) {
      promises.push(
        new Promise((resolve, reject) => {
          worker.call({
            method,
            args,
            resolve,
            reject,
            retries: 0,
          });
        }),
      );
    }

    promises.push(this.localWorker[method](this.workerApi, ...args));
    await Promise.all(promises);
    clearConfigCache();
  }

  async takeHeapSnapshot() {
    let snapshotId = getTimeId();

    try {
      let snapshotPaths = await Promise.all(
        [...this.workers.values()].map(
          worker =>
            new Promise((resolve, reject) => {
              worker.call({
                method: 'takeHeapSnapshot',
                args: [snapshotId],
                resolve,
                reject,
                retries: 0,
              });
            }),
        ),
      );

      logger.info({
        origin: '@parcel/workers',
        message: escapeMarkdown(
          'Wrote heap snapshots to the following paths:\n' +

Review comment (performance):
As mentioned, I think you can use this function to call the `clearConfigCache` function in each worker from parcel.js. Something like this:

```js
await this.#farm.callAllWorkers('clearConfigCache', []);
```

This will call the clearConfigCache function in each worker.

Code after:
    logger.info({
      origin: '@parcel/workers',
      message: escapeMarkdown(`Wrote profile to ${filename}`),
    });
  }

  async callAllWorkers(method: string, args: Array<any>) {
    let promises = [];
    for (let worker of this.workers.values()) {
      promises.push(
        new Promise((resolve, reject) => {
          worker.call({
            method,
            args,
            resolve,
            reject,
            retries: 0,
          });
        }),
      );
    }

    promises.push(this.localWorker[method](this.workerApi, ...args));
    await Promise.all(promises);
  }

  async takeHeapSnapshot() {
    let snapshotId = getTimeId();

    try {
      let snapshotPaths = await Promise.all(
        [...this.workers.values()].map(
          worker =>
            new Promise((resolve, reject) => {
              worker.call({
                method: 'takeHeapSnapshot',
                args: [snapshotId],
                resolve,
                reject,
                retries: 0,
              });
            }),
        ),
      );

      logger.info({
        origin: '@parcel/workers',
        message: escapeMarkdown(
          'Wrote heap snapshots to the following paths:\n' +
            snapshotPaths.join('\n'),
        ),
Repository: parcel-bundler/parcel
File: packages/core/workers/src/WorkerFarm.js (JavaScript)

Code before:
  startMaxWorkers(): void {
    // Starts workers until the maximum is reached
    if (this.workers.size < this.options.maxConcurrentWorkers) {
      let toStart = this.options.maxConcurrentWorkers - this.workers.size;
      while (toStart--) {
        this.startChild();
      }
    }
  }

  shouldUseRemoteWorkers(): boolean {
    return (
      !this.options.useLocalWorker ||
      ((this.warmWorkers >= this.workers.size || !this.options.warmWorkers) &&
        this.options.maxConcurrentWorkers > 0)
    );
  }

  createReverseHandle(fn: HandleFunction): Handle {
    let handle = new Handle({fn});
    this.handles.set(handle.id, handle);
    return handle;
  }

  clearConfigCache(): void {
    loadConfig.clear();
  }

  async createSharedReference(
    value: mixed,
    // An optional, pre-serialized representation of the value to be used
    // in its place.
    buffer?: Buffer,
  ): Promise<{|ref: SharedReference, dispose(): Promise<mixed>|}> {
    let ref = referenceId++;
    this.sharedReferences.set(ref, value);
    this.sharedReferencesByValue.set(value, ref);

    let toSend = buffer ? buffer.buffer : value;
    let promises = [];
    for (let worker of this.workers.values()) {
      if (worker.ready) {
        promises.push(worker.sendSharedReference(ref, toSend));
      }
    }

    await Promise.all(promises);

    return {
      ref,
      dispose: () => {

Review comment (performance):
This will only clear the cache on the main thread. We need to send this to all of the workers. See above comment.

Code after:
  startMaxWorkers(): void {
    // Starts workers until the maximum is reached
    if (this.workers.size < this.options.maxConcurrentWorkers) {
      let toStart = this.options.maxConcurrentWorkers - this.workers.size;
      while (toStart--) {
        this.startChild();
      }
    }
  }

  shouldUseRemoteWorkers(): boolean {
    return (
      !this.options.useLocalWorker ||
      ((this.warmWorkers >= this.workers.size || !this.options.warmWorkers) &&
        this.options.maxConcurrentWorkers > 0)
    );
  }

  createReverseHandle(fn: HandleFunction): Handle {
    let handle = new Handle({fn});
    this.handles.set(handle.id, handle);
    return handle;
  }

  async createSharedReference(
    value: mixed,
    // An optional, pre-serialized representation of the value to be used
    // in its place.
    buffer?: Buffer,
  ): Promise<{|ref: SharedReference, dispose(): Promise<mixed>|}> {
    let ref = referenceId++;
    this.sharedReferences.set(ref, value);
    this.sharedReferencesByValue.set(value, ref);

    let toSend = buffer ? buffer.buffer : value;
    let promises = [];
    for (let worker of this.workers.values()) {
      if (worker.ready) {
        promises.push(worker.sendSharedReference(ref, toSend));
      }
    }

    await Promise.all(promises);

    return {
      ref,
      dispose: () => {
        this.sharedReferences.delete(ref);
        this.sharedReferencesByValue.delete(value);
        let promises = [];
        for (let worker of this.workers.values()) {
Repository: parcel-bundler/parcel
File: packages/core/core/src/Parcel.js (JavaScript)

Code before:
    }

    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(),
    ]);
    this.#farm.clearConfigCache();
  }

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

    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;
      }
    }
  }

  async watch(
    cb?: (err: ?Error, buildEvent?: BuildEvent) => mixed,
  ): Promise<AsyncSubscription> {
    if (!this.#initialized) {
      await this._init();
    }

    let watchEventsDisposable;
    if (cb) {
      watchEventsDisposable = this.#watchEvents.addListener(

Review comment (performance):
```suggestion
    await this.#farm.callAllWorkers('clearConfigCache', []);
```

In worker.js, you'll need to add this:

```js
export function clearConfigCache() {
  loadConfig.clearCache();
}
```

Each worker has its own individual cache, so we need to call all of them to clear them. The `callAllWorkers` function will take care of calling the `clearConfigCache` function in worker.js inside each worker.

Code after:
    await Promise.all([
      this.#disposable.dispose(),
      await this.#requestTracker.writeToCache(),
    ]);
    await this.#farm.callAllWorkers('clearConfigCache', []);
  }

  async _startNextBuild() {
    this.#watchAbortController = new AbortController();
    await this.#farm.callAllWorkers('clearConfigCache', []);

    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;
      }
    }
  }

  async watch(
    cb?: (err: ?Error, buildEvent?: BuildEvent) => mixed,
  ): Promise<AsyncSubscription> {
    if (!this.#initialized) {
      await this._init();
    }

    let watchEventsDisposable;
    if (cb) {
      watchEventsDisposable = this.#watchEvents.addListener(
        ({error, buildEvent}) => cb(error, buildEvent),
      );
    }

    if (this.#watcherCount === 0) {
      this.#watcherSubscription = await this._getWatcherSubscription();
      await this.#reporterRunner.report({type: 'watchStart'});

      // Kick off a first build, but don't await its results. Its results will
      // be provided to the callback.
      this.#watchQueue.add(() => this._startNextBuild());
      this.#watchQueue.run();
    }

    this.#watcherCount++;
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/react-refresh.js (JavaScript)

Code before:
    window,
    randoms,
    subscription,
    root;

  b = bundler(entry, {
    inputFS: fs,
    outputFS: fs,
    serve: {
      https: false,
      port,
      host: '127.0.0.1',
    },
    hot: {
      port,
    },
    defaultConfig: path.join(
      __dirname,
      'integration/custom-configs/.parcelrc-dev-server',
    ),
  });

  subscription = await b.watch();
  let bundleEvent: BuildEvent = await getNextBuild(b);
  let bundleGraph;
  if (bundleEvent.type === 'buildSuccess') {
    bundleGraph = bundleEvent.bundleGraph;
  }
  assert.equal(bundleEvent.type, 'buildSuccess');
  let dom = await JSDOM.JSDOM.fromURL(
    'http://127.0.0.1:' + port + '/index.html',
    {
      runScripts: 'dangerously',
      resources: 'usable',
      pretendToBeVisual: true,
    },
  );
  window = dom.window;
  await new Promise(res =>
    window.document.addEventListener('load', () => {
      res();
    }),
  );
  window.console.clear = () => {};
  window.MessageChannel = MessageChannel;
  root = window.document.getElementById('root');

  let bundle = nullthrows(
    nullthrows(bundleGraph)
      .getBundles()
      .find(b => b.type === 'js'),

Review comment (suggestion):
I'm sure this is probably just a hack to get flow to work, but can't this logic just be:

```js
assert.equal(bundleEvent.type, 'buildSuccess');
let bundleGraph = bundleEvent.bundleGraph;
```

Code after:
    b,
    window,
    randoms,
    subscription,
    root;

  b = bundler(entry, {
    inputFS: fs,
    outputFS: fs,
    serve: {
      https: false,
      port,
      host: '127.0.0.1',
    },
    hot: {
      port,
    },
    defaultConfig: path.join(
      __dirname,
      'integration/custom-configs/.parcelrc-dev-server',
    ),
  });

  subscription = await b.watch();
  let bundleEvent: BuildEvent = await getNextBuild(b);
  invariant(bundleEvent.type === 'buildSuccess');
  let bundleGraph = bundleEvent.bundleGraph;
  let dom = await JSDOM.JSDOM.fromURL(
    'http://127.0.0.1:' + port + '/index.html',
    {
      runScripts: 'dangerously',
      resources: 'usable',
      pretendToBeVisual: true,
    },
  );
  window = dom.window;
  await new Promise(res =>
    window.document.addEventListener('load', () => {
      res();
    }),
  );
  window.console.clear = () => {};
  window.MessageChannel = MessageChannel;
  root = window.document.getElementById('root');

  let bundle = nullthrows(bundleGraph.getBundles().find(b => b.type === 'js'));
  let parcelRequire = Object.keys(window).find(k =>
    k.startsWith('parcelRequire'),
  );
  // ReactDOM.render
  await window[parcelRequire](
Repository: parcel-bundler/parcel
File: packages/utils/create-react-app/templates/default/src/parcel/index.js (JavaScript)

Code before:
import React from 'react';
import logoUrl from 'url:./logo.svg';
import frontUrl from 'url:./front.png';
import backUrl from 'url:./back.png';
import './index.css';

export default function Parcel() {
  return (
    <div className="parcel-container">
      <div className="parcel-heading">
        <div>
          <h1>Welcome to</h1>
          <img src={logoUrl} className="parcel-logo" alt="Parcel logo" />
        </div>

        <div className="parcel-box">
          <img src={backUrl} alt="" height="474" />
          <div id="icons" />
          <img
            src={frontUrl}
            className="parcel-box-front"
            alt="Parcel box"
            height="474"
          />
        </div>
      </div>

Review comment (suggestion):
```suggestion
import * as React from 'react';
```

Code after:
import * as React from 'react';
import logoUrl from 'url:./logo.svg';
import frontUrl from 'url:./front.png';
import backUrl from 'url:./back.png';
import './index.css';

export default function Parcel() {
  return (
    <div className="parcel-container">
      <div className="parcel-heading">
        <div>
          <h1>Welcome to</h1>
          <img src={logoUrl} className="parcel-logo" alt="Parcel logo" />
        </div>

        <div className="parcel-box">
          <img src={backUrl} alt="" height="474" />
          <div id="icons" />
          <img
            src={frontUrl}
            className="parcel-box-front"
            alt="Parcel box"
            height="474"
          />
        </div>
      </div>
Repository: parcel-bundler/parcel
File: packages/utils/create-react-app/src/cli.js (JavaScript)

Code before:
      ),
    );
    await fs.promises.writeFile(
      path.join(tempPath, 'package.json'),
      JSON.stringify(
        {
          name: packageName,
          ...packageJson,
        },
        null,
        2,
      ),
    );
  }

  await Promise.all([
    writePackageJson(),
    ncp(path.join(TEMPLATES_DIR, 'default'), tempPath),
  ]);

  log(emoji.progress, 'Installing packages...');
  await installPackages(['parcel@nightly'], {
    cwd: tempPath,
    isDevDependency: true,
  });
  await installPackages(['react', 'react-dom'], {cwd: tempPath});

  log(chalk.green(emoji.progress, 'Creating initial commit...'));
  await git.add('.');
  await git.commit('Initial commit created with @parcel/create-react-app');
}

async function fsExists(filePath: string): Promise<boolean> {
  try {
    return (await fs.promises.stat(filePath)) && true;
  } catch {
    return false;
  }
}

let usesYarn;
async function installPackages(
  packageExpressions: Array<string>,
  opts: {|
    cwd: string,
    isDevDependency?: boolean,
  |},
): Promise<void> {
  log(
    emoji.progress,
    chalk`{dim Installing}`,

Review comment (suggestion):
AFAICT, `@babel/core` and `postcss` aren't installed into the template. So you'd get various Yarn warnings about unsatisfied peer dependencies and if you're lucky even a build error when using CSS modules.

Code after:
      ),
    );
    await fs.promises.writeFile(
      path.join(tempPath, 'package.json'),
      JSON.stringify(
        {
          name: packageName,
          ...packageJson,
        },
        null,
        2,
      ),
    );
  }

  await Promise.all([
    writePackageJson(),
    ncp(path.join(TEMPLATES_DIR, 'default'), tempPath),
  ]);

  log(emoji.progress, 'Installing packages...');
  await installPackages(['parcel@nightly', 'postcss', '@babel/core'], {
    cwd: tempPath,
    isDevDependency: true,
  });
  await installPackages(['react', 'react-dom'], {cwd: tempPath});

  log(chalk.green(emoji.progress, 'Creating initial commit...'));
  await git.add('.');
  await git.commit('Initial commit created with @parcel/create-react-app');
}

async function fsExists(filePath: string): Promise<boolean> {
  try {
    return (await fs.promises.stat(filePath)) && true;
  } catch {
    return false;
  }
}

let usesYarn;
async function installPackages(
  packageExpressions: Array<string>,
  opts: {|
    cwd: string,
    isDevDependency?: boolean,
  |},
): Promise<void> {
  log(
    emoji.progress,
    chalk`{dim Installing}`,
Repository: parcel-bundler/parcel
File: packages/core/core/src/UncommittedAsset.js (JavaScript)

Code before:
      astKey != null &&
        this.options.cache.setBlob(
          astKey,
          // $FlowFixMe
          v8.serialize(this.ast),
        ),
    ]);
    this.value.contentKey = contentKey;
    this.value.mapKey = mapKey;
    this.value.astKey = astKey;
    this.value.outputHash = md5FromString(
      [
        this.value.hash,
        pipelineKey,
        await getInvalidationHash(this.getInvalidations(), this.options),
      ].join(':'),
    );

    if (this.content != null) {
      this.value.stats.size = size;
    }

    this.value.committed = true;
  }

  async commitContent(contentKey: string): number {
    let content = await this.content;
    if (content == null) {
      return 0;
    }

    let size = 0;
    if (content instanceof Readable) {
      await this.options.cache.setStream(
        contentKey,
        content.pipe(
          new TapStream(buf => {
            size += buf.length;
          }),
        ),
      );

      return size;
    }

    if (typeof content === 'string') {
      size = Buffer.byteLength(content);
    } else {
      size = content.length;
    }


Review comment (suggestion):
```suggestion
  async commitContent(contentKey: string): Promise<number> {
```

Code after:
      astKey != null &&
        this.options.cache.setBlob(
          astKey,
          // $FlowFixMe
          v8.serialize(this.ast),
        ),
    ]);
    this.value.contentKey = contentKey;
    this.value.mapKey = mapKey;
    this.value.astKey = astKey;
    this.value.outputHash = md5FromString(
      [
        this.value.hash,
        pipelineKey,
        await getInvalidationHash(this.getInvalidations(), this.options),
      ].join(':'),
    );

    if (this.content != null) {
      this.value.stats.size = size;
    }

    this.value.committed = true;
  }

  async commitContent(contentKey: string): Promise<number> {
    let content = await this.content;
    if (content == null) {
      return 0;
    }

    let size = 0;
    if (content instanceof Readable) {
      await this.options.cache.setStream(
        contentKey,
        content.pipe(
          new TapStream(buf => {
            size += buf.length;
          }),
        ),
      );

      return size;
    }

    if (typeof content === 'string') {
      size = Buffer.byteLength(content);
    } else {
      size = content.length;
    }
Repository: parcel-bundler/parcel
File: packages/core/integration-tests/test/html.js (JavaScript)

Code before:
    await assertBundles(b, [
      {
        type: 'js',
        assets: ['index.html'],
      },
      {
        name: 'index.html',
        assets: ['index.html'],
      },
    ]);

    let html = await outputFS.readFile(
      path.join(distDir, 'index.html'),
      'utf8',
    );
    assert(html.includes('<script type="module">'));
    assert(html.includes('document.write("Hello world")'));
  });

  it('should correctly bundle loaders for nested dynamic imports', async function() {
    let b = await bundle(
      path.join(
        __dirname,
        '/integration/html-js-shared-dynamic-nested/index.html',
      ),
      {mode: 'production', production: true, scopeHoist: true},
    );

    await assertBundles(b, [
      {
        type: 'js',
        assets: [
          'bundle-manifest.js',
          'bundle-url.js',
          'cacheLoader.js',
          'index.js',
          'index.js',
          'index.js',
          'js-loader.js',
          'JSRuntime.js',
          'JSRuntime.js',
          'relative-path.js',
        ],
      },
      {
        name: 'index.html',
        assets: ['index.html'],
      },
      {
        type: 'js',
        assets: ['simpleHasher.js'],

Review comment (suggestion):
Can you remove these `production: true` properties? These are leftovers from Parcel 1, looks like these tests actually once ran in production mode 😄 

Code after:
      path.join(distDir, 'index.html'),
      'utf8',
    );
    assert(html.includes('console.log("test")'));
  });

  it('should support protocol-relative urls', async function() {
    let b = await bundle(
      path.join(__dirname, '/integration/html-protocol-relative/index.html'),
    );

    assertBundles(b, [
      {
        name: 'index.html',
        assets: ['index.html'],
      },
      {
        type: 'css',
        assets: ['index.css'],
      },
    ]);

    for (let bundle of b.getBundles()) {
      let contents = await outputFS.readFile(bundle.filePath, 'utf8');
      assert(contents.includes('//unpkg.com/xyz'));
    }
  });

  it('should support inline <script type="module">', async function() {
    let b = await bundle(
      path.join(__dirname, '/integration/html-inline-js-module/index.html'),
      {scopeHoist: true},
    );

    assertBundles(b, [
      {
        type: 'js',
        assets: ['index.html'],
      },
      {
        name: 'index.html',
        assets: ['index.html'],
      },
    ]);

    let html = await outputFS.readFile(
      path.join(distDir, 'index.html'),
      'utf8',
    );
    assert(html.includes('<script type="module">'));
    assert(html.includes('document.write("Hello world")'));