ronantakizawa/codereview-bench huggingface dataset:
Repository: angular/angular
File: adev/src/content/examples/drag-drop/src/root-element/app/app.component.ts (TypeScript)

Code before:
import {CdkDrag} from '@angular/cdk/drag-drop';
import {Overlay, OverlayRef} from '@angular/cdk/overlay';
import {TemplatePortal} from '@angular/cdk/portal';
import {
  AfterViewInit,
  Component,
  OnDestroy,
  TemplateRef,
  ViewContainerRef,
  inject,
  viewChild,
} from '@angular/core';

/**
 * @title Drag&Drop with alternate root element
 */
@Component({
  selector: 'cdk-drag-drop-root-element-example',
  templateUrl: 'app.component.html',
  styleUrl: 'app.component.css',
  imports: [CdkDrag],
})
export class CdkDragDropRootElementExample implements AfterViewInit, OnDestroy {
  private _overlay = inject(Overlay);
  private _viewContainerRef = inject(ViewContainerRef);
  _dialogTemplate = viewChild.required(TemplateRef);
  private _overlayRef!: OverlayRef;
  private _portal!: TemplatePortal;

  ngAfterViewInit() {
    this._portal = new TemplatePortal(this._dialogTemplate(), this._viewContainerRef);
    this._overlayRef = this._overlay.create({
      positionStrategy: this._overlay.position().global().centerHorizontally().centerVertically(),
      hasBackdrop: true,
    });
    this._overlayRef.backdropClick().subscribe(() => this._overlayRef.detach());
  }

  ngOnDestroy() {
    this._overlayRef.dispose();
  }

  openDialog() {
    this._overlayRef.attach(this._portal);
  }
}


Review comment (suggestion):
nit: Could you also make it private, it's unused in the template (and also that was hinted by the leading underscore. 
```suggestion
  private _dialogTemplate = viewChild.required(TemplateRef);
```

Code after:
import {CdkDrag} from '@angular/cdk/drag-drop';
import {Overlay, OverlayRef} from '@angular/cdk/overlay';
import {TemplatePortal} from '@angular/cdk/portal';
import {
  AfterViewInit,
  Component,
  OnDestroy,
  TemplateRef,
  ViewContainerRef,
  inject,
  viewChild,
} from '@angular/core';

/**
 * @title Drag&Drop with alternate root element
 */
@Component({
  selector: 'cdk-drag-drop-root-element-example',
  templateUrl: 'app.component.html',
  styleUrl: 'app.component.css',
  imports: [CdkDrag],
})
export class CdkDragDropRootElementExample implements AfterViewInit, OnDestroy {
  private _overlay = inject(Overlay);
  private _viewContainerRef = inject(ViewContainerRef);
  private _dialogTemplate = viewChild.required(TemplateRef);
  private _overlayRef!: OverlayRef;
  private _portal!: TemplatePortal;

  ngAfterViewInit() {
    this._portal = new TemplatePortal(this._dialogTemplate(), this._viewContainerRef);
    this._overlayRef = this._overlay.create({
      positionStrategy: this._overlay.position().global().centerHorizontally().centerVertically(),
      hasBackdrop: true,
    });
    this._overlayRef.backdropClick().subscribe(() => this._overlayRef.detach());
  }

  ngOnDestroy() {
    this._overlayRef.dispose();
  }

  openDialog() {
    this._overlayRef.attach(this._portal);
  }
}
Repository: angular/angular
File: adev/shared-docs/interfaces/navigation-item.ts (TypeScript)

Code before:
/*!
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

export interface NavigationItem {
  label?: string;
  path?: string;
  children?: NavigationItem[];
  isExternal?: boolean;
  isExpanded?: boolean;
  level?: number;
  parent?: NavigationItem;
  contentPath?: string;
  status?: 'new' | 'updated';
  category?: string;
  sortCategories?: false; // true by default
}


Review comment (bug):
Maybe we can name this something like `preserveOtherCategoryOrder` that more accurately captures that this is only related to the "other" category and makes the `false` the natural default. I would also change the type to `boolean`, there's nothing wrong with explicitly setting the default in the json right?

Code after:
/*!
 * @license
 * Copyright Google LLC All Rights Reserved.
 *
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.dev/license
 */

export interface NavigationItem {
  label?: string;
  path?: string;
  children?: NavigationItem[];
  isExternal?: boolean;
  isExpanded?: boolean;
  level?: number;
  parent?: NavigationItem;
  contentPath?: string;
  status?: 'new' | 'updated';
  category?: string;
  preserveOtherCategoryOrder?: boolean; // true by default
}
Repository: angular/angular
File: packages/core/schematics/rollup.config.js (JavaScript)

Code before:
    if (!banner) {
      return;
    }

    const [bannerContent] = banner;
    const pos = code.indexOf(bannerContent);
    if (pos !== -1) {
      const result = code.slice(0, pos) + code.slice(pos + bannerContent.length);

      return {
        code: result.trimStart(),
        map: null,
      };
    }

    return {
      code: code,
      map: null,
    };
  },
};

const banner = `'use strict';
/**
 * @license Angular v0.0.0-PLACEHOLDER
 * (c) 2010-2026 Google LLC. https://angular.io/
 * License: MIT
 */`;

const plugins = [
  pathPlugin({tsconfigPath: 'packages/core/schematics/tsconfig.json'}),
  nodeResolve({
    jail: process.cwd(),
  }),
  stripBannerPlugin,
  commonjs(),
];

/** @type {import('rollup').RollupOptions} */
const config = {
  plugins,
  external: ['typescript', 'tslib', /@angular-devkit\/.+/, /@angular\//],
  output: {
    exports: 'auto',
    chunkFileNames: '[name]-[hash].cjs',
    entryFileNames: '[name].cjs',
    banner,
  },
};

module.exports = config;

Review comment (suggestion):
```suggestion
 * (c) 2010-2026 Google LLC. https://angular.dev/
```

Code after:
    if (!banner) {
      return;
    }

    const [bannerContent] = banner;
    const pos = code.indexOf(bannerContent);
    if (pos !== -1) {
      const result = code.slice(0, pos) + code.slice(pos + bannerContent.length);

      return {
        code: result.trimStart(),
        map: null,
      };
    }

    return {
      code: code,
      map: null,
    };
  },
};

const banner = `'use strict';
/**
 * @license Angular v0.0.0-PLACEHOLDER
 * (c) 2010-2026 Google LLC. https://angular.dev/
 * License: MIT
 */`;

const plugins = [
  pathPlugin({tsconfigPath: 'packages/core/schematics/tsconfig.json'}),
  nodeResolve({
    jail: process.cwd(),
  }),
  stripBannerPlugin,
  commonjs(),
];

/** @type {import('rollup').RollupOptions} */
const config = {
  plugins,
  external: ['typescript', 'tslib', /@angular-devkit\/.+/, /@angular\//],
  output: {
    exports: 'auto',
    chunkFileNames: '[name]-[hash].cjs',
    entryFileNames: '[name].cjs',
    banner,
  },
};

module.exports = config;
Repository: angular/angular
File: packages/core/src/hydration/event_replay.ts (TypeScript)

Code before:
}

export function invokeRegisteredReplayListeners(
  injector: Injector,
  event: Event,
  currentTarget: Element | null,
) {
  const blockName =
    (currentTarget && currentTarget.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE)) ?? '';
  if (/d\d+/.test(blockName)) {
    const queue = injector.get(EVENT_REPLAY_QUEUE);
    hydrateAndInvokeBlockListeners(blockName, injector, event, currentTarget!, queue);
  } else if (event.eventPhase === EventPhase.REPLAY) {
    invokeListeners(event, currentTarget);
  }
}

function hydrateAndInvokeBlockListeners(
  blockName: string,
  injector: Injector,
  event: Event,
  currentTarget: Element,
  queue: EventReplayQueue,
) {
  queue.push({event, currentTarget});
  triggerHydrationFromBlockName(injector, blockName, replayQueuedBlockEvents(queue));
}

function replayQueuedBlockEvents(queue: EventReplayQueue) {
  return (hydratedBlocks: string[]) => {
    const hydrated = new Set<string>(hydratedBlocks);
    const newQueue: EventReplayQueue = [];
    for (let {event, currentTarget} of queue) {
      const blockName = currentTarget.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE)!;
      if (hydrated.has(blockName)) {
        invokeListeners(event, currentTarget);
      } else {
        // requeue events that weren't yet hydrated
        newQueue.push({event, currentTarget});
      }
    }
    queue.length = 0;
    queue.push(...newQueue);
  };
}


Review comment (suggestion):
nit: it'd be great to rename the function to match its new behavior:
```suggestion
  triggerHydrationFromBlockName(injector, blockName, createReplayQueuedBlockEventsFn(queue));
```

Code after:
}

export function invokeRegisteredReplayListeners(
  injector: Injector,
  event: Event,
  currentTarget: Element | null,
) {
  const blockName =
    (currentTarget && currentTarget.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE)) ?? '';
  if (/d\d+/.test(blockName)) {
    hydrateAndInvokeBlockListeners(blockName, injector, event, currentTarget!);
  } else if (event.eventPhase === EventPhase.REPLAY) {
    invokeListeners(event, currentTarget);
  }
}

function hydrateAndInvokeBlockListeners(
  blockName: string,
  injector: Injector,
  event: Event,
  currentTarget: Element,
) {
  const queue = injector.get(EVENT_REPLAY_QUEUE);
  queue.push({event, currentTarget});
  triggerHydrationFromBlockName(injector, blockName, createReplayQueuedBlockEventsFn(queue));
}

function createReplayQueuedBlockEventsFn(queue: EventReplayQueue) {
  return (hydratedBlocks: string[]) => {
    const hydrated = new Set<string>(hydratedBlocks);
    const newQueue: EventReplayQueue = [];
    for (let {event, currentTarget} of queue) {
      const blockName = currentTarget.getAttribute(DEFER_BLOCK_SSR_ID_ATTRIBUTE)!;
      if (hydrated.has(blockName)) {
        invokeListeners(event, currentTarget);
      } else {
        // requeue events that weren't yet hydrated
        newQueue.push({event, currentTarget});
      }
    }
    queue.length = 0;
    queue.push(...newQueue);
  };
}
Repository: angular/angular
File: packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts (TypeScript)

Code before:
          }
        `;

      expect(tcb(TEMPLATE, undefined, {checkControlFlowBodies: false})).toContain(
        'switch (((this).expr)) { ' + 'case 1: break; ' + 'case 2: break; ' + 'default: break; }',
      );
    });

    it('should generate a switch block with exhaustiveness checking', () => {
      const TEMPLATE = `
        @switch (expr) {
          @case (1) {
            {{one()}}
          }
          @case (2) {
            {{two()}}
          }
          @default never;
        }
      `;

      expect(tcb(TEMPLATE)).toContain(
        'switch (((this).expr)) { ' +
          'case 1: "" + ((this).one()); break; ' +
          'case 2: "" + ((this).two()); break; ' +
          'default: const tcbExhaustive_1: never = ((this).expr);',
      );
    });

    it('should not generate exhaustiveness checking when there is a consecutive default case', () => {
      const TEMPLATE = `
        @switch (expr) {
          @case (1) {
            {{one()}}
          }
          @case (2)
          @default {
            {{default()}}
          }
        }
      `;

      expect(tcb(TEMPLATE)).toContain(
        'switch (((this).expr)) { ' +
          'case 1: "" + ((this).one()); break; ' +
          'case 2: ' +
          'default: "" + ((this).default()); break; }',
      );
    });

    it('should generate the right TCB if default is not the last case', () => {

Review comment (suggestion):
Might be worth checking if this won't cause apps with [`noUnusedLocals`](https://www.typescriptlang.org/tsconfig/#noUnusedLocals) to get weird diagnostics.

Code after:
          }
        `;

      expect(tcb(TEMPLATE, undefined, {checkControlFlowBodies: false})).toContain(
        'switch (((this).expr)) { ' + 'case 1: break; ' + 'case 2: break; ' + 'default: break; }',
      );
    });

    it('should generate a switch block with exhaustiveness checking', () => {
      const TEMPLATE = `
        @switch (expr) {
          @case (1) {
            {{one()}}
          }
          @case (2) {
            {{two()}}
          }
          @default never;
        }
      `;

      expect(tcb(TEMPLATE)).toContain(
        'switch (((this).expr)) { ' +
          'case 1: "" + ((this).one()); break; ' +
          'case 2: "" + ((this).two()); break; ' +
          'default: const tcbExhaustive_1: never = ((this).expr);',
      );
    });

    it('should not report unused locals for exhaustiveness check variable', () => {
      const TEMPLATE = `
        @switch (expr) {
          @case (1) {}
          @default never;
        }
      `;
      const SOURCE = `
        export class TestComponent {
          expr!: 1|2; 
        }
      `;
      expect(diagnose(TEMPLATE, SOURCE, undefined, [], undefined, {noUnusedLocals: true})).toEqual([
        `TestComponent.html(2, 18): Type '2' is not assignable to type 'never'.`,
      ]);
    });

    it('should not generate exhaustiveness checking when there is a consecutive default case', () => {
      const TEMPLATE = `
        @switch (expr) {
          @case (1) {
            {{one()}}
Repository: angular/angular
File: adev/shared-docs/pipeline/shared/shiki.mts (TypeScript)

Code before:
      if (highlight?.has(lineNumber)) {
        this.addClassToHast(node, 'highlighted');
      }
    },
  };
}

/** A custom transformer which removes all of the whitespace between lines of code in the generated output. */
function removeWhitespaceTransformer(highlight?: Set<number>): ShikiTransformer {
  return {
    code(code) {
      code.children = code.children.filter(
        (line) => line.type !== 'text' || line.value.trim().length !== 0,
      );
    },
  };
}

/** A custom transformer which removes all of the whitespace between lines of code in the generated output. */
function linkApiEntriesTransformer(apiEntries?: ApiEntries): ShikiTransformer {
  if (apiEntries === undefined) {
    return {};
  }
  return {
    preprocess(code, options) {
      options.decorations ||= [];
      scanner.setText(code);
      let token = scanner.scan();

      while (token !== SyntaxKind.EndOfFileToken) {
        if (token === SyntaxKind.Identifier) {
          const symbolUrl = getSymbolUrl(scanner.getTokenText(), apiEntries);
          if (symbolUrl !== undefined) {
            options.decorations?.push({
              transform: (el) => {
                el.tagName = 'a';
                el.properties['href'] = symbolUrl;
                return el;
              },
              start: scanner.getTokenStart(),
              end: scanner.getTokenEnd(),
            });
          }
        }

        token = scanner.scan();
      }

      return code;
    },
  };

Review comment (suggestion):
nit:
```suggestion
      options.decorations ??= [];
```

Code after:
      if (highlight?.has(lineNumber)) {
        this.addClassToHast(node, 'highlighted');
      }
    },
  };
}

/** A custom transformer which removes all of the whitespace between lines of code in the generated output. */
function removeWhitespaceTransformer(): ShikiTransformer {
  return {
    code(code) {
      code.children = code.children.filter(
        (line) => line.type !== 'text' || line.value.trim().length !== 0,
      );
    },
  };
}

/** A custom transformer which adds a link to local API entries whenever a matching identifier is discovered in the code block. */
function linkApiEntriesTransformer(apiEntries?: ApiEntries): ShikiTransformer {
  if (apiEntries === undefined) {
    return {};
  }
  return {
    preprocess(code, options) {
      options.decorations ??= [];
      scanner.setText(code);
      let token = scanner.scan();

      while (token !== ts.SyntaxKind.EndOfFileToken) {
        if (token === ts.SyntaxKind.Identifier) {
          const symbolUrl = getSymbolUrl(scanner.getTokenText(), apiEntries);
          if (symbolUrl !== undefined) {
            options.decorations.push({
              transform: (el) => {
                el.tagName = 'a';
                el.properties['href'] = symbolUrl;
                return el;
              },
              start: scanner.getTokenStart(),
              end: scanner.getTokenEnd(),
            });
          }
        }

        token = scanner.scan();
      }

      return code;
    },
  };
Repository: angular/angular
File: adev/shared-docs/pipeline/shared/shiki.mts (TypeScript)

Code before:
function removeWhitespaceTransformer(highlight?: Set<number>): ShikiTransformer {
  return {
    code(code) {
      code.children = code.children.filter(
        (line) => line.type !== 'text' || line.value.trim().length !== 0,
      );
    },
  };
}

/** A custom transformer which removes all of the whitespace between lines of code in the generated output. */
function linkApiEntriesTransformer(apiEntries?: ApiEntries): ShikiTransformer {
  if (apiEntries === undefined) {
    return {};
  }
  return {
    preprocess(code, options) {
      options.decorations ||= [];
      scanner.setText(code);
      let token = scanner.scan();

      while (token !== SyntaxKind.EndOfFileToken) {
        if (token === SyntaxKind.Identifier) {
          const symbolUrl = getSymbolUrl(scanner.getTokenText(), apiEntries);
          if (symbolUrl !== undefined) {
            options.decorations?.push({
              transform: (el) => {
                el.tagName = 'a';
                el.properties['href'] = symbolUrl;
                return el;
              },
              start: scanner.getTokenStart(),
              end: scanner.getTokenEnd(),
            });
          }
        }

        token = scanner.scan();
      }

      return code;
    },
  };
}


Review comment (suggestion):
We should be ok without it. 
```suggestion
            options.decorations.push({
```

Code after:
function removeWhitespaceTransformer(): ShikiTransformer {
  return {
    code(code) {
      code.children = code.children.filter(
        (line) => line.type !== 'text' || line.value.trim().length !== 0,
      );
    },
  };
}

/** A custom transformer which adds a link to local API entries whenever a matching identifier is discovered in the code block. */
function linkApiEntriesTransformer(apiEntries?: ApiEntries): ShikiTransformer {
  if (apiEntries === undefined) {
    return {};
  }
  return {
    preprocess(code, options) {
      options.decorations ??= [];
      scanner.setText(code);
      let token = scanner.scan();

      while (token !== ts.SyntaxKind.EndOfFileToken) {
        if (token === ts.SyntaxKind.Identifier) {
          const symbolUrl = getSymbolUrl(scanner.getTokenText(), apiEntries);
          if (symbolUrl !== undefined) {
            options.decorations.push({
              transform: (el) => {
                el.tagName = 'a';
                el.properties['href'] = symbolUrl;
                return el;
              },
              start: scanner.getTokenStart(),
              end: scanner.getTokenEnd(),
            });
          }
        }

        token = scanner.scan();
      }

      return code;
    },
  };
}
Repository: angular/angular
File: packages/forms/signals/src/field/node.ts (TypeScript)

Code before:
   * Note this does not change the data model, which can be reset directly if desired.
   *
   * @param value Optional value to set to the form. If not passed, the value will not be changed.
   */
  reset(value?: unknown): void {
    untracked(() => this._reset(value));
  }

  private _reset(value?: unknown) {
    if (value !== undefined) {
      this.value.set(value);
    }

    this.nodeState.markAsUntouched();
    this.nodeState.markAsPristine();

    for (const child of this.structure.children()) {
      child._reset();
    }
  }

  /**
   * Creates a linked signal that initiates a {@link debounceSync} when set.
   */
  private controlValueSignal(): WritableSignal<unknown> {
    const signal = createLinkedSignal(this.value, identityFn);
    const node = signal[SIGNAL] as LinkedSignalNode<unknown, unknown>;
    const writableSignal = signal as LinkedSignalGetter<unknown, unknown> & WritableSignal<unknown>;

    writableSignal.set = (newValue) => {
      linkedSignalSetFn(node, newValue);
      this.markAsDirty();
      this.debounceSync();
    };
    writableSignal.update = (updateFn) => {
      linkedSignalUpdateFn(node, updateFn);
      this.markAsDirty();
      this.debounceSync();
    };
    writableSignal.asReadonly = signalAsReadonlyFn.bind(signal);

    return writableSignal;
  }

  /**
   * Synchronizes the {@link controlValue} with the {@link value} signal immediately.
   */
  private sync() {
    this.value.set(this.controlValue());
  }


Review comment (suggestion):
You might've already seen my slack message, but I wouldn't bring in `primitives/signals` for this. Instead:
```ts
const signal = linkedSignal(...);
const rawSet = signal.set;
const rawUpdate = signal.update;
signal.set = (value: unknown) => {
  rawSet(value);
  this.markAsDirty();
  this.debounceSync();
};
signal.update = (updateFn) => {
  rawUpdate(updateFn);
  this.markAsDirty();
  this.debounceSync();
};
```

Code after:
      this.value.set(value);
    }

    this.nodeState.markAsUntouched();
    this.nodeState.markAsPristine();

    for (const child of this.structure.children()) {
      child._reset();
    }
  }

  /**
   * Creates a linked signal that initiates a {@link debounceSync} when set.
   */
  private controlValueSignal(): WritableSignal<unknown> {
    const controlValue = linkedSignal(this.value);
    const {set, update} = controlValue;

    controlValue.set = (newValue) => {
      set(newValue);
      this.markAsDirty();
      this.debounceSync();
    };
    controlValue.update = (updateFn) => {
      update(updateFn);
      this.markAsDirty();
      this.debounceSync();
    };

    return controlValue;
  }

  /**
   * Synchronizes the {@link controlValue} with the {@link value} signal immediately.
   */
  private sync() {
    this.value.set(this.controlValue());
  }

  /**
   * If there is a pending sync, abort it and sync immediately.
   */
  private flushSync() {
    const pending = this.pendingSync();
    if (pending && !pending.signal.aborted) {
      pending.abort();
      this.sync();
    }
  }

  /**